opt: fix error due to incorrect number of columns after EliminateDistinct #40468
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @justinj, @RaduBerinde, and @rytaft)
pkg/sql/opt/norm/rules/groupby.opt, line 21 at r1 (raw file):
# redundant and can be eliminated. # # Since a DistrinctOn operator can serve as a projection operator, we need to
nit: DistrinctOn
…inct This commit fixes an error that could occur due to the EliminateDistinct rule, which eliminates a DistinctOn operator when the grouping columns are known to form a strong key. The problem with eliminating the DistinctOn completely is that it has a secondary purpose as a Project operator, and removing it could result in an incorrect number of output columns. This commit fixes that rule so that it replaces the DistinctOn with a Project operator. Release note (bug fix): Fixed an internal planning error that could occur when a DISTINCT or GROUP BY expression was contained in a subquery.
There was a problem hiding this comment.
TFTR!
Don't we have some testrace tool that validates that distinct sets of logical properties are mutually consistent somehow? I wonder if it would be possible to apply that to norm rules somehow?
That's a good idea, although it would be tricky. As written, checkExpr checks that the logical properties calculated from each expression in a group (produced by exploration rules) are consistent. The problem with norm rules is that they only add one expression to the group, so we'd need to find a way to selectively disable some norm rules and check that all resulting expressions produce the same logical properties. Might be worth doing as a test infrastructure project once we finish all the 19.2 issues....
Reviewable status:
pkg/sql/opt/norm/rules/groupby.opt, line 21 at r1 (raw file):
Previously, justinj (Justin Jaffray) wrote…
nit:
DistrinctOn
Done.
There was a problem hiding this comment.
bors r+
Reviewable status:
40468: opt: fix error due to incorrect number of columns after EliminateDistinct r=rytaft a=rytaft This commit fixes an error that could occur due to the `EliminateDistinct` rule, which eliminates a `DistinctOn` operator when the grouping columns are known to form a strong key. The problem with eliminating the `DistinctOn` completely is that it has a secondary purpose as a `Project` operator, and removing it could result in an incorrect number of output columns. This commit fixes that rule so that it replaces the `DistinctOn` with a `Project` operator. Fixes #40295 Release note (bug fix): Fixed an internal planning error that could occur when a DISTINCT or GROUP BY expression was contained in a subquery. Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
Build succeeded |
This commit fixes an error that could occur due to the
EliminateDistinctrule, which eliminates a
DistinctOnoperator when the grouping columnsare known to form a strong key. The problem with eliminating the
DistinctOncompletely is that it has a secondary purpose as a
Projectoperator, andremoving it could result in an incorrect number of output columns. This
commit fixes that rule so that it replaces the
DistinctOnwith aProjectoperator.
Fixes #40295
Release note (bug fix): Fixed an internal planning error that could
occur when a DISTINCT or GROUP BY expression was contained in a subquery.