Skip to content

Commit c7772ac

Browse files
andy-kimballandy-kimball
committed
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 cockroachdb#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 cockroachdb#28818. Release note: None
1 parent 6c463b9 commit c7772ac

File tree

2 files changed

+132
-6
lines changed

2 files changed

+132
-6
lines changed

‎pkg/sql/opt/norm/rules/join.opt‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,18 @@
4949
# Given this mapping, we can safely push the filter down to both sides and
5050
# remove it from the ON filters list.
5151
#
52-
# Note that this rule is only applied to InnerJoin and SemiJoin, not
53-
# InnerJoinApply or SemiJoinApply. The apply variants would cause a
54-
# non-detectable cycle with TryDecorrelateSelect, causing the filters to get
55-
# remapped to both sides and pushed down over and over again.
52+
# Note that this rule is only applied when the left and right inputs do not have
53+
# outer columns. If they do, then this rule can cause undetectable cycles with
54+
# TryDecorrelateSelect, since the filter is pushed down to both sides, but then
55+
# only pulled up from the right side by TryDecorrelateSelect. For this reason,
56+
# the rule also does not apply to InnerJoinApply or SemiJoinApply.
5657
#
5758
# NOTE: It is important that this rule is first among the join filter push-down
5859
# rules.
5960
[PushFilterIntoJoinLeftAndRight, Normalize]
6061
(InnerJoin | SemiJoin
61-
$left:*
62-
$right:*
62+
$left:* & ^(HasOuterCols $left)
63+
$right:* & ^(HasOuterCols $right)
6364
$filters:(Filters
6465
$list:[
6566
...

‎pkg/sql/opt/norm/testdata/rules/join‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,24 @@ TABLE b
1919
└── INDEX primary
2020
└── x int not null
2121

22+
exec-ddl
23+
CREATE TABLE xy (x INT PRIMARY KEY, y INT)
24+
----
25+
TABLE xy
26+
├── x int not null
27+
├── y int
28+
└── INDEX primary
29+
└── x int not null
30+
31+
exec-ddl
32+
CREATE TABLE uv (u INT PRIMARY KEY, v INT)
33+
----
34+
TABLE uv
35+
├── u int not null
36+
├── v int
37+
└── INDEX primary
38+
└── u int not null
39+
2240
# --------------------------------------------------
2341
# EnsureJoinFiltersAnd
2442
# --------------------------------------------------
@@ -939,6 +957,113 @@ inner-join
939957
└── filters [type=bool, outer=(1,3), constraints=(/1: (/NULL - ]; /3: (/NULL - ]), fd=(1)==(3), (3)==(1)]
940958
└── a = b [type=bool, outer=(1,3), constraints=(/1: (/NULL - ]; /3: (/NULL - ])]
941959

960+
# Regression for issue 28818. Try to trigger undetectable cycle between the
961+
# PushFilterIntoJoinLeftAndRight and TryDecorrelateSelect rules.
962+
opt
963+
SELECT 1
964+
FROM a
965+
WHERE EXISTS (
966+
SELECT 1
967+
FROM xy
968+
INNER JOIN uv
969+
ON EXISTS (
970+
SELECT 1
971+
FROM b
972+
WHERE a.s >= 'foo'
973+
LIMIT 10
974+
)
975+
WHERE
976+
(SELECT s FROM a) = 'foo'
977+
)
978+
----
979+
project
980+
├── columns: "?column?":22(int!null)
981+
├── fd: ()-->(22)
982+
├── distinct-on
983+
│ ├── columns: a.k:1(int!null)
984+
│ ├── grouping columns: a.k:1(int!null)
985+
│ ├── key: (1)
986+
│ └── select
987+
│ ├── columns: a.k:1(int!null) xy.x:6(int!null) u:8(int!null) true_agg:14(bool!null)
988+
│ ├── key: (1,6,8)
989+
│ ├── fd: (1,6,8)-->(14)
990+
│ ├── group-by
991+
│ │ ├── columns: a.k:1(int!null) xy.x:6(int!null) u:8(int!null) true_agg:14(bool)
992+
│ │ ├── grouping columns: a.k:1(int!null) xy.x:6(int!null) u:8(int!null)
993+
│ │ ├── key: (1,6,8)
994+
│ │ ├── fd: (1,6,8)-->(14)
995+
│ │ ├── project
996+
│ │ │ ├── columns: true:13(bool!null) a.k:1(int!null) xy.x:6(int!null) u:8(int!null)
997+
│ │ │ ├── fd: ()-->(13)
998+
│ │ │ ├── inner-join-apply
999+
│ │ │ │ ├── columns: a.k:1(int!null) a.s:4(string) xy.x:6(int!null) u:8(int!null)
1000+
│ │ │ │ ├── fd: (1)-->(4)
1001+
│ │ │ │ ├── scan a
1002+
│ │ │ │ │ ├── columns: a.k:1(int!null) a.s:4(string)
1003+
│ │ │ │ │ ├── key: (1)
1004+
│ │ │ │ │ └── fd: (1)-->(4)
1005+
│ │ │ │ ├── inner-join
1006+
│ │ │ │ │ ├── columns: xy.x:6(int!null) u:8(int!null)
1007+
│ │ │ │ │ ├── outer: (4)
1008+
│ │ │ │ │ ├── inner-join
1009+
│ │ │ │ │ │ ├── columns: xy.x:6(int!null) u:8(int!null)
1010+
│ │ │ │ │ │ ├── key: (6,8)
1011+
│ │ │ │ │ │ ├── select
1012+
│ │ │ │ │ │ │ ├── columns: xy.x:6(int!null)
1013+
│ │ │ │ │ │ │ ├── key: (6)
1014+
│ │ │ │ │ │ │ ├── scan xy
1015+
│ │ │ │ │ │ │ │ ├── columns: xy.x:6(int!null)
1016+
│ │ │ │ │ │ │ │ └── key: (6)
1017+
│ │ │ │ │ │ │ └── filters [type=bool]
1018+
│ │ │ │ │ │ │ └── eq [type=bool]
1019+
│ │ │ │ │ │ │ ├── subquery [type=string]
1020+
│ │ │ │ │ │ │ │ └── max1-row
1021+
│ │ │ │ │ │ │ │ ├── columns: a.s:19(string)
1022+
│ │ │ │ │ │ │ │ ├── cardinality: [0 - 1]
1023+
│ │ │ │ │ │ │ │ ├── key: ()
1024+
│ │ │ │ │ │ │ │ ├── fd: ()-->(19)
1025+
│ │ │ │ │ │ │ │ └── scan a
1026+
│ │ │ │ │ │ │ │ └── columns: a.s:19(string)
1027+
│ │ │ │ │ │ │ └── const: 'foo' [type=string]
1028+
│ │ │ │ │ │ ├── select
1029+
│ │ │ │ │ │ │ ├── columns: u:8(int!null)
1030+
│ │ │ │ │ │ │ ├── key: (8)
1031+
│ │ │ │ │ │ │ ├── scan uv
1032+
│ │ │ │ │ │ │ │ ├── columns: u:8(int!null)
1033+
│ │ │ │ │ │ │ │ └── key: (8)
1034+
│ │ │ │ │ │ │ └── filters [type=bool]
1035+
│ │ │ │ │ │ │ └── eq [type=bool]
1036+
│ │ │ │ │ │ │ ├── subquery [type=string]
1037+
│ │ │ │ │ │ │ │ └── max1-row
1038+
│ │ │ │ │ │ │ │ ├── columns: a.s:19(string)
1039+
│ │ │ │ │ │ │ │ ├── cardinality: [0 - 1]
1040+
│ │ │ │ │ │ │ │ ├── key: ()
1041+
│ │ │ │ │ │ │ │ ├── fd: ()-->(19)
1042+
│ │ │ │ │ │ │ │ └── scan a
1043+
│ │ │ │ │ │ │ │ └── columns: a.s:19(string)
1044+
│ │ │ │ │ │ │ └── const: 'foo' [type=string]
1045+
│ │ │ │ │ │ └── true [type=bool]
1046+
│ │ │ │ │ ├── limit
1047+
│ │ │ │ │ │ ├── outer: (4)
1048+
│ │ │ │ │ │ ├── cardinality: [0 - 10]
1049+
│ │ │ │ │ │ ├── select
1050+
│ │ │ │ │ │ │ ├── outer: (4)
1051+
│ │ │ │ │ │ │ ├── scan b
1052+
│ │ │ │ │ │ │ └── filters [type=bool, outer=(4), constraints=(/4: [/'foo' - ]; tight)]
1053+
│ │ │ │ │ │ │ └── a.s >= 'foo' [type=bool, outer=(4), constraints=(/4: [/'foo' - ]; tight)]
1054+
│ │ │ │ │ │ └── const: 10 [type=int]
1055+
│ │ │ │ │ └── true [type=bool]
1056+
│ │ │ │ └── true [type=bool]
1057+
│ │ │ └── projections [outer=(1,6,8)]
1058+
│ │ │ └── true [type=bool]
1059+
│ │ └── aggregations [outer=(13)]
1060+
│ │ └── const-not-null-agg [type=bool, outer=(13)]
1061+
│ │ └── variable: true [type=bool, outer=(13)]
1062+
│ └── filters [type=bool, outer=(14), constraints=(/14: (/NULL - ]; tight)]
1063+
│ └── true_agg IS NOT NULL [type=bool, outer=(14), constraints=(/14: (/NULL - ]; tight)]
1064+
└── projections
1065+
└── const: 1 [type=int]
1066+
9421067
# --------------------------------------------------
9431068
# PushFilterIntoJoinLeft + PushFilterIntoJoinRight
9441069
# --------------------------------------------------

0 commit comments

Comments
 (0)