diff --git a/pkg/sql/delete.go b/pkg/sql/delete.go index 00134b08cd2f..b331c4a16fc7 100644 --- a/pkg/sql/delete.go +++ b/pkg/sql/delete.go @@ -50,8 +50,8 @@ type deleteRun struct { traceKV bool // partialIndexDelValsOffset is the offset of partial index delete - // indicators in the source values. It is equal to the number of fetched - // columns. + // indicators in the source values. It is equal to the sum of the number + // of fetched columns and the number of passthrough columns. partialIndexDelValsOffset int // rowIdxToRetIdx is the mapping from the columns returned by the deleter @@ -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{} @@ -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..f3d467529e8e 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,194 @@ 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 ok +INSERT INTO u_a VALUES (1, 'a', 10), (2, 'b', 20), (3, 'c', 30), (4, 'd', 40) + +statement ok +INSERT INTO u_b VALUES (10, 'a'), (20, 'b'), (30, 'c'), (40, 'd') + +statement ok +INSERT INTO u_c VALUES (1, 'a', 10), (2, 'b', 50), (3, 'c', 50), (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 ok +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 ok +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 ok +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 ok +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 ok +INSERT INTO u_d VALUES (1, 10), (2, 20), (3, 30), (4, 40) + +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), + INDEX (a) WHERE a > 3 +) + +statement ok +INSERT INTO pindex VALUES (1.0), (2.0), (3.0), (4.0), (5.0), (8.0) + +statement ok +DELETE FROM pindex USING (VALUES (5.0), (6.0)) v(b) WHERE pindex.a = v.b + +query F +SELECT * FROM pindex; +---- +1.00 +2.00 +3.00 +4.00 +8.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..dce9996092ec 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 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..7ef2830579a2 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. @@ -1696,7 +1697,8 @@ func (ef *execFactory) ConstructDelete( source: input.(planNode), run: deleteRun{ td: tableDeleter{rd: rd, alloc: ef.getDatumAlloc()}, - partialIndexDelValsOffset: len(rd.FetchCols), + partialIndexDelValsOffset: len(rd.FetchCols) + len(passthrough), + 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 }