opt: project required ordering for EXPLAIN to input columns#88441
opt: project required ordering for EXPLAIN to input columns#88441craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
8c00f82 to
b11e75f
Compare
mgartner
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball and @mgartner)
pkg/sql/logictest/testdata/logic_test/explain line 33 at r1 (raw file):
FROM generate_series(1, 5) AS g; statement ok
Can you make this a query T instead so we can see that the query plan has the correct sorting applied?
Previously, `EXPLAIN` operators could require an ordering on non-output columns from their enclosed expression, which causes a panic. This could happen after column-pruning rules fired. This commit modifies the order-building logic to project the ordering required of the child of an `EXPLAIN` operator to only reference output columns. Fixes cockroachdb#88037 Release note (bug fix): Fixed a longstanding bug that could cause a panic when running a query with `EXPLAIN` that attempts to order on a non-output column.
b11e75f to
884fc3a
Compare
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/explain line 33 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
Can you make this a
query Tinstead so we can see that the query plan has the correct sorting applied?
Good idea. Done.
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:
|
Let's merge this to fix the sqlsmith failures. bors r=mgartner,yuzefovich |
|
Build succeeded: |
|
@DrewKimball Should this be backported to the 22.2.0 branch? SQLancer might be running into the bug: sqlancer/sqlancer#587 (comment) |
|
I wanted to avoid backporting a fix for an old bug, but it's had some time to bake at this point, so I guess we could. How much of a problem are the failures? |
|
Good point. They aren't a huge problem, we can ignore them in SQLancer until v22.2.1 is released. |
Previously,
EXPLAINoperators could require an ordering on non-output columns from their enclosed expression, which causes a panic. This could happen after column-pruning rules fired. This commit modifies the order-building logic to project the ordering required of the child of anEXPLAINoperator to only reference output columns.Fixes #88037
Release note (bug fix): Fixed a longstanding bug that could cause a panic when running a query with
EXPLAINthat attempts to order on a non-output column.