Skip to content

Commit

Permalink
opt: use correct ordering for mutation input in execbuilder
Browse files Browse the repository at this point in the history 
We were setting up a projection on the mutation's input but we were
accidentally using the parent's ordering instead of that of the input.

Fixes #35564.

Release note (bug fix): Fixed a "column not in input" crash when
`INSERT / UPDATE / UPSERT ... RETURNING` is used inside a clause that
requires an ordering.
  • Loading branch information
@RaduBerinde
RaduBerinde committed Mar 12, 2019
1 parent 57fc3f3 commit 49d15e2
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 4 deletions.
8 changes: 4 additions & 4 deletions pkg/sql/opt/exec/execbuilder/relational_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1297,7 +1297,7 @@ func (b *Builder) buildInsert(ins *memo.InsertExpr) (execPlan, error) {
colList := make(opt.ColList, 0, len(ins.InsertCols)+len(ins.CheckCols))
colList = appendColsWhenPresent(colList, ins.InsertCols)
colList = appendColsWhenPresent(colList, ins.CheckCols)
input, err = b.ensureColumns(input, colList, nil, ins.ProvidedPhysical().Ordering)
input, err = b.ensureColumns(input, colList, nil, ins.Input.ProvidedPhysical().Ordering)
if err != nil {
return execPlan{}, err
}
Expand Down Expand Up @@ -1348,7 +1348,7 @@ func (b *Builder) buildUpdate(upd *memo.UpdateExpr) (execPlan, error) {
colList = appendColsWhenPresent(colList, upd.FetchCols)
colList = appendColsWhenPresent(colList, upd.UpdateCols)
colList = appendColsWhenPresent(colList, upd.CheckCols)
input, err = b.ensureColumns(input, colList, nil, upd.ProvidedPhysical().Ordering)
input, err = b.ensureColumns(input, colList, nil, upd.Input.ProvidedPhysical().Ordering)
if err != nil {
return execPlan{}, err
}
Expand Down Expand Up @@ -1414,7 +1414,7 @@ func (b *Builder) buildUpsert(ups *memo.UpsertExpr) (execPlan, error) {
colList = append(colList, ups.CanaryCol)
}
colList = appendColsWhenPresent(colList, ups.CheckCols)
input, err = b.ensureColumns(input, colList, nil, ups.ProvidedPhysical().Ordering)
input, err = b.ensureColumns(input, colList, nil, ups.Input.ProvidedPhysical().Ordering)
if err != nil {
return execPlan{}, err
}
Expand Down Expand Up @@ -1473,7 +1473,7 @@ func (b *Builder) buildDelete(del *memo.DeleteExpr) (execPlan, error) {
// Upgrade execution engine to not require this.
colList := make(opt.ColList, 0, len(del.FetchCols))
colList = appendColsWhenPresent(colList, del.FetchCols)
input, err = b.ensureColumns(input, colList, nil, del.ProvidedPhysical().Ordering)
input, err = b.ensureColumns(input, colList, nil, del.Input.ProvidedPhysical().Ordering)
if err != nil {
return execPlan{}, err
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/insert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,30 @@ count · · ()

statement ok
ROLLBACK

# Regression test for #35564: make sure we use the Insert's input required
# ordering for the internal projection.

statement ok
CREATE TABLE abc (a INT, b INT, c INT, INDEX(c) STORING(a,b))

statement ok
CREATE TABLE xyz (x INT, y INT, z INT)

query TTTTT
EXPLAIN (VERBOSE) SELECT * FROM [INSERT INTO xyz SELECT a, b, c FROM abc RETURNING z] ORDER BY z
----
render · · (z) +z
│ render 0 z · ·
└── run · · (x, y, z, rowid[hidden]) ·
└── insert · · (x, y, z, rowid[hidden]) ·
│ into xyz(x, y, z, rowid) · ·
│ strategy inserter · ·
└── render · · (a, b, c, column9) +c
│ render 0 a · ·
│ render 1 b · ·
│ render 2 c · ·
│ render 3 unique_rowid() · ·
└── scan · · (a, b, c) +c
· table abc@abc_c_idx · ·
· spans ALL · ·
26 changes: 26 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/update
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,3 +296,29 @@ Del /Table/57/1/1/1/1
Del /Table/57/1/1/2/1
fast path completed
rows affected: 1

# Regression test for #35564: make sure we use the Update's input required
# ordering for the internal projection.

statement ok
CREATE TABLE abc (a INT, b INT, c INT, INDEX(c) STORING(a,b))

query TTTTT
EXPLAIN (VERBOSE) SELECT * FROM [ UPDATE abc SET a=c RETURNING a ] ORDER BY a
----
render · · (a) +a
│ render 0 a · ·
└── run · · (a, b, c, rowid[hidden]) ·
└── update · · (a, b, c, rowid[hidden]) ·
│ table abc · ·
│ set a · ·
│ strategy updater · ·
└── render · · (a, b, c, rowid, c) +c
│ render 0 a · ·
│ render 1 b · ·
│ render 2 c · ·
│ render 3 rowid · ·
│ render 4 c · ·
└── scan · · (a, b, c, rowid[hidden]) +c
· table abc@abc_c_idx · ·
· spans ALL · ·
35 changes: 35 additions & 0 deletions pkg/sql/opt/exec/execbuilder/testdata/upsert
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,3 +307,38 @@ INSERT INTO t5 VALUES (1, 10, 9) ON CONFLICT (k) DO NOTHING

statement ok
INSERT INTO t5 VALUES (1, 10, 20) ON CONFLICT (k) DO NOTHING

# Regression test for #35564: make sure we use the Upsert's input required
# ordering for the internal projection.

statement ok
CREATE TABLE abc (a INT, b INT, c INT, INDEX(c) STORING(a,b))

statement ok
CREATE TABLE xyz (x INT, y INT, z INT)

query TTTTT
EXPLAIN (VERBOSE) SELECT * FROM [UPSERT INTO xyz SELECT a, b, c FROM abc RETURNING z] ORDER BY z
----
render · · (z) +z
│ render 0 z · ·
└── run · · (x, y, z, rowid[hidden]) ·
└── upsert · · (x, y, z, rowid[hidden]) ·
│ into xyz(x, y, z, rowid) · ·
│ strategy opt upserter · ·
└── render · · (a, b, c, column9, a, b, c) +c
│ render 0 a · ·
│ render 1 b · ·
│ render 2 c · ·
│ render 3 column9 · ·
│ render 4 a · ·
│ render 5 b · ·
│ render 6 c · ·
└── render · · (column9, a, b, c) +c
│ render 0 unique_rowid() · ·
│ render 1 a · ·
│ render 2 b · ·
│ render 3 c · ·
└── scan · · (a, b, c) +c
· table abc@abc_c_idx · ·
· spans ALL · ·

0 comments on commit 49d15e2

Please sign in to comment.