From 49d15e2c6c292105fb7a02188218e2ac62a9dcc7 Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Tue, 12 Mar 2019 10:58:39 -0400 Subject: [PATCH] opt: use correct ordering for mutation input in execbuilder 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. --- .../exec/execbuilder/relational_builder.go | 8 ++--- pkg/sql/opt/exec/execbuilder/testdata/insert | 27 ++++++++++++++ pkg/sql/opt/exec/execbuilder/testdata/update | 26 ++++++++++++++ pkg/sql/opt/exec/execbuilder/testdata/upsert | 35 +++++++++++++++++++ 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/pkg/sql/opt/exec/execbuilder/relational_builder.go b/pkg/sql/opt/exec/execbuilder/relational_builder.go index 3cc4ef9bddf9..a310ab5cea06 100644 --- a/pkg/sql/opt/exec/execbuilder/relational_builder.go +++ b/pkg/sql/opt/exec/execbuilder/relational_builder.go @@ -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 } @@ -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 } @@ -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 } @@ -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 } diff --git a/pkg/sql/opt/exec/execbuilder/testdata/insert b/pkg/sql/opt/exec/execbuilder/testdata/insert index 927f4f416b21..f455d2e2cc95 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/insert +++ b/pkg/sql/opt/exec/execbuilder/testdata/insert @@ -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 · · diff --git a/pkg/sql/opt/exec/execbuilder/testdata/update b/pkg/sql/opt/exec/execbuilder/testdata/update index f2b6db8ca2c9..02ee30a15baf 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/update +++ b/pkg/sql/opt/exec/execbuilder/testdata/update @@ -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 · · diff --git a/pkg/sql/opt/exec/execbuilder/testdata/upsert b/pkg/sql/opt/exec/execbuilder/testdata/upsert index 1b6b5e2bc3d1..49ee92abe999 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/upsert +++ b/pkg/sql/opt/exec/execbuilder/testdata/upsert @@ -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 · ·