Skip to content

Commit

Permalink
opt: do not generate unnecessary cross-joins on lookup join input
Browse files Browse the repository at this point in the history
This commit fixes a bug that caused unnecessary cross-joins on the input
of lookup joins, causing both suboptimal query plans and incorrect query
results. The bug only affected lookup joins with lookup expressions.

Fixes #79384

Release note (bug fix): A bug has been fixed that caused the optimizer
to generate query plans with logically incorrect lookup joins. The bug
can only occur in queries with an inner join, e.g., `t1 JOIN t2`, if all
of the following are true:
  1. The join contains an equality condition between columns of both
     tables, e.g., `t1.a = t2.a`.
  2. A query filter or `CHECK` constraint constrains a column to a set
     of specific values, e.g., `t2.b IN (1, 2, 3)`. In the case of a
     `CHECK` constraint, the column must be `NOT NULL`.
  3. A query filter or `CHECK` constraint constrains a column to a
     range, e.g., `t2.c > 0`. In the case of a `CHECK` constraint, the
     column must be `NOT NULL`.
  4. An index contains a column from each of the criteria above, e.g.,
     `INDEX t2(a, b, c)`.
This bug has been present since version 21.2.0.
  • Loading branch information
mgartner committed Apr 5, 2022
1 parent bb2c29c commit 3bf035f
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 135 deletions.
28 changes: 28 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/lookup_join
Original file line number Diff line number Diff line change
Expand Up @@ -665,3 +665,31 @@ SELECT * FROM (VALUES (1, 10), (2, 20), (3, NULL)) AS u(w, x) WHERE NOT EXISTS (
)
----
3 NULL

# Regression test for #79384. Do not generate unnecessary cross-joins on a
# lookup join's input when the lookup join uses a lookup expression.
statement ok
CREATE TABLE t79384a (
k INT NOT NULL
)

statement ok
CREATE TABLE t79384b (
a INT,
b INT,
c INT,
INDEX (a, b, c)
)

statement ok
INSERT INTO t79384a VALUES (1)

statement ok
INSERT INTO t79384b VALUES (1, 1, 1)

# The joined tables have a single row each, so this query should never return
# more than one row.
query I
SELECT k FROM t79384a INNER LOOKUP JOIN t79384b ON k = a AND b IN (1, 2, 3) AND c > 0
----
1
1 change: 0 additions & 1 deletion pkg/sql/opt/exec/execbuilder/testdata/inner-join
Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,3 @@ vectorized: true
missing stats
table: def@def_pkey
spans: FULL SCAN

12 changes: 5 additions & 7 deletions pkg/sql/opt/exec/execbuilder/testdata/lookup_join_spans
Original file line number Diff line number Diff line change
Expand Up @@ -837,16 +837,14 @@ vectorized: true
│ equality cols are key
└── • lookup join
│ estimated row count: 0
│ table: metric_values@secondary
│ lookup condition: ((metric_id = id) AND (nullable = 1)) AND ("time" < '2020-01-01 00:00:10+00:00')
└── • render
│ estimated row count: 1
└── • scan
estimated row count: 1 (10% of the table; stats collected <hidden> ago)
table: metrics@name_index
spans: [/'cpu' - /'cpu']
└── • scan
estimated row count: 1 (10% of the table; stats collected <hidden> ago)
table: metrics@name_index
spans: [/'cpu' - /'cpu']


# Regression test for issue #68200. This ensures that we properly construct the
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/opt/xform/join_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,9 @@ func (c *CustomFuncs) generateLookupJoinsImpl(

// Reset KeyCols since we're not using it anymore.
lookupJoin.KeyCols = opt.ColList{}
// Reset input since we don't need any constant values that may have
// been joined on the input above.
lookupJoin.Input = input
}

