Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

opt: prevent columns reuse in Union and UnionAll #58477

Merged
merged 3 commits into from
Jan 7, 2021
Merged

opt: prevent columns reuse in Union and UnionAll #58477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c8ffd86
opt: fix columns in SplitScanIntoUnionScans constraint
mgartner Jan 6, 2021
58b3efb
opt: do not reuse columns for Unions in SplitScanIntoUnionScans
mgartner Jan 5, 2021
80d7605
opt: add Union column ID check to CheckExpr
mgartner Jan 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions pkg/sql/opt/memo/check_expr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,30 @@ func (m *Memo) CheckExpr(e opt.Expr) {
case *SelectExpr:
checkFilters(t.Filters)

case *UnionExpr, *UnionAllExpr:
setPrivate := t.Private().(*SetPrivate)
outColSet := setPrivate.OutCols.ToSet()

// Check that columns on the left side of the union are not reused in
// the output.
leftColSet := setPrivate.LeftCols.ToSet()
if outColSet.Intersects(leftColSet) {
panic(errors.AssertionFailedf(
"union reuses columns in left input: %v",
outColSet.Intersection(leftColSet),
))
}

// Check that columns on the right side of the union are not reused in
// the output.
rightColSet := setPrivate.RightCols.ToSet()
if outColSet.Intersects(rightColSet) {
panic(errors.AssertionFailedf(
"union reuses columns in right input: %v",
outColSet.Intersection(rightColSet),
))
}

case *AggregationsExpr:
var checkAggs func(scalar opt.ScalarExpr)
checkAggs = func(scalar opt.ScalarExpr) {
Expand Down
38 changes: 23 additions & 15 deletions pkg/sql/opt/norm/general_funcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,26 @@ func (c *CustomFuncs) RedundantCols(input memo.RelExpr, cols opt.ColSet) opt.Col
return cols.Difference(reducedCols)
}

// DuplicateColumnIDs duplicates a table and set of columns IDs in the metadata.
// It returns the new table's ID and the new set of columns IDs.
func (c *CustomFuncs) DuplicateColumnIDs(
table opt.TableID, cols opt.ColSet,
) (opt.TableID, opt.ColSet) {
md := c.mem.Metadata()
tabMeta := md.TableMeta(table)
newTableID := md.DuplicateTable(table, c.RemapCols)

// Build a new set of column IDs from the new TableMeta.
var newColIDs opt.ColSet
for col, ok := cols.Next(0); ok; col, ok = cols.Next(col + 1) {
ord := tabMeta.MetaID.ColumnOrdinal(col)
newColID := newTableID.ColumnID(ord)
newColIDs.Add(newColID)
}

return newTableID, newColIDs
}

// RemapCols remaps columns IDs in the input ScalarExpr by replacing occurrences
// of the keys of colMap with the corresponding values. If column IDs are
// encountered in the input ScalarExpr that are not keys in colMap, they are not
Expand Down Expand Up @@ -1054,23 +1074,11 @@ func (c *CustomFuncs) DatumsEqual(first, second tree.Datum) bool {
// ScanPrivate, so the new ScanPrivate will not have constraints even if the old
// one did.
func (c *CustomFuncs) DuplicateScanPrivate(sp *memo.ScanPrivate) *memo.ScanPrivate {
md := c.mem.Metadata()
tabMeta := md.TableMeta(sp.Table)
newTableID := md.DuplicateTable(sp.Table, c.RemapCols)

// Build a new set of column IDs from the new TableMeta.
var newColIDs opt.ColSet
cols := sp.Cols
for col, ok := cols.Next(0); ok; col, ok = cols.Next(col + 1) {
ord := tabMeta.MetaID.ColumnOrdinal(col)
newColID := newTableID.ColumnID(ord)
newColIDs.Add(newColID)
}

table, cols := c.DuplicateColumnIDs(sp.Table, sp.Cols)
return &memo.ScanPrivate{
Table: newTableID,
Table: table,
Index: sp.Index,
Cols: newColIDs,
Cols: cols,
Flags: sp.Flags,
Locking: sp.Locking,
}
Expand Down
29 changes: 20 additions & 9 deletions pkg/sql/opt/xform/limit_funcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,11 @@ func (c *CustomFuncs) SplitScanIntoUnionScans(

// makeNewUnion extends the Union tree rooted at 'last' to include 'newScan'.
// The ColumnIDs of the original Scan are used by the resulting expression.
oldColList := scan.Relational().OutputCols.ToList()
makeNewUnion := func(last, newScan memo.RelExpr) memo.RelExpr {
makeNewUnion := func(last, newScan memo.RelExpr, outCols opt.ColList) memo.RelExpr {
return c.e.f.ConstructUnion(last, newScan, &memo.SetPrivate{
LeftCols: last.Relational().OutputCols.ToList(),
RightCols: newScan.Relational().OutputCols.ToList(),
OutCols: oldColList,
OutCols: outCols,
})
}

Expand All @@ -263,7 +262,7 @@ func (c *CustomFuncs) SplitScanIntoUnionScans(
// construct a single unlimited Scan, which will also be added to the Unions.
var noLimitSpans constraint.Spans
var last memo.RelExpr
for i := 0; i < spans.Count(); i++ {
for i, n := 0, spans.Count(); i < n; i++ {
if i >= budgetExceededIndex {
// The Scan budget has been reached; no additional Scans can be created.
noLimitSpans.Append(spans.Get(i))
Expand All @@ -276,15 +275,27 @@ func (c *CustomFuncs) SplitScanIntoUnionScans(
noLimitSpans.Append(spans.Get(i))
continue
}
for j := 0; j < singleKeySpans.Count(); j++ {
for j, m := 0, singleKeySpans.Count(); j < m; j++ {
if last == nil {
// This is the first limited Scan, so no Union necessary.
last = c.makeNewScan(sp, cons.Columns, newHardLimit, singleKeySpans.Get(j))
continue
}
// Construct a new Scan for each span and add it to the Union tree.
// Construct a new Scan for each span.
newScan := c.makeNewScan(sp, cons.Columns, newHardLimit, singleKeySpans.Get(j))
last = makeNewUnion(last, newScan)

// Add the scan to the union tree. If it is the final union in the
// tree, use the original scan's columns as the union's out columns.
// Otherwise, create new output column IDs for the union.
var outCols opt.ColList
finalUnion := i == n-1 && j == m-1 && noLimitSpans.Count() == 0
if finalUnion {
outCols = sp.Cols.ToList()
} else {
_, cols := c.DuplicateColumnIDs(sp.Table, sp.Cols)
outCols = cols.ToList()
}
last = makeNewUnion(last, newScan, outCols)
}
}
if noLimitSpans.Count() == spans.Count() {
Expand All @@ -302,11 +313,11 @@ func (c *CustomFuncs) SplitScanIntoUnionScans(
// construct an unlimited Scan and add it to the Union tree.
newScanPrivate := c.DuplicateScanPrivate(sp)
newScanPrivate.Constraint = &constraint.Constraint{
Columns: sp.Constraint.Columns,
Columns: sp.Constraint.Columns.RemapColumns(sp.Table, newScanPrivate.Table),
Spans: noLimitSpans,
}
newScan := c.e.f.ConstructScan(newScanPrivate)
return makeNewUnion(last, newScan)
return makeNewUnion(last, newScan, sp.Cols.ToList())
}

// indexHasOrderingSequence returns whether the Scan can provide a given
Expand Down
36 changes: 18 additions & 18 deletions pkg/sql/opt/xform/testdata/external/trading
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,25 +612,25 @@ project
│ │ │ ├── limit hint: 1.00
│ │ │ └── union
│ │ │ ├── columns: dealerid:8!null version:16!null
│ │ │ ├── left columns: dealerid:8!null version:16!null
│ │ │ ├── right columns: dealerid:65 version:73
│ │ │ ├── left columns: dealerid:75 version:83
│ │ │ ├── right columns: dealerid:85 version:93
│ │ │ ├── cardinality: [0 - 4]
│ │ │ ├── stats: [rows=4, distinct(8,16)=4, null(8,16)=0]
│ │ │ ├── key: (8,16)
│ │ │ ├── union
│ │ │ │ ├── columns: dealerid:8!null version:16!null
│ │ │ │ ├── left columns: dealerid:8!null version:16!null
│ │ │ │ ├── right columns: dealerid:55 version:63
│ │ │ │ ├── columns: dealerid:75!null version:83!null
│ │ │ │ ├── left columns: dealerid:55 version:63
│ │ │ │ ├── right columns: dealerid:65 version:73
│ │ │ │ ├── cardinality: [0 - 3]
│ │ │ │ ├── stats: [rows=3, distinct(8,16)=3, null(8,16)=0]
│ │ │ │ ├── key: (8,16)
│ │ │ │ ├── stats: [rows=3, distinct(75,83)=3, null(75,83)=0]
│ │ │ │ ├── key: (75,83)
│ │ │ │ ├── union
│ │ │ │ │ ├── columns: dealerid:8!null version:16!null
│ │ │ │ │ ├── columns: dealerid:55!null version:63!null
│ │ │ │ │ ├── left columns: dealerid:35 version:43
│ │ │ │ │ ├── right columns: dealerid:45 version:53
│ │ │ │ │ ├── cardinality: [0 - 2]
│ │ │ │ │ ├── stats: [rows=2, distinct(8,16)=2, null(8,16)=0]
│ │ │ │ │ ├── key: (8,16)
│ │ │ │ │ ├── stats: [rows=2, distinct(55,63)=2, null(55,63)=0]
│ │ │ │ │ ├── key: (55,63)
│ │ │ │ │ ├── scan cardsinfo@cardsinfoversionindex,rev
│ │ │ │ │ │ ├── columns: dealerid:35!null version:43!null
│ │ │ │ │ │ ├── constraint: /35/43: [/1 - /1]
Expand All @@ -646,19 +646,19 @@ project
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(45,53)
│ │ │ │ └── scan cardsinfo@cardsinfoversionindex,rev
│ │ │ │ ├── columns: dealerid:55!null version:63!null
│ │ │ │ ├── constraint: /55/63: [/3 - /3]
│ │ │ │ ├── columns: dealerid:65!null version:73!null
│ │ │ │ ├── constraint: /65/73: [/3 - /3]
│ │ │ │ ├── limit: 1(rev)
│ │ │ │ ├── stats: [rows=1, distinct(55)=1, null(55)=0, distinct(55,63)=1, null(55,63)=0]
│ │ │ │ ├── stats: [rows=1, distinct(65)=1, null(65)=0, distinct(65,73)=1, null(65,73)=0]
│ │ │ │ ├── key: ()
│ │ │ │ └── fd: ()-->(55,63)
│ │ │ │ └── fd: ()-->(65,73)
│ │ │ └── scan cardsinfo@cardsinfoversionindex,rev
│ │ │ ├── columns: dealerid:65!null version:73!null
│ │ │ ├── constraint: /65/73: [/4 - /4]
│ │ │ ├── columns: dealerid:85!null version:93!null
│ │ │ ├── constraint: /85/93: [/4 - /4]
│ │ │ ├── limit: 1(rev)
│ │ │ ├── stats: [rows=1, distinct(65)=1, null(65)=0, distinct(65,73)=1, null(65,73)=0]
│ │ │ ├── stats: [rows=1, distinct(85)=1, null(85)=0, distinct(85,93)=1, null(85,93)=0]
│ │ │ ├── key: ()
│ │ │ └── fd: ()-->(65,73)
│ │ │ └── fd: ()-->(85,93)
│ │ └── 1
│ └── aggregations
│ └── const-agg [as=max:33, outer=(16)]
Expand Down
36 changes: 18 additions & 18 deletions pkg/sql/opt/xform/testdata/external/trading-mutation
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,25 +618,25 @@ project
│ │ │ ├── limit hint: 1.00
│ │ │ └── union
│ │ │ ├── columns: dealerid:8!null version:16!null
│ │ │ ├── left columns: dealerid:8!null version:16!null
│ │ │ ├── right columns: dealerid:81 version:89
│ │ │ ├── left columns: dealerid:95 version:103
│ │ │ ├── right columns: dealerid:109 version:117
│ │ │ ├── cardinality: [0 - 4]
│ │ │ ├── stats: [rows=4, distinct(8,16)=4, null(8,16)=0]
│ │ │ ├── key: (8,16)
│ │ │ ├── union
│ │ │ │ ├── columns: dealerid:8!null version:16!null
│ │ │ │ ├── left columns: dealerid:8!null version:16!null
│ │ │ │ ├── right columns: dealerid:67 version:75
│ │ │ │ ├── columns: dealerid:95!null version:103!null
│ │ │ │ ├── left columns: dealerid:67 version:75
│ │ │ │ ├── right columns: dealerid:81 version:89
│ │ │ │ ├── cardinality: [0 - 3]
│ │ │ │ ├── stats: [rows=3, distinct(8,16)=3, null(8,16)=0]
│ │ │ │ ├── key: (8,16)
│ │ │ │ ├── stats: [rows=3, distinct(95,103)=3, null(95,103)=0]
│ │ │ │ ├── key: (95,103)
│ │ │ │ ├── union
│ │ │ │ │ ├── columns: dealerid:8!null version:16!null
│ │ │ │ │ ├── columns: dealerid:67!null version:75!null
│ │ │ │ │ ├── left columns: dealerid:39 version:47
│ │ │ │ │ ├── right columns: dealerid:53 version:61
│ │ │ │ │ ├── cardinality: [0 - 2]
│ │ │ │ │ ├── stats: [rows=2, distinct(8,16)=2, null(8,16)=0]
│ │ │ │ │ ├── key: (8,16)
│ │ │ │ │ ├── stats: [rows=2, distinct(67,75)=2, null(67,75)=0]
│ │ │ │ │ ├── key: (67,75)
│ │ │ │ │ ├── scan cardsinfo@cardsinfoversionindex,rev
│ │ │ │ │ │ ├── columns: dealerid:39!null version:47!null
│ │ │ │ │ │ ├── constraint: /39/47: [/1 - /1]
Expand All @@ -652,19 +652,19 @@ project
│ │ │ │ │ ├── key: ()
│ │ │ │ │ └── fd: ()-->(53,61)
│ │ │ │ └── scan cardsinfo@cardsinfoversionindex,rev
│ │ │ │ ├── columns: dealerid:67!null version:75!null
│ │ │ │ ├── constraint: /67/75: [/3 - /3]
│ │ │ │ ├── columns: dealerid:81!null version:89!null
│ │ │ │ ├── constraint: /81/89: [/3 - /3]
│ │ │ │ ├── limit: 1(rev)
│ │ │ │ ├── stats: [rows=1, distinct(67)=1, null(67)=0, distinct(67,75)=1, null(67,75)=0]
│ │ │ │ ├── stats: [rows=1, distinct(81)=1, null(81)=0, distinct(81,89)=1, null(81,89)=0]
│ │ │ │ ├── key: ()
│ │ │ │ └── fd: ()-->(67,75)
│ │ │ │ └── fd: ()-->(81,89)
│ │ │ └── scan cardsinfo@cardsinfoversionindex,rev
│ │ │ ├── columns: dealerid:81!null version:89!null
│ │ │ ├── constraint: /81/89: [/4 - /4]
│ │ │ ├── columns: dealerid:109!null version:117!null
│ │ │ ├── constraint: /109/117: [/4 - /4]
│ │ │ ├── limit: 1(rev)
│ │ │ ├── stats: [rows=1, distinct(81)=1, null(81)=0, distinct(81,89)=1, null(81,89)=0]
│ │ │ ├── stats: [rows=1, distinct(109)=1, null(109)=0, distinct(109,117)=1, null(109,117)=0]
│ │ │ ├── key: ()
│ │ │ └── fd: ()-->(81,89)
│ │ │ └── fd: ()-->(109,117)
│ │ └── 1
│ └── aggregations
│ └── const-agg [as=max:37, outer=(16)]
Expand Down
Loading