From e009a266ae434d7a786ad25a248ced116f95a2e3 Mon Sep 17 00:00:00 2001 From: Rebecca Taft Date: Tue, 9 Aug 2022 14:15:14 -0500 Subject: [PATCH] opt: respect NO_INDEX_JOIN flag Prior to this commit, it was possible that the optimizer could produce a plan with an index join even if the user hinted that index joins should be avoided by using the NO_INDEX_JOIN hint. This commit fixes that oversight, and we no longer plan an index join in this case. This commit also adds assertions that an index join is not planned if NO_INDEX_JOIN is used to prevent this bug from recurring. Fixes #85841 Release note (bug fix): Fixed an issue where the NO_INDEX_JOIN hint could be ignored by the optimizer in some cases, causing it to create a query plan with an index join. --- pkg/sql/opt/memo/check_expr.go | 3 ++ pkg/sql/opt/xform/index_scan_builder.go | 3 ++ pkg/sql/opt/xform/limit_funcs.go | 2 +- pkg/sql/opt/xform/select_funcs.go | 10 ++++++ pkg/sql/opt/xform/testdata/rules/select | 47 +++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) diff --git a/pkg/sql/opt/memo/check_expr.go b/pkg/sql/opt/memo/check_expr.go index 07f8685604b9..213a4d541cf6 100644 --- a/pkg/sql/opt/memo/check_expr.go +++ b/pkg/sql/opt/memo/check_expr.go @@ -204,6 +204,9 @@ func (m *Memo) CheckExpr(e opt.Expr) { if t.Cols.Empty() { panic(errors.AssertionFailedf("index join with no columns")) } + if scan, ok := t.Input.(*ScanExpr); ok && scan.Flags.NoIndexJoin { + panic(errors.AssertionFailedf("index join used with NoIndexJoin flag")) + } case *LookupJoinExpr: if len(t.KeyCols) == 0 && len(t.LookupExpr) == 0 { diff --git a/pkg/sql/opt/xform/index_scan_builder.go b/pkg/sql/opt/xform/index_scan_builder.go index d23a1633bff8..85e36debbe2d 100644 --- a/pkg/sql/opt/xform/index_scan_builder.go +++ b/pkg/sql/opt/xform/index_scan_builder.go @@ -163,6 +163,9 @@ func (b *indexScanBuilder) AddSelectAfterSplit( // AddIndexJoin wraps the input expression with an IndexJoin expression that // produces the given set of columns by lookup in the primary index. func (b *indexScanBuilder) AddIndexJoin(cols opt.ColSet) { + if b.scanPrivate.Flags.NoIndexJoin { + panic(errors.AssertionFailedf("attempt to create an index join with NoIndexJoin flag")) + } if b.hasIndexJoin() { panic(errors.AssertionFailedf("cannot call AddIndexJoin twice")) } diff --git a/pkg/sql/opt/xform/limit_funcs.go b/pkg/sql/opt/xform/limit_funcs.go index 9e0e537d6b3d..8c0f001c3678 100644 --- a/pkg/sql/opt/xform/limit_funcs.go +++ b/pkg/sql/opt/xform/limit_funcs.go @@ -68,7 +68,7 @@ func (c *CustomFuncs) CanLimitFilteredScan( if scanPrivate.IsVirtualTable(md) && !required.Any() { return false } - ok, _ := ordering.ScanPrivateCanProvide(c.e.mem.Metadata(), scanPrivate, &required) + ok, _ := ordering.ScanPrivateCanProvide(md, scanPrivate, &required) return ok } diff --git a/pkg/sql/opt/xform/select_funcs.go b/pkg/sql/opt/xform/select_funcs.go index d131326fa96b..1b44a180687f 100644 --- a/pkg/sql/opt/xform/select_funcs.go +++ b/pkg/sql/opt/xform/select_funcs.go @@ -117,6 +117,12 @@ func (c *CustomFuncs) GeneratePartialIndexScans( return } + // Otherwise, try to construct an IndexJoin operator that provides the + // columns missing from the index. + if scanPrivate.Flags.NoIndexJoin { + return + } + // Calculate the PK columns once. if pkCols.Empty() { pkCols = c.PrimaryKeyCols(scanPrivate.Table) @@ -790,6 +796,10 @@ func (c *CustomFuncs) GenerateInvertedIndexScans( newScanPrivate.SetConstraint(c.e.evalCtx, constraint) newScanPrivate.InvertedConstraint = spansToRead + if scanPrivate.Flags.NoIndexJoin { + return + } + // Calculate the PK columns once. if pkCols.Empty() { pkCols = c.PrimaryKeyCols(scanPrivate.Table) diff --git a/pkg/sql/opt/xform/testdata/rules/select b/pkg/sql/opt/xform/testdata/rules/select index 4f1ca68fcced..08b23239d59f 100644 --- a/pkg/sql/opt/xform/testdata/rules/select +++ b/pkg/sql/opt/xform/testdata/rules/select @@ -306,6 +306,29 @@ memo (optimized, ~14KB, required=[presentation: k:1,i:2,f:3,s:4,b:5]) ├── G16: (variable s) └── G17: (const 'foo') +# GeneratePartialIndexScans will be triggered, but not add an index +# scan to the memo since NO_INDEX_JOIN is specified. +memo expect-not=GeneratePartialIndexScans +SELECT * FROM p@{NO_INDEX_JOIN} WHERE i > 0 AND s = 'foo' +---- +memo (optimized, ~7KB, required=[presentation: k:1,i:2,f:3,s:4,b:5]) + ├── G1: (select G2 G3) + │ └── [presentation: k:1,i:2,f:3,s:4,b:5] + │ ├── best: (select G2 G3) + │ └── cost: 1135.06 + ├── G2: (scan p,cols=(1-5)) + │ └── [] + │ ├── best: (scan p,cols=(1-5)) + │ └── cost: 1125.02 + ├── G3: (filters G4 G5) + ├── G4: (gt G6 G7) + ├── G5: (eq G8 G9) + ├── G6: (variable i) + ├── G7: (const 0) + ├── G8: (variable s) + └── G9: (const 'foo') + + # Do not generate a partial index scan when the predicate is not implied by the # filter. memo expect-not=GeneratePartialIndexScans @@ -2185,6 +2208,30 @@ memo (optimized, ~7KB, required=[presentation: k:1]) ├── G8: (variable j) └── G9: (const '{"a": "b"}') +# GenerateInvertedIndexScans will be triggered, but not add an index +# scan to the memo since NO_INDEX_JOIN is specified. +memo expect-not=GenerateInvertedIndexScans +SELECT k FROM b@{NO_INDEX_JOIN} WHERE j @> '{"a": "b"}' +---- +memo (optimized, ~5KB, required=[presentation: k:1]) + ├── G1: (project G2 G3 k) + │ └── [presentation: k:1] + │ ├── best: (project G2 G3 k) + │ └── cost: 1095.77 + ├── G2: (select G4 G5) + │ └── [] + │ ├── best: (select G4 G5) + │ └── cost: 1094.65 + ├── G3: (projections) + ├── G4: (scan b,cols=(1,4)) + │ └── [] + │ ├── best: (scan b,cols=(1,4)) + │ └── cost: 1084.62 + ├── G5: (filters G6) + ├── G6: (contains G7 G8) + ├── G7: (variable j) + └── G8: (const '{"a": "b"}') + # Query requiring an index join with no remaining filter. opt expect=GenerateInvertedIndexScans SELECT u, k FROM b WHERE j @> '{"a": "b"}'