Skip to content

Commit

Permalink
opt: Fix rule cycle bug
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@andy-kimball
andy-kimball committed Aug 20, 2018
1 parent 6c463b9 commit c7772ac
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 6 deletions.
13 changes: 7 additions & 6 deletions pkg/sql/opt/norm/rules/join.opt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:[
...
Expand Down
125 changes: 125 additions & 0 deletions pkg/sql/opt/norm/testdata/rules/join
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
# --------------------------------------------------
Expand Down Expand Up @@ -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
# --------------------------------------------------
Expand Down

0 comments on commit c7772ac

Please sign in to comment.