opt: fix detection of correlated subqueries in complex filters #46153
Conversation
|
This change is |
| @@ -1095,6 +1098,149 @@ inner-join (hash) | |||
| └── filters | |||
| └── d.x:11 = xy.x:14 [outer=(11,14), constraints=(/11: (/NULL - ]; /14: (/NULL - ]), fd=(11)==(14), (14)==(11)] | |||
|
|
|||
| # Regression test for #46151. Do not push down a filter with a correlated | |||
| # subquery. | |||
| norm expect-not=PushFilterIntoJoinLeftAndRight | |||
There was a problem hiding this comment.
Can this test be further simplified? These huge SQL-Smith style queries are a real pain to deal with when plans change.
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @RaduBerinde and @rytaft)
pkg/sql/opt/memo/logical_props_builder.go, line 1351 at r1 (raw file):
// expression's subtree. It will only recurse into a child when it is not // already caching properties. func BuildSharedProps(e opt.Expr, shared *props.Shared) {
I think we ran into a similar bug before.. Let's add a comment here explaining that shared is an "input-output" argument, and should be assumed to be "partially filled in" already. It's hard to explain in general, maybe it would also help to specify exactly which fields are filled in.
Prior to this commit, it was possible for a correlated subquery to go undetected if it was buried in a complex filter. In particular, a filter of the form: <correlated subquery> OR <non-correlated subquery> would incorrectly be marked as *not* containing a correlated subquery. This was because although the logical property HasCorrelatedSubquery was initially set to true upon encountering the first (correlated) subquery, the left-to-right recursive traversal of the OR expression caused HasCorrelatedSubquery to be overwritten to false upon encountering the second (non-correlated) subquery. This commit fixes the issue by never overwriting HasCorrelatedSubquery to false. Fixes cockroachdb#46151 Release note (bug fix): Fixed an internal error that could occur in the optimizer when a WHERE filter contained at least one correlated subquery and one non-correlated subquery. Release justification: This bug fix falls into the category "low risk, high benefit changes to existing functionality".
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/memo/logical_props_builder.go, line 1351 at r1 (raw file):
Previously, RaduBerinde wrote…
I think we ran into a similar bug before.. Let's add a comment here explaining that
sharedis an "input-output" argument, and should be assumed to be "partially filled in" already. It's hard to explain in general, maybe it would also help to specify exactly which fields are filled in.
Done.
There was a problem hiding this comment.
Reviewable status:
|
TFTR! bors r+ |
Build failed (retrying...) |
Build succeeded |
In a small oversight, cockroachdb#46153 added some new checks to the check_expr.go file, but did not wrap the error messages with log.Safe. This commit fixes these error messages to match the rest of the file. Release justification: This is a low-risk bug fix to new functionality. Release note: None
46229: opt: add safe logging to new test code r=rytaft a=rytaft In a small oversight, #46153 added some new checks to the `check_expr.go file`, but did not wrap the error messages with `log.Safe`. This commit fixes these error messages to match the rest of the file. Release justification: This is a low-risk bug fix to new functionality. Release note: None Co-authored-by: Rebecca Taft <[email protected]>
In a small oversight, cockroachdb#46153 added some new checks to the check_expr.go file, but did not wrap the error messages with log.Safe. This commit fixes these error messages to match the rest of the file. Release justification: This is a low-risk bug fix to new functionality. Release note: None
Prior to this commit, it was possible for a correlated subquery to
go undetected if it was buried in a complex filter. In particular,
a filter of the form:
would incorrectly be marked as not containing a correlated subquery.
This was because although the logical property
HasCorrelatedSubquerywas initially set to true upon encountering the first (correlated)
subquery, the left-to-right recursive traversal of the
ORexpressioncaused
HasCorrelatedSubqueryto be overwritten to false upon encounteringthe second (non-correlated) subquery.
This commit fixes the issue by never overwriting
HasCorrelatedSubqueryto false.
Fixes #46151
Release note (bug fix): Fixed an internal error that could occur in the
optimizer when a WHERE filter contained at least one correlated subquery
and one non-correlated subquery.
Release justification: This bug fix falls into the category "low risk,
high benefit changes to existing functionality".