From 198195fe44ba10e145b8db70bbc4190c90cdfc94 Mon Sep 17 00:00:00 2001 From: Rebecca Taft Date: Mon, 16 Mar 2020 10:47:27 -0500 Subject: [PATCH] opt: fix detection of correlated subqueries in complex filters 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: OR 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 #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". --- pkg/sql/opt/memo/check_expr.go | 13 ++ pkg/sql/opt/memo/logical_props_builder.go | 8 +- pkg/sql/opt/norm/testdata/rules/join | 146 ++++++++++++++++++++++ 3 files changed, 164 insertions(+), 3 deletions(-) diff --git a/pkg/sql/opt/memo/check_expr.go b/pkg/sql/opt/memo/check_expr.go index 043cb69b5cba..3eb33eace8eb 100644 --- a/pkg/sql/opt/memo/check_expr.go +++ b/pkg/sql/opt/memo/check_expr.go @@ -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)) } } diff --git a/pkg/sql/opt/memo/logical_props_builder.go b/pkg/sql/opt/memo/logical_props_builder.go index 007e41db60ae..be1ee2318e8c 100644 --- a/pkg/sql/opt/memo/logical_props_builder.go +++ b/pkg/sql/opt/memo/logical_props_builder.go @@ -1365,9 +1365,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: diff --git a/pkg/sql/opt/norm/testdata/rules/join b/pkg/sql/opt/norm/testdata/rules/join index db119b9a4065..8b6fef03357b 100644 --- a/pkg/sql/opt/norm/testdata/rules/join +++ b/pkg/sql/opt/norm/testdata/rules/join @@ -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) ---- @@ -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 +SELECT + subq_0.c0 AS c0, + subq_0.c0 AS c1, + subq_0.c0 AS c2, + subq_0.c0 AS c3, + CASE + WHEN subq_0.c1 IS NOT NULL THEN subq_0.c0 + ELSE subq_0.c0 + END + AS c4 +FROM + ( + SELECT + ref_0.h_data AS c0, + (SELECT i_name FROM item LIMIT 1 OFFSET 6) AS c1 + FROM + history AS ref_0 + INNER JOIN order_line AS ref_1 ON + (ref_0.h_data = ref_1.ol_dist_info) + WHERE + ( + EXISTS( + SELECT + ref_2.h_amount AS c0, + ref_2.h_c_w_id AS c1, + ref_2.h_c_d_id AS c2 + FROM + history AS ref_2 + WHERE + (ref_2.h_data IS NOT NULL) + AND (ref_1.ol_dist_info IS NOT NULL) + ) + ) + OR ( + (SELECT ol_i_id FROM order_line LIMIT 1 OFFSET 6) + IS NOT NULL + ) + LIMIT + 2 + ) + AS subq_0 +WHERE + subq_0.c1 IS NULL +LIMIT + 6 +---- +project + ├── columns: c0:9!null c1:9!null c2:9!null c3:9!null c4:48!null + ├── cardinality: [0 - 2] + ├── fd: (9)-->(48) + ├── select + │ ├── columns: ref_0.h_data:9!null c1:47 + │ ├── cardinality: [0 - 2] + │ ├── fd: ()-->(47) + │ ├── project + │ │ ├── columns: c1:47 ref_0.h_data:9!null + │ │ ├── cardinality: [0 - 2] + │ │ ├── fd: ()-->(47) + │ │ ├── limit + │ │ │ ├── columns: ref_0.h_data:9!null ref_1.ol_o_id:10!null ref_1.ol_d_id:11!null ref_1.ol_w_id:12!null ref_1.ol_number:13!null ref_1.ol_dist_info:19!null true_agg:40 + │ │ │ ├── cardinality: [0 - 2] + │ │ │ ├── fd: (10-13)-->(19,40), (9)==(19), (19)==(9) + │ │ │ ├── inner-join (hash) + │ │ │ │ ├── columns: ref_0.h_data:9!null ref_1.ol_o_id:10!null ref_1.ol_d_id:11!null ref_1.ol_w_id:12!null ref_1.ol_number:13!null ref_1.ol_dist_info:19!null true_agg:40 + │ │ │ │ ├── fd: (10-13)-->(19,40), (9)==(19), (19)==(9) + │ │ │ │ ├── limit hint: 2.00 + │ │ │ │ ├── scan ref_0 + │ │ │ │ │ └── columns: ref_0.h_data:9 + │ │ │ │ ├── select + │ │ │ │ │ ├── columns: ref_1.ol_o_id:10!null ref_1.ol_d_id:11!null ref_1.ol_w_id:12!null ref_1.ol_number:13!null ref_1.ol_dist_info:19 true_agg:40 + │ │ │ │ │ ├── key: (10-13) + │ │ │ │ │ ├── fd: (10-13)-->(19,40) + │ │ │ │ │ ├── group-by + │ │ │ │ │ │ ├── columns: ref_1.ol_o_id:10!null ref_1.ol_d_id:11!null ref_1.ol_w_id:12!null ref_1.ol_number:13!null ref_1.ol_dist_info:19 true_agg:40 + │ │ │ │ │ │ ├── grouping columns: ref_1.ol_o_id:10!null ref_1.ol_d_id:11!null ref_1.ol_w_id:12!null ref_1.ol_number:13!null + │ │ │ │ │ │ ├── key: (10-13) + │ │ │ │ │ │ ├── fd: (10-13)-->(19,40) + │ │ │ │ │ │ ├── left-join (cross) + │ │ │ │ │ │ │ ├── columns: ref_1.ol_o_id:10!null ref_1.ol_d_id:11!null ref_1.ol_w_id:12!null ref_1.ol_number:13!null ref_1.ol_dist_info:19 true:39 + │ │ │ │ │ │ │ ├── fd: (10-13)-->(19) + │ │ │ │ │ │ │ ├── scan ref_1 + │ │ │ │ │ │ │ │ ├── columns: ref_1.ol_o_id:10!null ref_1.ol_d_id:11!null ref_1.ol_w_id:12!null ref_1.ol_number:13!null ref_1.ol_dist_info:19 + │ │ │ │ │ │ │ │ ├── key: (10-13) + │ │ │ │ │ │ │ │ └── fd: (10-13)-->(19) + │ │ │ │ │ │ │ ├── project + │ │ │ │ │ │ │ │ ├── columns: true:39!null + │ │ │ │ │ │ │ │ ├── fd: ()-->(39) + │ │ │ │ │ │ │ │ ├── select + │ │ │ │ │ │ │ │ │ ├── columns: ref_2.h_data:28!null + │ │ │ │ │ │ │ │ │ ├── scan ref_2 + │ │ │ │ │ │ │ │ │ │ └── columns: ref_2.h_data:28 + │ │ │ │ │ │ │ │ │ └── filters + │ │ │ │ │ │ │ │ │ └── ref_2.h_data:28 IS NOT NULL [outer=(28), constraints=(/28: (/NULL - ]; tight)] + │ │ │ │ │ │ │ │ └── projections + │ │ │ │ │ │ │ │ └── true [as=true:39] + │ │ │ │ │ │ │ └── filters + │ │ │ │ │ │ │ └── ref_1.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=ref_1.ol_dist_info:19, outer=(19)] + │ │ │ │ │ │ └── ref_1.ol_dist_info:19 + │ │ │ │ │ └── filters + │ │ │ │ │ └── or [outer=(40), subquery] + │ │ │ │ │ ├── true_agg:40 IS NOT NULL + │ │ │ │ │ └── is-not + │ │ │ │ │ ├── subquery + │ │ │ │ │ │ └── offset + │ │ │ │ │ │ ├── columns: order_line.ol_i_id:33!null + │ │ │ │ │ │ ├── cardinality: [0 - 1] + │ │ │ │ │ │ ├── limit + │ │ │ │ │ │ │ ├── columns: order_line.ol_i_id:33!null + │ │ │ │ │ │ │ ├── cardinality: [0 - 7] + │ │ │ │ │ │ │ ├── scan order_line + │ │ │ │ │ │ │ │ ├── columns: order_line.ol_i_id:33!null + │ │ │ │ │ │ │ │ └── limit hint: 7.00 + │ │ │ │ │ │ │ └── 7 + │ │ │ │ │ │ └── 6 + │ │ │ │ │ └── NULL + │ │ │ │ └── filters + │ │ │ │ └── ref_0.h_data:9 = ref_1.ol_dist_info:19 [outer=(9,19), constraints=(/9: (/NULL - ]; /19: (/NULL - ]), fd=(9)==(19), (19)==(9)] + │ │ │ └── 2 + │ │ └── projections + │ │ └── subquery [as=c1:47, subquery] + │ │ └── offset + │ │ ├── columns: i_name:44 + │ │ ├── cardinality: [0 - 1] + │ │ ├── limit + │ │ │ ├── columns: i_name:44 + │ │ │ ├── cardinality: [0 - 7] + │ │ │ ├── scan item + │ │ │ │ ├── columns: i_name:44 + │ │ │ │ └── limit hint: 7.00 + │ │ │ └── 7 + │ │ └── 6 + │ └── filters + │ └── c1:47 IS NULL [outer=(47), constraints=(/47: [/NULL - /NULL]; tight), fd=()-->(47)] + └── projections + └── CASE WHEN c1:47 IS NOT NULL THEN ref_0.h_data:9 ELSE ref_0.h_data:9 END [as=c4:48, outer=(9,47)] + # --------------------------------- # MapEqualityIntoJoinLeftAndRight # ---------------------------------