From c7772acb337535f19ef304d06c5e88c48d428a07 Mon Sep 17 00:00:00 2001 From: Andrew Kimball Date: Mon, 20 Aug 2018 11:09:57 -0700 Subject: [PATCH] opt: Fix rule cycle bug The PushFilterIntoJoinLeftAndRight and TryDecorrelateSelect rules can cycle with one another in a rare case: 1. Right side of join has outer column due to being un-decorrelatable. 2. Filter conjunct is pushed down to both left and right side by mapping equivalencies in PushFilterIntoJoinLeftAndRight. 3. Left conjunct is pulled back up to join condition by TryDecorrelateSelect. Steps #2 and #3 will cycle with one another. Cycle detection is not possible in this case, because the left side keeps changing (because new conjuct is pushed down to it each time). The fix is simple: don't let PushFilterIntoJoinLeftAndRight push down filters if either the left or right side has outer column(s). This fixes #28818. Release note: None --- pkg/sql/opt/norm/rules/join.opt | 13 +-- pkg/sql/opt/norm/testdata/rules/join | 125 +++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 6 deletions(-) diff --git a/pkg/sql/opt/norm/rules/join.opt b/pkg/sql/opt/norm/rules/join.opt index 6c182e23d664..0240f7aa1f55 100644 --- a/pkg/sql/opt/norm/rules/join.opt +++ b/pkg/sql/opt/norm/rules/join.opt @@ -49,17 +49,18 @@ # Given this mapping, we can safely push the filter down to both sides and # remove it from the ON filters list. # -# Note that this rule is only applied to InnerJoin and SemiJoin, not -# InnerJoinApply or SemiJoinApply. The apply variants would cause a -# non-detectable cycle with TryDecorrelateSelect, causing the filters to get -# remapped to both sides and pushed down over and over again. +# Note that this rule is only applied when the left and right inputs do not have +# outer columns. If they do, then this rule can cause undetectable cycles with +# TryDecorrelateSelect, since the filter is pushed down to both sides, but then +# only pulled up from the right side by TryDecorrelateSelect. For this reason, +# the rule also does not apply to InnerJoinApply or SemiJoinApply. # # NOTE: It is important that this rule is first among the join filter push-down # rules. [PushFilterIntoJoinLeftAndRight, Normalize] (InnerJoin | SemiJoin - $left:* - $right:* + $left:* & ^(HasOuterCols $left) + $right:* & ^(HasOuterCols $right) $filters:(Filters $list:[ ... diff --git a/pkg/sql/opt/norm/testdata/rules/join b/pkg/sql/opt/norm/testdata/rules/join index 072fe0629144..a21883ef155e 100644 --- a/pkg/sql/opt/norm/testdata/rules/join +++ b/pkg/sql/opt/norm/testdata/rules/join @@ -19,6 +19,24 @@ TABLE b └── INDEX primary └── x int not null +exec-ddl +CREATE TABLE xy (x INT PRIMARY KEY, y INT) +---- +TABLE xy + ├── x int not null + ├── y int + └── INDEX primary + └── x int not null + +exec-ddl +CREATE TABLE uv (u INT PRIMARY KEY, v INT) +---- +TABLE uv + ├── u int not null + ├── v int + └── INDEX primary + └── u int not null + # -------------------------------------------------- # EnsureJoinFiltersAnd # -------------------------------------------------- @@ -939,6 +957,113 @@ inner-join └── filters [type=bool, outer=(1,3), constraints=(/1: (/NULL - ]; /3: (/NULL - ]), fd=(1)==(3), (3)==(1)] └── a = b [type=bool, outer=(1,3), constraints=(/1: (/NULL - ]; /3: (/NULL - ])] +# Regression for issue 28818. Try to trigger undetectable cycle between the +# PushFilterIntoJoinLeftAndRight and TryDecorrelateSelect rules. +opt +SELECT 1 +FROM a +WHERE EXISTS ( + SELECT 1 + FROM xy + INNER JOIN uv + ON EXISTS ( + SELECT 1 + FROM b + WHERE a.s >= 'foo' + LIMIT 10 + ) + WHERE + (SELECT s FROM a) = 'foo' +) +---- +project + ├── columns: "?column?":22(int!null) + ├── fd: ()-->(22) + ├── distinct-on + │ ├── columns: a.k:1(int!null) + │ ├── grouping columns: a.k:1(int!null) + │ ├── key: (1) + │ └── select + │ ├── columns: a.k:1(int!null) xy.x:6(int!null) u:8(int!null) true_agg:14(bool!null) + │ ├── key: (1,6,8) + │ ├── fd: (1,6,8)-->(14) + │ ├── group-by + │ │ ├── columns: a.k:1(int!null) xy.x:6(int!null) u:8(int!null) true_agg:14(bool) + │ │ ├── grouping columns: a.k:1(int!null) xy.x:6(int!null) u:8(int!null) + │ │ ├── key: (1,6,8) + │ │ ├── fd: (1,6,8)-->(14) + │ │ ├── project + │ │ │ ├── columns: true:13(bool!null) a.k:1(int!null) xy.x:6(int!null) u:8(int!null) + │ │ │ ├── fd: ()-->(13) + │ │ │ ├── inner-join-apply + │ │ │ │ ├── columns: a.k:1(int!null) a.s:4(string) xy.x:6(int!null) u:8(int!null) + │ │ │ │ ├── fd: (1)-->(4) + │ │ │ │ ├── scan a + │ │ │ │ │ ├── columns: a.k:1(int!null) a.s:4(string) + │ │ │ │ │ ├── key: (1) + │ │ │ │ │ └── fd: (1)-->(4) + │ │ │ │ ├── inner-join + │ │ │ │ │ ├── columns: xy.x:6(int!null) u:8(int!null) + │ │ │ │ │ ├── outer: (4) + │ │ │ │ │ ├── inner-join + │ │ │ │ │ │ ├── columns: xy.x:6(int!null) u:8(int!null) + │ │ │ │ │ │ ├── key: (6,8) + │ │ │ │ │ │ ├── select + │ │ │ │ │ │ │ ├── columns: xy.x:6(int!null) + │ │ │ │ │ │ │ ├── key: (6) + │ │ │ │ │ │ │ ├── scan xy + │ │ │ │ │ │ │ │ ├── columns: xy.x:6(int!null) + │ │ │ │ │ │ │ │ └── key: (6) + │ │ │ │ │ │ │ └── filters [type=bool] + │ │ │ │ │ │ │ └── eq [type=bool] + │ │ │ │ │ │ │ ├── subquery [type=string] + │ │ │ │ │ │ │ │ └── max1-row + │ │ │ │ │ │ │ │ ├── columns: a.s:19(string) + │ │ │ │ │ │ │ │ ├── cardinality: [0 - 1] + │ │ │ │ │ │ │ │ ├── key: () + │ │ │ │ │ │ │ │ ├── fd: ()-->(19) + │ │ │ │ │ │ │ │ └── scan a + │ │ │ │ │ │ │ │ └── columns: a.s:19(string) + │ │ │ │ │ │ │ └── const: 'foo' [type=string] + │ │ │ │ │ │ ├── select + │ │ │ │ │ │ │ ├── columns: u:8(int!null) + │ │ │ │ │ │ │ ├── key: (8) + │ │ │ │ │ │ │ ├── scan uv + │ │ │ │ │ │ │ │ ├── columns: u:8(int!null) + │ │ │ │ │ │ │ │ └── key: (8) + │ │ │ │ │ │ │ └── filters [type=bool] + │ │ │ │ │ │ │ └── eq [type=bool] + │ │ │ │ │ │ │ ├── subquery [type=string] + │ │ │ │ │ │ │ │ └── max1-row + │ │ │ │ │ │ │ │ ├── columns: a.s:19(string) + │ │ │ │ │ │ │ │ ├── cardinality: [0 - 1] + │ │ │ │ │ │ │ │ ├── key: () + │ │ │ │ │ │ │ │ ├── fd: ()-->(19) + │ │ │ │ │ │ │ │ └── scan a + │ │ │ │ │ │ │ │ └── columns: a.s:19(string) + │ │ │ │ │ │ │ └── const: 'foo' [type=string] + │ │ │ │ │ │ └── true [type=bool] + │ │ │ │ │ ├── limit + │ │ │ │ │ │ ├── outer: (4) + │ │ │ │ │ │ ├── cardinality: [0 - 10] + │ │ │ │ │ │ ├── select + │ │ │ │ │ │ │ ├── outer: (4) + │ │ │ │ │ │ │ ├── scan b + │ │ │ │ │ │ │ └── filters [type=bool, outer=(4), constraints=(/4: [/'foo' - ]; tight)] + │ │ │ │ │ │ │ └── a.s >= 'foo' [type=bool, outer=(4), constraints=(/4: [/'foo' - ]; tight)] + │ │ │ │ │ │ └── const: 10 [type=int] + │ │ │ │ │ └── true [type=bool] + │ │ │ │ └── true [type=bool] + │ │ │ └── projections [outer=(1,6,8)] + │ │ │ └── true [type=bool] + │ │ └── aggregations [outer=(13)] + │ │ └── const-not-null-agg [type=bool, outer=(13)] + │ │ └── variable: true [type=bool, outer=(13)] + │ └── filters [type=bool, outer=(14), constraints=(/14: (/NULL - ]; tight)] + │ └── true_agg IS NOT NULL [type=bool, outer=(14), constraints=(/14: (/NULL - ]; tight)] + └── projections + └── const: 1 [type=int] + # -------------------------------------------------- # PushFilterIntoJoinLeft + PushFilterIntoJoinRight # --------------------------------------------------