sql: fix: EXPLAIN checks for VectorizeRowCountThreshold #40757
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 5 of 5 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @rafiss)
pkg/sql/logictest/testdata/logic_test/vectorize_threshold, line 21 at r1 (raw file):
SELECT url FROM [EXPLAIN ANALYZE SELECT count(*) FROM small] ---- https://cockroachdb.github.io/distsqlplan/decode.html#eJyUkc1q6zAQhff3KcRZ3YJK7Cy16s8qm7gkKV0UUxRrMAbJMjMyTQh-92Ib2qbQNl3OmfmOPtAJbXS0toEE5hk5So2OY0UikcdoPli5A0ym0bRdn8a41KgiE8wJqUmeYLCze08bso54kUHDUbKNn2o7boLl440E6z00tp1txahraBR9MmodW4IGx1dRTNYZNRZIst6r1AQyKhNo7I-J3g_UHcpBI_bpQ0mSrQkmH_Tl2rd1zVTbFHmRn1vfF4_r3cumeNr-v7rALtiDChQiH1Uv9Ivi8i-KG5IutkJnet81Z0OpQa6m-fck9lzRA8dqemYei4mbAkeS5m0-D6t2Xo2Cn-H8R3j5BS6Hf28BAAD__xzlxHs=
I think you can now remove all EXPLAIN ANALYZE queries from this file. I used those to check whether the flow was vectorized, and with your change, EXPLAIN tells us that :)
pkg/sql/opt/exec/execbuilder/testdata/prepare, line 4 at r1 (raw file):
statement ok CREATE TABLE ab (a INT PRIMARY KEY, b INT); INSERT INTO ab (a, b) VALUES (1, 10)
We should check in with the optimizer team whether the data in these files needs to be actually distributed so that we don't lose test coverage.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/vectorize_threshold, line 21 at r1 (raw file):
Previously, yuzefovich wrote…
I think you can now remove all
EXPLAIN ANALYZEqueries from this file. I used those to check whether the flow was vectorized, and with your change,EXPLAINtells us that :)
ah yeah, good point!
pkg/sql/opt/exec/execbuilder/testdata/prepare, line 4 at r1 (raw file):
Previously, yuzefovich wrote…
We should check in with the optimizer team whether the data in these files needs to be actually distributed so that we don't lose test coverage.
i will ask them
EXPLAIN recently was updated so it shows if a query is going to run on the vectorized engine. But it was not respecting row count thresholds, so the output was not accurate. Fix and test. Also, switch optlogic tests that were running on fakedist to 5node-dist to make the tests less flaky when checking if a query will run vectorized. Release justification: This fixes a bug with a new UX that is part of 19.2, and also decreases test flakes. Release note: None
rafiss
force-pushed
the
explain-vec-fix
branch
from
0e41166
to
9b38e57
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/exec/execbuilder/testdata/prepare, line 4 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
i will ask them
Should be fine.
There was a problem hiding this comment.
There are now a few open issues because of this flakiness. Also, I'll need to rebase on top of the distsqlrun refactor.
Reviewed 2 of 2 files at r2.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/vectorize_threshold, line 21 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
ah yeah, good point!
Thanks!
|
thanks all! bors r+ |
40757: sql: fix: EXPLAIN checks for VectorizeRowCountThreshold r=rafiss a=rafiss EXPLAIN recently was updated so it shows if a query is going to run on the vectorized engine. But it was not respecting row count thresholds, so the output was not accurate. Fix and test. Also, switch optlogic tests that were running on fakedist to 5node-dist to make the tests less flaky when checking if a query will run vectorized. Release justification: This fixes a bug with a new UX that is part of 19.2, and also decreases test flakes. Release note: None Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Build succeeded |
EXPLAIN recently was updated so it shows if a query is going to run on
the vectorized engine. But it was not respecting row count thresholds,
so the output was not accurate. Fix and test.
Also, switch optlogic tests that were running on fakedist to 5node-dist
to make the tests less flaky when checking if a query will run
vectorized.
Release justification: This fixes a bug with a new UX that is part of
19.2, and also decreases test flakes.
Release note: None