Skip to content

Commit

Permalink
opt: fix panic in GenerateLookupJoin
Browse files Browse the repository at this point in the history
In cockroachdb#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 cockroachdb#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.
  • Loading branch information
mgartner committed Feb 3, 2021
1 parent 98674ef commit 6a3e689
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 12 deletions.
8 changes: 4 additions & 4 deletions pkg/sql/opt/xform/general_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,18 +229,18 @@ 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
}

// 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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/xform/join_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 13 additions & 6 deletions pkg/sql/opt/xform/select_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 29 additions & 0 deletions pkg/sql/opt/xform/testdata/rules/join
Original file line number Diff line number Diff line change
Expand Up @@ -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
# --------------------------------------------------
Expand Down

0 comments on commit 6a3e689

Please sign in to comment.