Skip to content

Commit d8eb072

Browse files
committed
opt: fix optional filter removal for lookup join
Fixes #159660 Previously, we don't reset the `optionalMultiValFilterSuffixLen` when we find a necessary multi-val filter for a column. This would cause an incorrect removal of an unoptional filter. For example, - Index: (x, y ,z) - ON filters: x = 1, z IN (1, 2) - Optional filters: y IN (3, 4) Since the filters are processed with the order of columns in the index, the filters will be processed in the order of `x = 1`, `y IN (3, 4)`, `z IN (1, 2)`. When the processing reaches `y IN (3, 4)`, since it is an optional filter, `optionalMultiValFilterSuffixLen = 1`. But `z IN (1, 2)` is actually a necessary filter, with `optionalMultiValFilterSuffixLen` not being reset, it will be eventually wrongly removed. Release note (bug fix): Fixed a bug causing a query predicate to be ignored when the predicate is on a column following one or more ENUM columns in an index, the predicate constrains the column to multiple values, and a lookup join to the index is chosen for the query plan. This bug was introduced in 24.3.0 and present in all versions since.
1 parent e29fd33 commit d8eb072

File tree

4 files changed

+47
-0
lines changed

4 files changed

+47
-0
lines changed

pkg/sql/opt/lookupjoin/constraint_builder.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ func (b *ConstraintBuilder) Build(
348348
if isOptional := allIdx >= len(onFilters); isOptional {
349349
optionalMultiValFilterSuffixLen++
350350
} else {
351+
optionalMultiValFilterSuffixLen = 0
351352
// There's no need to track optional filters for reducing the
352353
// remaining filters because they are not present in the ON
353354
// filters to begin with.

pkg/sql/opt/lookupjoin/testdata/key_cols

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,13 @@ optional: z IN (3, 4) AND y IN (10, 20)
191191
key cols:
192192
x = a
193193

194+
lookup-constraints left=(a int, b int, c int) right=(x int, y int, z int) index=(x, y, z)
195+
x = a AND z IN (3, 4)
196+
optional: y IN (10, 20)
197+
----
198+
lookup expression:
199+
((y IN (10, 20)) AND (z IN (3, 4))) AND (a = x)
200+
194201
lookup-constraints left=(a int, b int) right=(x int, y int, z int) index=(x, y, z)
195202
x = a AND y = b
196203
optional: z > 10

pkg/sql/opt/lookupjoin/testdata/lookup_expr

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,3 +489,10 @@ optional: y IN (10, 20, 30)
489489
----
490490
lookup expression:
491491
((y IN (10, 20, 30)) AND (z > b)) AND (a = x)
492+
493+
lookup-constraints left=(a int, b int, c int) right=(x int, y int, z int) index=(x, y, z)
494+
x = a AND z IN (100, 200, 300)
495+
optional: y IN (10, 20, 30)
496+
----
497+
lookup expression:
498+
((y IN (10, 20, 30)) AND (z IN (100, 200, 300))) AND (a = x)

pkg/sql/opt/xform/testdata/rules/join

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3123,6 +3123,38 @@ left-join (lookup lookup_expr [as=t])
31233123
│ └── column1:1 = w:7 [outer=(1,7), constraints=(/1: (/NULL - ]; /7: (/NULL - ]), fd=(1)==(7), (7)==(1)]
31243124
└── filters (true)
31253125

3126+
# The filters in the lookup expression should contains all columns of the
3127+
# idx_vrw index. Column r, though not in the ON exprs, should still be
3128+
# in the filters, because for the column w that comes behind it in
3129+
# idx_vrw, there is a necessary filter `w IN (10, 20)`.
3130+
opt expect=GenerateLookupJoinsWithFilter
3131+
SELECT * FROM (VALUES (1), (2), (3)) AS q(v) LEFT LOOKUP JOIN lookup_expr t
3132+
ON q.v = t.v WHERE w IN (10, 20)
3133+
----
3134+
inner-join (lookup lookup_expr [as=t])
3135+
├── columns: v:1!null r:2!null k:3!null u:4 v:5!null w:6!null x:7 y:8!null z:9!null
3136+
├── key columns: [2 3] = [2 3]
3137+
├── lookup columns are key
3138+
├── fd: ()-->(9), (2,3)-->(4-8), (1)==(5), (5)==(1)
3139+
├── inner-join (lookup lookup_expr@idx_vrw [as=t])
3140+
│ ├── columns: column1:1!null r:2!null k:3!null v:5!null w:6!null
3141+
│ ├── flags: force lookup join (into right side)
3142+
│ ├── lookup expression
3143+
│ │ └── filters
3144+
│ │ ├── r:2 IN ('east', 'west') [outer=(2), constraints=(/2: [/'east' - /'east'] [/'west' - /'west']; tight)]
3145+
│ │ ├── w:6 IN (10, 20) [outer=(6), constraints=(/6: [/10 - /10] [/20 - /20]; tight)]
3146+
│ │ └── column1:1 = v:5 [outer=(1,5), constraints=(/1: (/NULL - ]; /5: (/NULL - ]), fd=(1)==(5), (5)==(1)]
3147+
│ ├── fd: (2,3)-->(5,6), (1)==(5), (5)==(1)
3148+
│ ├── values
3149+
│ │ ├── columns: column1:1!null
3150+
│ │ ├── cardinality: [3 - 3]
3151+
│ │ ├── (1,)
3152+
│ │ ├── (2,)
3153+
│ │ └── (3,)
3154+
│ └── filters (true)
3155+
└── filters (true)
3156+
3157+
31263158
# Ensure that we constrain all columns in idx_vrw, not just v.
31273159
opt expect=GenerateLookupJoins
31283160
SELECT * FROM (VALUES (1, 10), (2, 20), (3, NULL)) AS q(v, w) LEFT JOIN lookup_expr t

0 commit comments

Comments
 (0)