sql: add support for DELETE FROM ... USING to execbuilder

Previously, while the optimizer would generate query plans
for sql statements of the form `DELETE FROM ... USING`
executing the statement in an instance of cockroachdb
may return errors, particularly with statements that
included `RETURNING` clauses. This commit adds support to
the execbuilder to execute statements of the form
`DELETE FROM ... USING`.

Release note (sql change): cockroachdb now supports
executing statements of the form `DELETE FROM ... USING`.

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
 		}

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")

pkg/sql/logictest/testdata/logic_test/cursor

@@ -598,4 +598,3 @@ FETCH 1 a b;
 
 statement ok
 COMMIT;
-

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 rowsort
+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

pkg/sql/logictest/testdata/logic_test/views

@@ -1492,4 +1492,3 @@ SELECT * FROM v;
 
 statement ok
 SET DATABASE = test;
-

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 {

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: <none>
+ ├── 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)