Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opt: fix detection of correlated subqueries in complex filters #46153

Merged
merged 1 commit into from
Mar 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions pkg/sql/opt/memo/check_expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,19 @@ func (m *Memo) CheckExpr(e opt.Expr) {

default:
if opt.IsJoinOp(e) {
left := e.Child(0).(RelExpr)
right := e.Child(1).(RelExpr)
// The left side cannot depend on the right side columns.
if left.Relational().OuterCols.Intersects(right.Relational().OutputCols) {
panic(errors.AssertionFailedf("%s left side has outer cols in right side", e.Op()))
}

// The reverse is allowed but only for apply variants.
if !opt.IsJoinApplyOp(e) {
if right.Relational().OuterCols.Intersects(left.Relational().OutputCols) {
panic(errors.AssertionFailedf("%s is correlated", e.Op()))
}
}
checkFilters(*e.Child(2).(*FiltersExpr))
}
}
Expand Down
13 changes: 10 additions & 3 deletions pkg/sql/opt/memo/logical_props_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1348,6 +1348,11 @@ func (b *logicalPropsBuilder) buildZipItemProps(item *ZipItem, scalar *props.Sca
// BuildSharedProps fills in the shared properties derived from the given
// expression's subtree. It will only recurse into a child when it is not
// already caching properties.
//
// Note that shared is an "input-output" argument, and should be assumed
// to be partially filled in already. Boolean fields such as HasPlaceholder,
// CanHaveSideEffects and HasCorrelatedSubquery should never be reset to false
// once set to true.
func BuildSharedProps(e opt.Expr, shared *props.Shared) {
switch t := e.(type) {
case *VariableExpr:
Expand All @@ -1365,9 +1370,11 @@ func BuildSharedProps(e opt.Expr, shared *props.Shared) {

case *SubqueryExpr, *ExistsExpr, *AnyExpr, *ArrayFlattenExpr:
shared.HasSubquery = true
shared.HasCorrelatedSubquery = !e.Child(0).(RelExpr).Relational().OuterCols.Empty()
if t.Op() == opt.AnyOp && !shared.HasCorrelatedSubquery {
shared.HasCorrelatedSubquery = hasOuterCols(e.Child(1))
if hasOuterCols(e.Child(0)) {
shared.HasCorrelatedSubquery = true
}
if t.Op() == opt.AnyOp && hasOuterCols(e.Child(1)) {
shared.HasCorrelatedSubquery = true
}

case *FunctionExpr:
Expand Down
88 changes: 88 additions & 0 deletions pkg/sql/opt/norm/testdata/rules/join
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import file=tpcc_schema
----

exec-ddl
CREATE TABLE a (k INT PRIMARY KEY, i INT, f FLOAT NOT NULL, s STRING, j JSON)
----
Expand Down Expand Up @@ -1095,6 +1098,91 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this test be further simplified? These huge SQL-Smith style queries are a real pain to deal with when plans change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

SELECT (SELECT i_name FROM item LIMIT 1)
FROM history INNER JOIN order_line ON h_data = ol_dist_info
WHERE (
EXISTS(
SELECT *
FROM history
WHERE h_data IS NOT NULL AND ol_dist_info IS NOT NULL
)
)
OR (SELECT ol_i_id FROM order_line LIMIT 1) IS NOT NULL;
----
project
├── columns: i_name:47
├── fd: ()-->(47)
├── inner-join (hash)
│ ├── columns: h_data:9!null ol_o_id:10!null ol_d_id:11!null ol_w_id:12!null ol_number:13!null ol_dist_info:19!null true_agg:40
│ ├── fd: (10-13)-->(19,40), (9)==(19), (19)==(9)
│ ├── scan history
│ │ └── columns: h_data:9
│ ├── select
│ │ ├── columns: ol_o_id:10!null ol_d_id:11!null ol_w_id:12!null ol_number:13!null ol_dist_info:19 true_agg:40
│ │ ├── key: (10-13)
│ │ ├── fd: (10-13)-->(19,40)
│ │ ├── group-by
│ │ │ ├── columns: ol_o_id:10!null ol_d_id:11!null ol_w_id:12!null ol_number:13!null ol_dist_info:19 true_agg:40
│ │ │ ├── grouping columns: ol_o_id:10!null ol_d_id:11!null ol_w_id:12!null ol_number:13!null
│ │ │ ├── key: (10-13)
│ │ │ ├── fd: (10-13)-->(19,40)
│ │ │ ├── left-join (cross)
│ │ │ │ ├── columns: ol_o_id:10!null ol_d_id:11!null ol_w_id:12!null ol_number:13!null ol_dist_info:19 true:39
│ │ │ │ ├── fd: (10-13)-->(19)
│ │ │ │ ├── scan order_line
│ │ │ │ │ ├── columns: ol_o_id:10!null ol_d_id:11!null ol_w_id:12!null ol_number:13!null ol_dist_info:19
│ │ │ │ │ ├── key: (10-13)
│ │ │ │ │ └── fd: (10-13)-->(19)
│ │ │ │ ├── project
│ │ │ │ │ ├── columns: true:39!null
│ │ │ │ │ ├── fd: ()-->(39)
│ │ │ │ │ ├── select
│ │ │ │ │ │ ├── columns: h_data:28!null
│ │ │ │ │ │ ├── scan history
│ │ │ │ │ │ │ └── columns: h_data:28
│ │ │ │ │ │ └── filters
│ │ │ │ │ │ └── h_data:28 IS NOT NULL [outer=(28), constraints=(/28: (/NULL - ]; tight)]
│ │ │ │ │ └── projections
│ │ │ │ │ └── true [as=true:39]
│ │ │ │ └── filters
│ │ │ │ └── ol_dist_info:19 IS NOT NULL [outer=(19), constraints=(/19: (/NULL - ]; tight)]
│ │ │ └── aggregations
│ │ │ ├── const-not-null-agg [as=true_agg:40, outer=(39)]
│ │ │ │ └── true:39
│ │ │ └── const-agg [as=ol_dist_info:19, outer=(19)]
│ │ │ └── ol_dist_info:19
│ │ └── filters
│ │ └── or [outer=(40), subquery]
│ │ ├── true_agg:40 IS NOT NULL
│ │ └── is-not
│ │ ├── subquery
│ │ │ └── limit
│ │ │ ├── columns: ol_i_id:33!null
│ │ │ ├── cardinality: [0 - 1]
│ │ │ ├── key: ()
│ │ │ ├── fd: ()-->(33)
│ │ │ ├── scan order_line
│ │ │ │ ├── columns: ol_i_id:33!null
│ │ │ │ └── limit hint: 1.00
│ │ │ └── 1
│ │ └── NULL
│ └── filters
│ └── h_data:9 = ol_dist_info:19 [outer=(9,19), constraints=(/9: (/NULL - ]; /19: (/NULL - ]), fd=(9)==(19), (19)==(9)]
└── projections
└── subquery [as=i_name:47, subquery]
└── limit
├── columns: item.i_name:44
├── cardinality: [0 - 1]
├── key: ()
├── fd: ()-->(44)
├── scan item
│ ├── columns: item.i_name:44
│ └── limit hint: 1.00
└── 1

# ---------------------------------
# MapEqualityIntoJoinLeftAndRight
# ---------------------------------
Expand Down