opt: fix bug in ExtractJoinEqualities rule #57524
Conversation
|
This change is |
You mean "with this fix"? |
| @@ -551,9 +551,13 @@ func (c *CustomFuncs) ExtractJoinEquality( | |||
| newFilters, | |||
| private, | |||
| ) | |||
| outputCols := leftCols | |||
| if joinOp != opt.SemiJoinOp && joinOp != opt.AntiJoinOp { | |||
There was a problem hiding this comment.
[nit] maybe explain here that semi and anti joins don't output columns from the right side, unless you think that's obvious enough that an explanation isn't warranted.
If the join is semi/anti, this rule generates an incorrect projection (which passes through columns not in input). Similar to cockroachdb#57501. Other normalization rules prune the columns so this doesn't lead to a bad outcome (at least in most cases). I will add an assertion in CheckExpr for this condition in a separate PR - without this fix, the assertion fires on an existing test. Release note: None
RaduBerinde
force-pushed
the
inline-eq-join-bug
branch
from
3478c03 to
7614e3f
Compare
There was a problem hiding this comment.
Without this fix (this PR), if I add the assertion to CheckExpr, it panics on an existing test. Which is to say, the condition addressed here will be covered by tests once I make that other change.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @mgartner and @rytaft)
pkg/sql/opt/norm/join_funcs.go, line 555 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
[nit] maybe explain here that semi and anti joins don't output columns from the right side, unless you think that's obvious enough that an explanation isn't warranted.
Done.
|
bors r+ |
|
Build failed: |
|
bors r+ |
|
Build succeeded: |
If the join is semi/anti, this rule generates an incorrect projection
(which passes through columns not in input).
Similar to #57501. Other normalization rules prune the columns so this
doesn't lead to a bad outcome (at least in most cases).
I will add an assertion in CheckExpr for this condition in a separate
PR - without this fix, the assertion fires on an existing test.
Release note: None