if len(lookupJoin.KeyCols) == 0 && len(lookupJoin.LookupExpr) == 0 {
Expand Down
77 changes: 33 additions & 44 deletions pkg/sql/opt/xform/testdata/external/tpce
Original file line number Diff line number Diff line change
Expand Up @@ -4029,34 +4029,31 @@ project
│ │ ├── fd: ()-->(1,8,27,30,31,45-47), (48)-->(49,50), (31)==(47), (47)==(31), (8)==(45), (45)==(8)
│ │ ├── limit hint: 1.00
│ │ ├── inner-join (lookup commission_rate)
│ │ │ ├── columns: c_id:1!null c_tier:8!null cr_c_tier:45!null cr_tt_id:46!null cr_ex_id:47!null cr_from_qty:48!null cr_to_qty:49!null commission_rate.cr_rate:50!null
│ │ │ ├── key columns: [8 57] = [45 46]
│ │ │ ├── key: (47,48)
│ │ │ ├── fd: ()-->(1,8,45,46), (47,48)-->(49,50), (8)==(45), (45)==(8)
│ │ │ ├── project
│ │ │ │ ├── columns: "lookup_join_const_col_@46":57!null c_id:1!null c_tier:8!null
│ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null cr_c_tier:45!null cr_tt_id:46!null cr_ex_id:47!null cr_from_qty:48!null cr_to_qty:49!null commission_rate.cr_rate:50!null
│ │ │ ├── lookup expression
│ │ │ │ └── filters
│ │ │ │ ├── cr_ex_id:47 = s_ex_id:31 [outer=(31,47), constraints=(/31: (/NULL - ]; /47: (/NULL - ]), fd=(31)==(47), (47)==(31)]
│ │ │ │ ├── cr_c_tier:45 IN (1, 2, 3) [outer=(45), constraints=(/45: [/1 - /1] [/2 - /2] [/3 - /3]; tight)]
│ │ │ │ ├── cr_tt_id:46 = 'TLS' [outer=(46), constraints=(/46: [/'TLS' - /'TLS']; tight), fd=()-->(46)]
│ │ │ │ └── cr_from_qty:48 <= 100 [outer=(48), constraints=(/48: (/NULL - /100]; tight)]
│ │ │ ├── key: (45,48)
│ │ │ ├── fd: ()-->(27,30,31,46,47), (45,48)-->(49,50), (31)==(47), (47)==(31)
│ │ │ ├── scan security
│ │ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ ├── key: ()
│ │ │ │ ├── fd: ()-->(1,8,57)
│ │ │ │ ├── scan customer
│ │ │ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(1,8)
│ │ │ │ └── projections
│ │ │ │ └── 'TLS' [as="lookup_join_const_col_@46":57]
│ │ │ │ └── fd: ()-->(27,30,31)
│ │ │ └── filters
│ │ │ ├── cr_from_qty:48 <= 100 [outer=(48), constraints=(/48: (/NULL - /100]; tight)]
│ │ │ └── cr_to_qty:49 >= 200 [outer=(49), constraints=(/49: [/200 - ]; tight)]
│ │ ├── scan security
│ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ ├── scan customer
│ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ ├── cardinality: [0 - 1]
│ │ │ ├── key: ()
│ │ │ └── fd: ()-->(27,30,31)
│ │ │ └── fd: ()-->(1,8)
│ │ └── filters
│ │ └── cr_ex_id:47 = s_ex_id:31 [outer=(31,47), constraints=(/31: (/NULL - ]; /47: (/NULL - ]), fd=(31)==(47), (47)==(31)]
│ │ └── cr_c_tier:45 = c_tier:8 [outer=(8,45), constraints=(/8: (/NULL - ]; /45: (/NULL - ]), fd=(8)==(45), (45)==(8)]
│ └── 1
└── projections
└── commission_rate.cr_rate:50::FLOAT8 [as=cr_rate:53, outer=(50), immutable]
Expand Down Expand Up @@ -4097,34 +4094,26 @@ project
│ │ ├── key: (48)
│ │ ├── fd: ()-->(1,8,27,30,31,45-47), (48)-->(49,50), (8)==(45), (45)==(8), (31)==(47), (47)==(31)
│ │ ├── limit hint: 1.00
│ │ ├── project
│ │ │ ├── columns: "lookup_join_const_col_@46":54!null c_id:1!null c_tier:8!null s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ ├── inner-join (cross)
│ │ │ ├── columns: c_id:1!null c_tier:8!null s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ ├── cardinality: [0 - 1]
│ │ │ ├── multiplicity: left-rows(zero-or-one), right-rows(zero-or-one)
│ │ │ ├── key: ()
│ │ │ ├── fd: ()-->(1,8,27,30,31,54)
│ │ │ ├── fd: ()-->(1,8,27,30,31)
│ │ │ ├── limit hint: 1.00
│ │ │ ├── inner-join (cross)
│ │ │ │ ├── columns: c_id:1!null c_tier:8!null s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ ├── scan customer
│ │ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ ├── multiplicity: left-rows(zero-or-one), right-rows(zero-or-one)
│ │ │ │ ├── key: ()
│ │ │ │ ├── fd: ()-->(1,8,27,30,31)
│ │ │ │ ├── limit hint: 1.00
│ │ │ │ ├── scan customer
│ │ │ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(1,8)
│ │ │ │ ├── scan security
│ │ │ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(27,30,31)
│ │ │ │ └── filters (true)
│ │ │ └── projections
│ │ │ └── 'TLS' [as="lookup_join_const_col_@46":54]
│ │ │ │ └── fd: ()-->(1,8)
│ │ │ ├── scan security
│ │ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ ├── key: ()
│ │ │ │ └── fd: ()-->(27,30,31)
│ │ │ └── filters (true)
│ │ └── filters
│ │ └── cr_to_qty:49 >= 200 [outer=(49), constraints=(/49: [/200 - ]; tight)]
│ └── 1
Expand Down
76 changes: 33 additions & 43 deletions pkg/sql/opt/xform/testdata/external/tpce-no-stats
Original file line number Diff line number Diff line change
Expand Up @@ -4060,34 +4060,31 @@ project
│ │ ├── fd: ()-->(1,8,27,30,31,45-47), (48)-->(49,50), (31)==(47), (47)==(31), (8)==(45), (45)==(8)
│ │ ├── limit hint: 1.00
│ │ ├── inner-join (lookup commission_rate)
│ │ │ ├── columns: c_id:1!null c_tier:8!null cr_c_tier:45!null cr_tt_id:46!null cr_ex_id:47!null cr_from_qty:48!null cr_to_qty:49!null commission_rate.cr_rate:50!null
│ │ │ ├── key columns: [8 57] = [45 46]
│ │ │ ├── key: (47,48)
│ │ │ ├── fd: ()-->(1,8,45,46), (47,48)-->(49,50), (8)==(45), (45)==(8)
│ │ │ ├── project
│ │ │ │ ├── columns: "lookup_join_const_col_@46":57!null c_id:1!null c_tier:8!null
│ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null cr_c_tier:45!null cr_tt_id:46!null cr_ex_id:47!null cr_from_qty:48!null cr_to_qty:49!null commission_rate.cr_rate:50!null
│ │ │ ├── lookup expression
│ │ │ │ └── filters
│ │ │ │ ├── cr_ex_id:47 = s_ex_id:31 [outer=(31,47), constraints=(/31: (/NULL - ]; /47: (/NULL - ]), fd=(31)==(47), (47)==(31)]
│ │ │ │ ├── cr_c_tier:45 IN (1, 2, 3) [outer=(45), constraints=(/45: [/1 - /1] [/2 - /2] [/3 - /3]; tight)]
│ │ │ │ ├── cr_tt_id:46 = 'TLS' [outer=(46), constraints=(/46: [/'TLS' - /'TLS']; tight), fd=()-->(46)]
│ │ │ │ └── cr_from_qty:48 <= 100 [outer=(48), constraints=(/48: (/NULL - /100]; tight)]
│ │ │ ├── key: (45,48)
│ │ │ ├── fd: ()-->(27,30,31,46,47), (45,48)-->(49,50), (31)==(47), (47)==(31)
│ │ │ ├── scan security
│ │ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ ├── key: ()
│ │ │ │ ├── fd: ()-->(1,8,57)
│ │ │ │ ├── scan customer
│ │ │ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(1,8)
│ │ │ │ └── projections
│ │ │ │ └── 'TLS' [as="lookup_join_const_col_@46":57]
│ │ │ │ └── fd: ()-->(27,30,31)
│ │ │ └── filters
│ │ │ ├── cr_from_qty:48 <= 100 [outer=(48), constraints=(/48: (/NULL - /100]; tight)]
│ │ │ └── cr_to_qty:49 >= 200 [outer=(49), constraints=(/49: [/200 - ]; tight)]
│ │ ├── scan security
│ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ ├── scan customer
│ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ ├── cardinality: [0 - 1]
│ │ │ ├── key: ()
│ │ │ └── fd: ()-->(27,30,31)
│ │ │ └── fd: ()-->(1,8)
│ │ └── filters
│ │ └── cr_ex_id:47 = s_ex_id:31 [outer=(31,47), constraints=(/31: (/NULL - ]; /47: (/NULL - ]), fd=(31)==(47), (47)==(31)]
│ │ └── cr_c_tier:45 = c_tier:8 [outer=(8,45), constraints=(/8: (/NULL - ]; /45: (/NULL - ]), fd=(8)==(45), (45)==(8)]
│ └── 1
└── projections
└── commission_rate.cr_rate:50::FLOAT8 [as=cr_rate:53, outer=(50), immutable]
Expand Down Expand Up @@ -4128,32 +4125,25 @@ project
│ │ ├── key: (48)
│ │ ├── fd: ()-->(1,8,27,30,31,45-47), (48)-->(49,50), (8)==(45), (45)==(8), (31)==(47), (47)==(31)
│ │ ├── limit hint: 1.00
│ │ ├── project
│ │ │ ├── columns: "lookup_join_const_col_@46":54!null c_id:1!null c_tier:8!null s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ ├── inner-join (cross)
│ │ │ ├── columns: c_id:1!null c_tier:8!null s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ ├── cardinality: [0 - 1]
│ │ │ ├── multiplicity: left-rows(zero-or-one), right-rows(zero-or-one)
│ │ │ ├── key: ()
│ │ │ ├── fd: ()-->(1,8,27,30,31,54)
│ │ │ ├── inner-join (cross)
│ │ │ │ ├── columns: c_id:1!null c_tier:8!null s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ ├── fd: ()-->(1,8,27,30,31)
│ │ │ ├── scan customer
│ │ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ ├── multiplicity: left-rows(zero-or-one), right-rows(zero-or-one)
│ │ │ │ ├── key: ()
│ │ │ │ ├── fd: ()-->(1,8,27,30,31)
│ │ │ │ ├── scan customer
│ │ │ │ │ ├── columns: c_id:1!null c_tier:8!null
│ │ │ │ │ ├── constraint: /1: [/0 - /0]
│ │ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(1,8)
│ │ │ │ ├── scan security
│ │ │ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(27,30,31)
│ │ │ │ └── filters (true)
│ │ │ └── projections
│ │ │ └── 'TLS' [as="lookup_join_const_col_@46":54]
│ │ │ │ └── fd: ()-->(1,8)
│ │ │ ├── scan security
│ │ │ │ ├── columns: s_symb:27!null s_name:30!null s_ex_id:31!null
│ │ │ │ ├── constraint: /27: [/'ROACH' - /'ROACH']
│ │ │ │ ├── cardinality: [0 - 1]
│ │ │ │ ├── key: ()
│ │ │ │ └── fd: ()-->(27,30,31)
│ │ │ └── filters (true)
│ │ └── filters
│ │ └── cr_to_qty:49 >= 200 [outer=(49), constraints=(/49: [/200 - ]; tight)]
│ └── 1
Expand Down
Loading

0 comments on commit 3bf035f

Please sign in to comment.