Skip to content

Commit

Permalink
Merge #46153 #46160
Browse files Browse the repository at this point in the history
46153: opt: fix detection of correlated subqueries in complex filters r=rytaft a=rytaft

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 #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".

46160: vendor: Bump pebble to c50a7b1164d9b1a971f1729d84ff9cdb7e987496 r=itsbilal a=itsbilal

Picks up these changes:
 - iterator: Copy iterValue before Prev() on internal iterator
 - db: add a section on RocksDB Compatibility
 - db: rework doc comment on Iterator.SeekPrefixGE

Release justification: Bug fixes.
Release note: None

Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>
  • Loading branch information
3 people committed Mar 16, 2020
3 parents 4ed99d9 + c1c285e + 370d834 commit 7af379d
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 6 deletions.
4 changes: 2 additions & 2 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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
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
2 changes: 1 addition & 1 deletion vendor

0 comments on commit 7af379d

Please sign in to comment.