diff --git a/pkg/sql/delete.go b/pkg/sql/delete.go index 00134b08cd2f..4b2c4666df0e 100644 --- a/pkg/sql/delete.go +++ b/pkg/sql/delete.go @@ -60,6 +60,11 @@ type deleteRun struct { // of the mutation. Otherwise, the value at the i-th index refers to the // index of the resultRowBuffer where the i-th column is to be returned. rowIdxToRetIdx []int + + // numPassthrough is the number of columns in addition to the set of columns + // of the target table being returned, that must be passed through from the + // input node. + numPassthrough int } var _ mutationPlanNode = &deleteNode{} @@ -157,7 +162,7 @@ func (d *deleteNode) processSourceRow(params runParams, sourceVals tree.Datums) // This set is passed as a argument to tableDeleter.row below. var pm row.PartialIndexUpdateHelper if n := len(d.run.td.tableDesc().PartialIndexes()); n > 0 { - offset := d.run.partialIndexDelValsOffset + offset := d.run.partialIndexDelValsOffset + d.run.numPassthrough partialIndexDelVals := sourceVals[offset : offset+n] err := pm.Init(nil /*partialIndexPutVals */, partialIndexDelVals, d.run.td.tableDesc()) @@ -184,12 +189,32 @@ func (d *deleteNode) processSourceRow(params runParams, sourceVals tree.Datums) // d.run.rows.NumCols() is guaranteed to only contain the requested // public columns. resultValues := make(tree.Datums, d.run.td.rows.NumCols()) - for i, retIdx := range d.run.rowIdxToRetIdx { + largestRetIdx := -1 + for i := range d.run.rowIdxToRetIdx { + retIdx := d.run.rowIdxToRetIdx[i] if retIdx >= 0 { + if retIdx >= largestRetIdx { + largestRetIdx = retIdx + } resultValues[retIdx] = sourceVals[i] } } + // At this point we've extracted all the RETURNING values that are part + // of the target table. We must now extract the columns in the RETURNING + // clause that refer to other tables (from the USING clause of the delete). + if d.run.numPassthrough > 0 { + passthroughBegin := len(d.run.td.rd.FetchCols) + passthroughEnd := passthroughBegin + d.run.numPassthrough + passthroughValues := sourceVals[passthroughBegin:passthroughEnd] + + for i := 0; i < d.run.numPassthrough; i++ { + largestRetIdx++ + resultValues[largestRetIdx] = passthroughValues[i] + } + + } + if _, err := d.run.td.rows.AddRow(params.ctx, resultValues); err != nil { return err } diff --git a/pkg/sql/distsql_spec_exec_factory.go b/pkg/sql/distsql_spec_exec_factory.go index 02cc1b8ff64e..84105be01a71 100644 --- a/pkg/sql/distsql_spec_exec_factory.go +++ b/pkg/sql/distsql_spec_exec_factory.go @@ -997,6 +997,7 @@ func (e *distSQLSpecExecFactory) ConstructDelete( table cat.Table, fetchCols exec.TableColumnOrdinalSet, returnCols exec.TableColumnOrdinalSet, + passthrough colinfo.ResultColumns, autoCommit bool, ) (exec.Node, error) { return nil, unimplemented.NewWithIssue(47473, "experimental opt-driven distsql planning: delete") diff --git a/pkg/sql/logictest/testdata/logic_test/cursor b/pkg/sql/logictest/testdata/logic_test/cursor index 733e9a32fddb..cb720f04468d 100644 --- a/pkg/sql/logictest/testdata/logic_test/cursor +++ b/pkg/sql/logictest/testdata/logic_test/cursor @@ -598,4 +598,3 @@ FETCH 1 a b; statement ok COMMIT; - diff --git a/pkg/sql/logictest/testdata/logic_test/delete b/pkg/sql/logictest/testdata/logic_test/delete index 719d5dc5721b..ffc14f3af7a9 100644 --- a/pkg/sql/logictest/testdata/logic_test/delete +++ b/pkg/sql/logictest/testdata/logic_test/delete @@ -307,9 +307,6 @@ SELECT x, y, z FROM family 1 1 NULL 3 3 NULL -statement error at or near "where": syntax error: unimplemented: this syntax -DELETE FROM family USING family, other_table WHERE x=2 - # Verify that the fast path does its deletes at the expected timestamp. statement ok CREATE TABLE a (a INT PRIMARY KEY) @@ -337,3 +334,217 @@ SELECT * FROM a AS OF SYSTEM TIME $ts 3 4 5 + +# Test that USING works + +statement ok +CREATE TABLE u_a ( + a INT NOT NULL PRIMARY KEY, + b STRING, + c INT +); + +statement ok +CREATE TABLE u_b ( + a INT NOT NULL PRIMARY KEY, + b STRING +); + +statement ok +CREATE TABLE u_c ( + a INT NOT NULL PRIMARY KEY, + b STRING, + c INT +); + +statement ok +CREATE TABLE u_d ( + a INT, + b INT +); + +statement count 4 +INSERT INTO u_a VALUES (1, 'a', 10), (2, 'b', 20), (3, 'c', 30), (4, 'd', 40); + +statement count 4 +INSERT INTO u_b VALUES (10, 'a'), (20, 'b'), (30, 'c'), (40, 'd'); + +statement count 4 +INSERT INTO u_c VALUES (1, 'a', 10), (2, 'b', 50), (3, 'c', 50), (4, 'd', 40); + +query ITI rowsort +SELECT * FROM u_a; +---- +1 a 10 +2 b 20 +3 c 30 +4 d 40 + +#Test a join with a filter +statement ok +DELETE FROM u_a USING u_b WHERE c = u_b.a AND u_b.b = 'd'; + +query ITI rowsort +SELECT * FROM u_a; +---- +1 a 10 +2 b 20 +3 c 30 + +# Test a self join +statement count 2 +INSERT INTO u_a VALUES (5, 'd', 5), (6, 'e', 6); + +statement ok +DELETE FROM u_a USING u_a u_a2 WHERE u_a.a = u_a2.c; + +query ITI rowsort +SELECT * FROM u_a; +---- +1 a 10 +2 b 20 +3 c 30 + +# Test when USING uses multiple tables + +statement count 1 +INSERT INTO u_c VALUES (30, 'a', 1); + +statement ok +DELETE FROM u_a USING u_b, u_c WHERE u_a.c = u_b.a AND u_a.c = u_c.a + +query ITI rowsort +SELECT * FROM u_a; +---- +1 a 10 +2 b 20 + +# Test if USING works well with RETURNING expressions that reference +# the USING table and target table +query ITIT colnames,rowsort +DELETE FROM u_a USING u_b WHERE u_a.c = u_b.a RETURNING u_b.a, u_b.b, u_a.a, u_a.b; +---- +a b a b +10 a 1 a +20 b 2 b + +query ITI rowsort +SELECT * FROM u_a; +---- + +statement count 4 +INSERT INTO u_a VALUES (1, 'a', 10), (2, 'b', 20), (3, 'c', 30), (4, 'd', 40); + +# Test if RETURNING * returns everything +query ITIITI colnames,rowsort +DELETE FROM u_a USING u_c WHERE u_a.c = u_c.c RETURNING *; +---- +a b c a b c +1 a 10 1 a 10 +4 d 40 4 d 40 + +# Clean u_a to input a new set of data, and to improve test readability +statement ok +TRUNCATE u_a; + +statement count 9 +INSERT INTO u_a VALUES (1, 'a', 5), (2, 'b', 10), (3, 'c', 15), (4, 'd', 20), (5, 'd', 25), (6, 'd', 30), (7, 'd', 35), (8, 'd', 40), (9, 'd', 45); + +# Using ORDER BY and LIMIT with a `DELETE ... USING` where ORDER BY and LIMIT references the USING +# table is not supported. +# TODO (faizaanmadhani): Add support in DELETE ... USING for ORDER BY clauses to reference the USING +# table. This is not supported in UPDATE ... FROM either: #89817 +statement error SELECT DISTINCT ON expressions must match initial ORDER BY expressions +DELETE FROM u_a AS foo USING u_b AS bar WHERE bar.a > foo.c ORDER BY bar.a DESC LIMIT 3 RETURNING *; + +# Test aliased table names, ORDER BY and LIMIT where ORDER BY references the target +# table. +query ITIIT +DELETE FROM u_a AS foo USING u_b AS bar WHERE bar.a > foo.c ORDER BY foo.a DESC LIMIT 3 RETURNING *; +---- +7 d 35 40 d +6 d 30 40 d +5 d 25 40 d + +query ITI rowsort +SELECT * FROM u_a; +---- +1 a 5 +2 b 10 +3 c 15 +4 d 20 +8 d 40 +9 d 45 + +statement 4 +INSERT INTO u_d VALUES (1, 10), (2, 20), (3, 30), (4, 40) + +query ITI rowsort +SELECT * FROM u_a; +---- +1 a 5 +2 b 10 +3 c 15 +4 d 20 +8 d 40 +9 d 45 + +query IT rowsort +SELECT * FROM u_b; +---- +10 a +20 b +30 c +40 d + +query ITI rowsort +SELECT * FROM u_c; +---- +1 a 10 +2 b 50 +3 c 50 +4 d 40 +30 a 1 + +# Test if DELETE FROM ... USING works with LATERAL + +statement ok +DELETE FROM u_a USING u_b, LATERAL (SELECT u_c.a, u_c.b, u_c.c FROM u_c WHERE u_b.b = u_c.b) AS other WHERE other.c = 1 AND u_a.c = 35; + +query ITI rowsort +SELECT * FROM u_a +---- +1 a 5 +2 b 10 +3 c 15 +4 d 20 +8 d 40 +9 d 45 + +# Test if DELETE FROM ... USING works with partial indexes +statement ok +CREATE TABLE pindex ( + a DECIMAL (10, 2), + CHECK (a > 0) +); + +statement count 9 +INSERT INTO pindex VALUES (0.1), (0.2), (0.3), (1.0), (1.0), (1.1), (1.2), (1.3), (2.0); + +query FF colnames +DELETE FROM pindex p USING (VALUES (1.0)) v(b) WHERE p.a = v.b RETURNING p.a, v.b; +---- +a b +1.00 1.0 +1.00 1.0 + +query F rowsort +SELECT * FROM pindex; +---- +0.10 +0.20 +0.30 +1.10 +1.20 +1.30 +2.00 diff --git a/pkg/sql/logictest/testdata/logic_test/views b/pkg/sql/logictest/testdata/logic_test/views index 61796b891858..a985d90b22bf 100644 --- a/pkg/sql/logictest/testdata/logic_test/views +++ b/pkg/sql/logictest/testdata/logic_test/views @@ -1492,4 +1492,3 @@ SELECT * FROM v; statement ok SET DATABASE = test; - diff --git a/pkg/sql/opt/exec/execbuilder/mutation.go b/pkg/sql/opt/exec/execbuilder/mutation.go index 430a12477e47..f24df385ba7c 100644 --- a/pkg/sql/opt/exec/execbuilder/mutation.go +++ b/pkg/sql/opt/exec/execbuilder/mutation.go @@ -493,25 +493,39 @@ func (b *Builder) buildDelete(del *memo.DeleteExpr) (execPlan, error) { // // TODO(andyk): Using ensureColumns here can result in an extra Render. // Upgrade execution engine to not require this. - colList := make(opt.ColList, 0, len(del.FetchCols)+len(del.PartialIndexDelCols)) + colList := make(opt.ColList, 0, len(del.FetchCols)+len(del.PartialIndexDelCols)+len(del.PassthroughCols)) colList = appendColsWhenPresent(colList, del.FetchCols) colList = appendColsWhenPresent(colList, del.PartialIndexDelCols) + if del.NeedResults() { + colList = append(colList, del.PassthroughCols...) + } + input, err := b.buildMutationInput(del, del.Input, colList, &del.MutationPrivate) if err != nil { return execPlan{}, err } - // Construct the Delete node. md := b.mem.Metadata() tab := md.Table(del.Table) fetchColOrds := ordinalSetFromColList(del.FetchCols) returnColOrds := ordinalSetFromColList(del.ReturnCols) + + //Construct the result columns for the passthrough set + var passthroughCols colinfo.ResultColumns + if del.NeedResults() { + for _, passthroughCol := range del.PassthroughCols { + colMeta := b.mem.Metadata().ColumnMeta(passthroughCol) + passthroughCols = append(passthroughCols, colinfo.ResultColumn{Name: colMeta.Alias, Typ: colMeta.Type}) + } + } + node, err := b.factory.ConstructDelete( input.root, tab, fetchColOrds, returnColOrds, + passthroughCols, b.allowAutoCommit && len(del.FKChecks) == 0 && len(del.FKCascades) == 0, ) if err != nil { diff --git a/pkg/sql/opt/exec/explain/testdata/gists b/pkg/sql/opt/exec/explain/testdata/gists index 50c85a0793ee..933be8d58b74 100644 --- a/pkg/sql/opt/exec/explain/testdata/gists +++ b/pkg/sql/opt/exec/explain/testdata/gists @@ -699,8 +699,8 @@ explain(gist): gist-explain-roundtrip DELETE FROM foo ---- -hash: 5369057709634423529 -plan-gist: AgFqAgAHAAAAI2oB +hash: 17378315733259356217 +plan-gist: AgFqAgAHAAAAI2oAAQ== explain(shape): • delete │ from: foo @@ -722,8 +722,8 @@ explain(gist): gist-explain-roundtrip DELETE FROM foo WHERE a = 1 ---- -hash: 7691685103096689151 -plan-gist: AgFqAgAHAgAAI2oB +hash: 11485970487285265051 +plan-gist: AgFqAgAHAgAAI2oAAQ== explain(shape): • delete │ from: foo @@ -741,6 +741,27 @@ explain(gist): table: foo@foo_pkey spans: 1 span +opt +DELETE FROM foo WHERE a = 1 +---- +delete t.public.foo + ├── columns: + ├── fetch columns: t.public.foo.a:7(int) t.public.foo.b:8(int[]) t.public.foo.c:9(string) + ├── cardinality: [0 - 0] + ├── volatile, mutations + ├── stats: [rows=0] + ├── cost: 5.08 + └── scan t.public.foo + ├── columns: t.public.foo.a:7(int!null) t.public.foo.b:8(int[]) t.public.foo.c:9(string) + ├── constraint: /7: [/1 - /1] + ├── cardinality: [0 - 1] + ├── stats: [rows=1, distinct(7)=1, null(7)=0] + ├── cost: 5.07 + ├── key: () + ├── fd: ()-->(7-9) + └── prune: (8,9) + + # createTableOp gist-explain-roundtrip CREATE TABLE t1 (x int) diff --git a/pkg/sql/opt/exec/explain/testdata/gists_tpce b/pkg/sql/opt/exec/explain/testdata/gists_tpce index 4e3f741bf4b8..6152776af388 100644 --- a/pkg/sql/opt/exec/explain/testdata/gists_tpce +++ b/pkg/sql/opt/exec/explain/testdata/gists_tpce @@ -210,8 +210,8 @@ update_trade_submitted AS ( ) SELECT * FROM request_list; ---- -hash: 7096273538769246907 -plan-gist: AgGkAQIAHwIAAAcQBRAhpAEAAAcCMAGUAQIAHwAAAAMHCDAxBQIUAJQBAgIBBQgHCAUII5QBAAcCMDEFAgcGBQYwH5IBADEFAhQFkAECAgEqMQUCFAWwAQICASoHAjAxBQIUAJABAgIBBRwHIAUgMCGQAQAAMQUCFAWwAQICASoHAjAxBQgGCA== +hash: 14329018118666014305 +plan-gist: AgGkAQIAHwIAAAcQBRAhpAEAAAcCMAGUAQIAHwAAAAMHCDAxBQIUAJQBAgIBBQgHCAUII5QBAAAHAjAxBQIHBgUGMB+SAQAxBQIUBZABAgIBKjEFAhQFsAECAgEqBwIwMQUCFACQAQICAQUcByAFIDAhkAEAADEFAhQFsAECAgEqBwIwMQUIBgg= explain(shape): • root │ diff --git a/pkg/sql/opt/exec/factory.opt b/pkg/sql/opt/exec/factory.opt index e655a7f5deda..97efd738b4f0 100644 --- a/pkg/sql/opt/exec/factory.opt +++ b/pkg/sql/opt/exec/factory.opt @@ -556,11 +556,17 @@ define Upsert { # The fetchCols set contains the ordinal positions of the fetch columns in # the target table. The input must contain those columns in the same order # as they appear in the table schema. +# +# The passthrough parameter contains all the result columns that are part of +# the input node that the update node needs to return (passing through from +# the input). The pass through columns are used to return any column from the +# USING tables that are referenced in the RETURNING clause. define Delete { Input exec.Node Table cat.Table FetchCols exec.TableColumnOrdinalSet ReturnCols exec.TableColumnOrdinalSet + Passthrough colinfo.ResultColumns # If set, the operator will commit the transaction as part of its execution. # This is false when executing inside an explicit transaction, or there are diff --git a/pkg/sql/opt_exec_factory.go b/pkg/sql/opt_exec_factory.go index 9e6c223c9b7a..f8aaf9a6d48e 100644 --- a/pkg/sql/opt_exec_factory.go +++ b/pkg/sql/opt_exec_factory.go @@ -1669,6 +1669,7 @@ func (ef *execFactory) ConstructDelete( table cat.Table, fetchColOrdSet exec.TableColumnOrdinalSet, returnColOrdSet exec.TableColumnOrdinalSet, + passthrough colinfo.ResultColumns, autoCommit bool, ) (exec.Node, error) { // Derive table and column descriptors. @@ -1697,6 +1698,7 @@ func (ef *execFactory) ConstructDelete( run: deleteRun{ td: tableDeleter{rd: rd, alloc: ef.getDatumAlloc()}, partialIndexDelValsOffset: len(rd.FetchCols), + numPassthrough: len(passthrough), }, } @@ -1707,6 +1709,9 @@ func (ef *execFactory) ConstructDelete( // order they are defined in the table. del.columns = colinfo.ResultColumnsFromColumns(tabDesc.GetID(), returnCols) + // Add the passthrough columns to the returning columns. + del.columns = append(del.columns, passthrough...) + del.run.rowIdxToRetIdx = row.ColMapping(rd.FetchCols, returnCols) del.run.rowsNeeded = true }