From 6a3e6890e83431f178dcda82506a0b7512c53e8d Mon Sep 17 00:00:00 2001 From: Marcus Gartner Date: Tue, 2 Feb 2021 16:58:42 -0800 Subject: [PATCH] opt: fix panic in GenerateLookupJoin In #57690 a new code path was introduced from `findConstantFilterCols` from `GenerateLookupJoins`. This new code path made it possible for the filters passed to `findConstantFilterCols` to contain columns that are not part of the given table. This violated the assumption that the filter only contains columns in the given table and caused a panic. This commit fixes the issue by neglecting constant filters for columns not in the given table. Fixes #59738 Release note (bug fix): A bug has been fixed that caused errors when joining two tables when one of the tables had a computed column. This bug was present since version 21.1.0-alpha.2 and not present in any production releases. --- pkg/sql/opt/xform/general_funcs.go | 8 ++++---- pkg/sql/opt/xform/join_funcs.go | 4 ++-- pkg/sql/opt/xform/select_funcs.go | 19 ++++++++++++------ pkg/sql/opt/xform/testdata/rules/join | 29 +++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/pkg/sql/opt/xform/general_funcs.go b/pkg/sql/opt/xform/general_funcs.go index 30835d704377..7a569c397fde 100644 --- a/pkg/sql/opt/xform/general_funcs.go +++ b/pkg/sql/opt/xform/general_funcs.go @@ -229,9 +229,9 @@ func (c *CustomFuncs) initIdxConstraintForIndex( // The values of both columns in that index are known, enabling a single value // constraint to be generated. func (c *CustomFuncs) computedColFilters( - tabID opt.TableID, requiredFilters, optionalFilters memo.FiltersExpr, + scanPrivate *memo.ScanPrivate, requiredFilters, optionalFilters memo.FiltersExpr, ) memo.FiltersExpr { - tabMeta := c.e.mem.Metadata().TableMeta(tabID) + tabMeta := c.e.mem.Metadata().TableMeta(scanPrivate.Table) if len(tabMeta.ComputedCols) == 0 { return nil } @@ -239,8 +239,8 @@ func (c *CustomFuncs) computedColFilters( // Start with set of constant columns, as derived from the list of filter // conditions. constCols := make(map[opt.ColumnID]opt.ScalarExpr) - c.findConstantFilterCols(constCols, tabID, requiredFilters) - c.findConstantFilterCols(constCols, tabID, optionalFilters) + c.findConstantFilterCols(constCols, scanPrivate, requiredFilters) + c.findConstantFilterCols(constCols, scanPrivate, optionalFilters) if len(constCols) == 0 { // No constant values could be derived from filters, so assume that there // are also no constant computed columns. diff --git a/pkg/sql/opt/xform/join_funcs.go b/pkg/sql/opt/xform/join_funcs.go index 512644fa23f5..8eb8aa9253bf 100644 --- a/pkg/sql/opt/xform/join_funcs.go +++ b/pkg/sql/opt/xform/join_funcs.go @@ -224,7 +224,7 @@ func (c *CustomFuncs) GenerateLookupJoins( // Generate implicit filters from CHECK constraints and computed columns as // optional filters to help generate lookup join keys. optionalFilters := c.checkConstraintFilters(scanPrivate.Table) - computedColFilters := c.computedColFilters(scanPrivate.Table, on, optionalFilters) + computedColFilters := c.computedColFilters(scanPrivate, on, optionalFilters) optionalFilters = append(optionalFilters, computedColFilters...) var pkCols opt.ColList @@ -505,7 +505,7 @@ func (c *CustomFuncs) GenerateInvertedJoins( // using them here would result in a reduced set of optional // filters. optionalFilters = c.checkConstraintFilters(scanPrivate.Table) - computedColFilters := c.computedColFilters(scanPrivate.Table, on, optionalFilters) + computedColFilters := c.computedColFilters(scanPrivate, on, optionalFilters) optionalFilters = append(optionalFilters, computedColFilters...) eqColsAndOptionalFiltersCalculated = true diff --git a/pkg/sql/opt/xform/select_funcs.go b/pkg/sql/opt/xform/select_funcs.go index 6aeb69c43a9b..a0693794d94a 100644 --- a/pkg/sql/opt/xform/select_funcs.go +++ b/pkg/sql/opt/xform/select_funcs.go @@ -209,7 +209,7 @@ func (c *CustomFuncs) GenerateConstrainedScans( // Generate implicit filters from constraints and computed columns as // optional filters to help constrain an index scan. optionalFilters := c.checkConstraintFilters(scanPrivate.Table) - computedColFilters := c.computedColFilters(scanPrivate.Table, explicitFilters, optionalFilters) + computedColFilters := c.computedColFilters(scanPrivate, explicitFilters, optionalFilters) optionalFilters = append(optionalFilters, computedColFilters...) filterColumns := c.FilterOuterCols(explicitFilters) @@ -380,9 +380,11 @@ func (c *CustomFuncs) GenerateConstrainedScans( // This would add a mapping from x => 5 and y => 'foo', which constants can // then be used to prove that dependent computed columns are also constant. func (c *CustomFuncs) findConstantFilterCols( - constFilterCols map[opt.ColumnID]opt.ScalarExpr, tabID opt.TableID, filters memo.FiltersExpr, + constFilterCols map[opt.ColumnID]opt.ScalarExpr, + scanPrivate *memo.ScanPrivate, + filters memo.FiltersExpr, ) { - tab := c.e.mem.Metadata().Table(tabID) + tab := c.e.mem.Metadata().Table(scanPrivate.Table) for i := range filters { // If filter constraints are not tight, then no way to derive constant // values. @@ -399,14 +401,19 @@ func (c *CustomFuncs) findConstantFilterCols( continue } + // Skip columns that aren't in the scanned table. + colID := cons.Columns.Get(0).ID() + if !scanPrivate.Cols.Contains(colID) { + continue + } + // Skip columns with a data type that uses a composite key encoding. // Each of these data types can have multiple distinct values that // compare equal. For example, 0 == -0 for the FLOAT data type. It's // not safe to treat these as constant inputs to computed columns, // since the computed expression may differentiate between the // different forms of the same value. - colID := cons.Columns.Get(0).ID() - colTyp := tab.Column(tabID.ColumnOrdinal(colID)).DatumType() + colTyp := tab.Column(scanPrivate.Table.ColumnOrdinal(colID)).DatumType() if colinfo.HasCompositeKeyEncoding(colTyp) { continue } @@ -723,7 +730,7 @@ func (c *CustomFuncs) GenerateInvertedIndexScans( // Generate implicit filters from constraints and computed columns as // optional filters to help constrain an index scan. optionalFilters := c.checkConstraintFilters(scanPrivate.Table) - computedColFilters := c.computedColFilters(scanPrivate.Table, filters, optionalFilters) + computedColFilters := c.computedColFilters(scanPrivate, filters, optionalFilters) optionalFilters = append(optionalFilters, computedColFilters...) // Iterate over all inverted indexes. diff --git a/pkg/sql/opt/xform/testdata/rules/join b/pkg/sql/opt/xform/testdata/rules/join index 06634509eb19..c9be0d966cc4 100644 --- a/pkg/sql/opt/xform/testdata/rules/join +++ b/pkg/sql/opt/xform/testdata/rules/join @@ -3174,6 +3174,35 @@ anti-join (hash) └── filters └── column1:1 = y:3 [outer=(1,3), constraints=(/1: (/NULL - ]; /3: (/NULL - ]), fd=(1)==(3), (3)==(1)] +# Regression test for #59738. findConstantFilterCols should not panic when it +# finds constant filters for columns that are not in the index of the lookup +# table. +exec-ddl +CREATE TABLE t59738_ab (a INT, b INT AS (a + 10) STORED, INDEX (a)) +---- + +exec-ddl +CREATE TABLE t59738_cd (c INT, d INT) +---- + +opt expect=GenerateLookupJoins +SELECT * FROM t59738_cd LEFT LOOKUP JOIN t59738_ab ON a = c AND d > 5 +---- +left-join (lookup t59738_ab) + ├── columns: c:1 d:2 a:5 b:6 + ├── key columns: [7] = [7] + ├── lookup columns are key + ├── left-join (lookup t59738_ab@secondary) + │ ├── columns: c:1 d:2 a:5 t59738_ab.rowid:7 + │ ├── flags: force lookup join (into right side) + │ ├── key columns: [1] = [5] + │ ├── fd: (7)-->(5) + │ ├── scan t59738_cd + │ │ └── columns: c:1 d:2 + │ └── filters + │ └── d:2 > 5 [outer=(2), constraints=(/2: [/6 - ]; tight)] + └── filters (true) + # -------------------------------------------------- # GenerateLookupJoinsWithFilter # --------------------------------------------------