Skip to content

Commit

Permalink
Merge #62921
Browse files Browse the repository at this point in the history 
62921: opt: fix internal error when calculating stats for mutation passthrough cols r=rytaft a=rytaft

Prior to this commit, an attempt to calculate statistics for a passthrough
column in a mutation would cause an error, since passthrough columns were
inadvertently ignored by the `MapToInputCols` function. This commit fixes
the problem by updating `MapToInputCols` so that it checks whether any of the
given columns are in `PassthroughCols` before trying to map them to a table
column.

Fixes #62692

Release note (bug fix): Fixed an internal error that could occur during
planning when a query used the output of an UPDATE's RETURNING clause,
and one or more of the columns in the RETURNING clause were from a table
specified in the FROM clause of the UPDATE (i.e., not from the table being
updated).

Co-authored-by: Rebecca Taft <[email protected]>
  • Loading branch information
@rytaft
craig[bot] and rytaft committed Apr 1, 2021
2 parents 7160798 + 527204f commit 3f70efc
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 3 deletions.
17 changes: 14 additions & 3 deletions pkg/sql/opt/memo/expr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,17 +699,28 @@ func (m *MutationPrivate) MapToInputID(tabColID opt.ColumnID) opt.ColumnID {
return m.ReturnCols[ord]
}

// MapToInputCols maps the given set of table columns to a corresponding set of
// input columns using the MapToInputID function.
func (m *MutationPrivate) MapToInputCols(tabCols opt.ColSet) opt.ColSet {
// MapToInputCols maps the given set of columns to a corresponding set of
// input columns using the PassthroughCols list and MapToInputID function.
func (m *MutationPrivate) MapToInputCols(cols opt.ColSet) opt.ColSet {
var inCols opt.ColSet

// First see if any of the columns come from the passthrough columns.
for _, c := range m.PassthroughCols {
if cols.Contains(c) {
inCols.Add(c)
}
}

// The remaining columns must come from the table.
tabCols := cols.Difference(inCols)
tabCols.ForEach(func(t opt.ColumnID) {
id := m.MapToInputID(t)
if id == 0 {
panic(errors.AssertionFailedf("could not find input column for %d", log.Safe(t)))
}
inCols.Add(id)
})

return inCols
}

Expand Down
88 changes: 88 additions & 0 deletions pkg/sql/opt/memo/testdata/stats/update
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,91 @@ update xyz
│ └── false [type=bool, constraints=(contradiction; tight)]
└── projections
└── 'foo' [as=x_new:9, type=string]

# Regression test for #62692. Ensure we don't error when calculating stats for
# mutation passthrough columns

exec-ddl
CREATE TABLE parent (p INT PRIMARY KEY)
----

exec-ddl
CREATE TABLE child (x INT, c INT REFERENCES parent (p))
----

build
WITH q AS (UPDATE child SET c = p FROM parent WHERE p = 1 RETURNING p) SELECT * FROM q WHERE p = 1
----
with &2 (q)
├── columns: p:14(int!null)
├── volatile, mutations
├── stats: [rows=1000, distinct(14)=1, null(14)=0]
├── fd: ()-->(14)
├── project
│ ├── columns: parent.p:9(int)
│ ├── volatile, mutations
│ ├── stats: [rows=1000, distinct(9)=1, null(9)=0]
│ ├── fd: ()-->(9)
│ └── update child
│ ├── columns: x:1(int) child.c:2(int!null) rowid:3(int!null) parent.p:9(int) parent.crdb_internal_mvcc_timestamp:10(decimal)
│ ├── fetch columns: x:5(int) child.c:6(int) rowid:7(int)
│ ├── update-mapping:
│ │ └── parent.p:9 => child.c:2
│ ├── input binding: &1
│ ├── volatile, mutations
│ ├── stats: [rows=1000, distinct(9)=1, null(9)=0]
│ ├── key: (3)
│ ├── fd: ()-->(2,9,10), (2)==(9), (9)==(2), (3)-->(1)
│ ├── select
│ │ ├── columns: x:5(int) child.c:6(int) rowid:7(int!null) child.crdb_internal_mvcc_timestamp:8(decimal) parent.p:9(int!null) parent.crdb_internal_mvcc_timestamp:10(decimal)
│ │ ├── stats: [rows=1000, distinct(9)=1, null(9)=0]
│ │ ├── key: (7)
│ │ ├── fd: ()-->(9,10), (7)-->(5,6,8)
│ │ ├── inner-join (cross)
│ │ │ ├── columns: x:5(int) child.c:6(int) rowid:7(int!null) child.crdb_internal_mvcc_timestamp:8(decimal) parent.p:9(int!null) parent.crdb_internal_mvcc_timestamp:10(decimal)
│ │ │ ├── stats: [rows=1000000, distinct(7)=1000, null(7)=0, distinct(9)=1000, null(9)=0]
│ │ │ ├── key: (7,9)
│ │ │ ├── fd: (7)-->(5,6,8), (9)-->(10)
│ │ │ ├── scan child
│ │ │ │ ├── columns: x:5(int) child.c:6(int) rowid:7(int!null) child.crdb_internal_mvcc_timestamp:8(decimal)
│ │ │ │ ├── stats: [rows=1000, distinct(7)=1000, null(7)=0]
│ │ │ │ ├── key: (7)
│ │ │ │ └── fd: (7)-->(5,6,8)
│ │ │ ├── scan parent
│ │ │ │ ├── columns: parent.p:9(int!null) parent.crdb_internal_mvcc_timestamp:10(decimal)
│ │ │ │ ├── stats: [rows=1000, distinct(9)=1000, null(9)=0]
│ │ │ │ ├── key: (9)
│ │ │ │ └── fd: (9)-->(10)
│ │ │ └── filters (true)
│ │ └── filters
│ │ └── parent.p:9 = 1 [type=bool, outer=(9), constraints=(/9: [/1 - /1]; tight), fd=()-->(9)]
│ └── f-k-checks
│ └── f-k-checks-item: child(c) -> parent(p)
│ └── anti-join (hash)
│ ├── columns: c:11(int!null)
│ ├── stats: [rows=1e-10]
│ ├── fd: ()-->(11)
│ ├── with-scan &1
│ │ ├── columns: c:11(int!null)
│ │ ├── mapping:
│ │ │ └── parent.p:9(int) => c:11(int)
│ │ ├── stats: [rows=1000, distinct(11)=1, null(11)=0]
│ │ └── fd: ()-->(11)
│ ├── scan parent
│ │ ├── columns: parent.p:12(int!null)
│ │ ├── stats: [rows=1000, distinct(12)=1000, null(12)=0]
│ │ └── key: (12)
│ └── filters
│ └── c:11 = parent.p:12 [type=bool, outer=(11,12), constraints=(/11: (/NULL - ]; /12: (/NULL - ]), fd=(11)==(12), (12)==(11)]
└── select
├── columns: p:14(int!null)
├── stats: [rows=1000, distinct(14)=1, null(14)=0]
├── fd: ()-->(14)
├── with-scan &2 (q)
│ ├── columns: p:14(int)
│ ├── mapping:
│ │ └── parent.p:9(int) => p:14(int)
│ ├── stats: [rows=1000, distinct(14)=1, null(14)=0]
│ └── fd: ()-->(14)
└── filters
└── p:14 = 1 [type=bool, outer=(14), constraints=(/14: [/1 - /1]; tight), fd=()-->(14)]

0 comments on commit 3f70efc

Please sign in to comment.