opt: improve CanBeCompositeSensitive #68007
Conversation
|
This change is |
RaduBerinde
force-pushed
the
comp-insensitive-cleanup
branch
from
ad20a21
to
41aaaa2
Compare
There was a problem hiding this comment.
Reviewed 2 of 9 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mgartner)
There was a problem hiding this comment.
Brilliant. 
Reviewed 9 of 9 files at r1.
Reviewable status:
The current implementation of CanBeCompositeSensitive can be quadratic in the expression size because each node builds the entire set of outer columns for that subtree. This change reworks the code and defines the logical property that we were approximating by checking the outer cols directly. This also reveals a bug in the previous version which did not detect relational subtrees when there are no outer columns. Release note: None
RaduBerinde
force-pushed
the
comp-insensitive-cleanup
branch
from
41aaaa2
to
6c45df7
Compare
|
Had to update a test for |
There was a problem hiding this comment.
Had to update a test for PushFilterIntoSetOp - the composite check now disallows pushing down things with subqueries. Not sure if it's a problem; if it is, maybe the composite check can ignore self-contained (uncorrelated) subqueries.
The sub-queries that were being pushed down before seemed to add unnecessary work - there were multiple scans on a and now there is just one. I can't think of a case where pushing an uncorrelated sub-query into a set op would be desirable.
Reviewed 1 of 1 files at r2.
Reviewable status:
pkg/sql/opt/norm/testdata/rules/select, line 1667 at r2 (raw file):
norm expect=PushFilterIntoSetOp SELECT * FROM (SELECT k FROM b
Did you intend to remove this test? I guess it's similar to the one below?
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/norm/testdata/rules/select, line 1667 at r2 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
Did you intend to remove this test? I guess it's similar to the one below?
Yeah, didn't seem helpful given that we no longer push it down in this case.
|
bors r+ |
|
Build succeeded: |
The current implementation of CanBeCompositeSensitive can be quadratic
in the expression size because each node builds the entire set of
outer columns for that subtree.
This change reworks the code and defines the logical property that we
were approximating by checking the outer cols directly. This also
reveals a bug in the previous version which did not detect relational
subtrees when there are no outer columns.
Release note: None