From 1823ff350e686c3166ce84b8b70a4cd36ce75de0 Mon Sep 17 00:00:00 2001 From: Faizaan Madhani Date: Wed, 28 Sep 2022 18:10:13 -0400 Subject: [PATCH] sql: add support for `DELETE FROM ... USING` to optbuilder Previously, the optbuilder would return an error when given sql statements of the form `DELETE FROM USING`. this commit adds support to the Optbuilder to build query plans for statements of the form `DELETE FROM ... USING`. Release note (sql change): Optbuilder will not build query plans for sql statements of the form `DELETE FROM ... USING`. --- pkg/sql/logictest/testdata/logic_test/delete | 6 +- pkg/sql/opt/ops/mutation.opt | 7 +- pkg/sql/opt/optbuilder/delete.go | 5 + pkg/sql/opt/optbuilder/mutation_builder.go | 56 ++++-- pkg/sql/opt/optbuilder/testdata/delete | 190 +++++++++++++++++++ pkg/sql/sem/tree/pretty.go | 2 +- 6 files changed, 244 insertions(+), 22 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/delete b/pkg/sql/logictest/testdata/logic_test/delete index 719d5dc5721b..eb5467b0b756 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,6 @@ SELECT * FROM a AS OF SYSTEM TIME $ts 3 4 5 + +# Test that USING works + diff --git a/pkg/sql/opt/ops/mutation.opt b/pkg/sql/opt/ops/mutation.opt index 734e1ca0efc8..4d08fd33e719 100644 --- a/pkg/sql/opt/ops/mutation.opt +++ b/pkg/sql/opt/ops/mutation.opt @@ -162,9 +162,10 @@ define MutationPrivate { # PassthroughCols are columns that the mutation needs to passthrough from # its input. It's similar to the passthrough columns in projections. This - # is useful for `UPDATE .. FROM` mutations where the `RETURNING` clause - # references columns from tables in the `FROM` clause. When this happens - # the update will need to pass through those refenced columns from its input. + # is useful for `UPDATE .. FROM` and `DELETE ... USING` mutations where the + # `RETURNING` clause references columns from tables in the `FROM` or `USING` + # clause, respectively. When this happens the mutation will need to pass through + # those refenced columns from its input. PassthroughCols ColList # Mutation operators can act similarly to a With operator: they buffer their diff --git a/pkg/sql/opt/optbuilder/delete.go b/pkg/sql/opt/optbuilder/delete.go index 16d9cbcb3f4d..834eee850a8c 100644 --- a/pkg/sql/opt/optbuilder/delete.go +++ b/pkg/sql/opt/optbuilder/delete.go @@ -83,6 +83,11 @@ func (mb *mutationBuilder) buildDelete(returning tree.ReturningExprs) { mb.projectPartialIndexDelCols() private := mb.makeMutationPrivate(returning != nil) + for _, col := range mb.extraAccessibleCols { + if col.id != 0 { + private.PassthroughCols = append(private.PassthroughCols, col.id) + } + } mb.outScope.expr = mb.b.factory.ConstructDelete( mb.outScope.expr, mb.uniqueChecks, mb.fkChecks, private, ) diff --git a/pkg/sql/opt/optbuilder/mutation_builder.go b/pkg/sql/opt/optbuilder/mutation_builder.go index 5dba82f2f80e..6cbefb4212e0 100644 --- a/pkg/sql/opt/optbuilder/mutation_builder.go +++ b/pkg/sql/opt/optbuilder/mutation_builder.go @@ -184,8 +184,9 @@ type mutationBuilder struct { // extraAccessibleCols stores all the columns that are available to the // mutation that are not part of the target table. This is useful for - // UPDATE ... FROM queries, as the columns from the FROM tables must be - // made accessible to the RETURNING clause. + // UPDATE ... FROM queries and DELETE ... USING queries, as the columns + // from the FROM and USING tables must be made accessible to the + // RETURNING clause, respectively. extraAccessibleCols []scopeColumn // fkCheckHelper is used to prevent allocating the helper separately. @@ -375,11 +376,12 @@ func (mb *mutationBuilder) buildInputForUpdate( // buildInputForDelete constructs a Select expression from the fields in // the Delete operator, similar to this: // -// SELECT -// FROM -// WHERE -// ORDER BY -// LIMIT +// SELECT +// FROM
+// USING +// WHERE +// ORDER BY +// LIMIT // // All columns from the table to update are added to fetchColList. // TODO(andyk): Do needed column analysis to project fewer columns if possible. @@ -418,7 +420,35 @@ func (mb *mutationBuilder) buildInputForDelete( inScope, false, /* disableNotVisibleIndex */ ) - mb.outScope = mb.fetchScope + + // USING + usingClausePresent := len(using) > 0 + if usingClausePresent { + + usingScope := mb.b.buildFromTables(using, noRowLocking, inScope) + + // Check that the same table name is not used multiple times + mb.b.validateJoinTableNames(mb.fetchScope, usingScope) + + mb.extraAccessibleCols = usingScope.cols + + // Add the columns to the USING scope. + // We create a new scope so that fetchScope is not modified + // as fetchScope contains the set of columns from the target + // table specified by USING. This will be used later with partial + // index predicate expressions and will prevent ambiguities with + // column names in the USING clause. + mb.outScope = mb.fetchScope.replace() + mb.outScope.appendColumnsFromScope(mb.fetchScope) + mb.outScope.appendColumnsFromScope(usingScope) + + left := mb.fetchScope.expr + right := usingScope.expr + + mb.outScope.expr = mb.b.factory.ConstructInnerJoin(left, right, memo.TrueFilter, memo.EmptyJoinPrivate) + } else { + mb.outScope = mb.fetchScope + } // WHERE mb.b.buildWhere(where, mb.outScope) @@ -430,11 +460,6 @@ func (mb *mutationBuilder) buildInputForDelete( mb.b.buildOrderBy(mb.outScope, projectionsScope, orderByScope) mb.b.constructProjectForScope(mb.outScope, projectionsScope) - // USING - if using != nil { - panic("DELETE USING is unimplemented so should not be used") - } - // LIMIT if limit != nil { mb.b.buildLimit(limit, inScope, projectionsScope) @@ -1011,8 +1036,9 @@ func (mb *mutationBuilder) buildReturning(returning tree.ReturningExprs) { // extraAccessibleCols contains all the columns that the RETURNING // clause can refer to in addition to the table columns. This is useful for - // UPDATE ... FROM statements, where all columns from tables in the FROM clause - // are in scope for the RETURNING clause. + // UPDATE ... FROM and DELETE ... USING statements, where all columns from + // tables in the FROM clause and USING clause are in scope for the RETURNING + // clause, respectively. inScope.appendColumns(mb.extraAccessibleCols) // Construct the Project operator that projects the RETURNING expressions. diff --git a/pkg/sql/opt/optbuilder/testdata/delete b/pkg/sql/opt/optbuilder/testdata/delete index f507f1ac9b63..31d8ae6058ed 100644 --- a/pkg/sql/opt/optbuilder/testdata/delete +++ b/pkg/sql/opt/optbuilder/testdata/delete @@ -32,6 +32,29 @@ CREATE TABLE mutation ( ) ---- +exec-ddl +CREATE TABLE u_a ( + a INT NOT NULL PRIMARY KEY, + b STRING, + c INT +) +---- + +exec-ddl +CREATE TABLE u_b ( + a INT NOT NULL PRIMARY KEY, + b STRING +) +---- + +exec-ddl +CREATE TABLE u_c ( + a INT NOT NULL PRIMARY KEY, + b STRING, + c INT +) +---- + # ------------------------------------------------------------------------------ # Basic tests. # ------------------------------------------------------------------------------ @@ -427,3 +450,170 @@ build DELETE FROM mutation ORDER BY p LIMIT 2 ---- error (42P10): column "p" is being backfilled + +# ------------------------------------------------------------------------------ +# Test USING. +# ------------------------------------------------------------------------------ + +# Test a simple join with a filter +build +DELETE FROM u_a USING u_b WHERE c = u_b.a AND u_b.b = 'd' +---- +delete u_a + ├── columns: + ├── fetch columns: u_b.a:11 u_b.b:12 c:8 + └── select + ├── columns: u_a.a:6!null u_a.b:7 c:8!null u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_b.a:11!null u_b.b:12!null u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + ├── inner-join (cross) + │ ├── columns: u_a.a:6!null u_a.b:7 c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + │ ├── scan u_a + │ │ └── columns: u_a.a:6!null u_a.b:7 c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 + │ ├── scan u_b + │ │ └── columns: u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + │ └── filters (true) + └── filters + └── (c:8 = u_b.a:11) AND (u_b.b:12 = 'd') + +# Test a self join +build +DELETE FROM u_a USING u_a u_a2 WHERE u_a.a = u_a2.c +---- +delete u_a + ├── columns: + ├── fetch columns: u_a2.a:11 u_a2.b:12 u_a2.c:13 + └── select + ├── columns: u_a.a:6!null u_a.b:7 u_a.c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_a2.a:11!null u_a2.b:12 u_a2.c:13!null u_a2.crdb_internal_mvcc_timestamp:14 u_a2.tableoid:15 + ├── inner-join (cross) + │ ├── columns: u_a.a:6!null u_a.b:7 u_a.c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_a2.a:11!null u_a2.b:12 u_a2.c:13 u_a2.crdb_internal_mvcc_timestamp:14 u_a2.tableoid:15 + │ ├── scan u_a + │ │ └── columns: u_a.a:6!null u_a.b:7 u_a.c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 + │ ├── scan u_a [as=u_a2] + │ │ └── columns: u_a2.a:11!null u_a2.b:12 u_a2.c:13 u_a2.crdb_internal_mvcc_timestamp:14 u_a2.tableoid:15 + │ └── filters (true) + └── filters + └── u_a.a:6 = u_a2.c:13 + +# Test when USING uses multiple tables +build +DELETE FROM u_a USING u_b, u_c WHERE u_a.c = u_b.a AND u_a.c = u_c.a +---- +delete u_a + ├── columns: + ├── fetch columns: u_c.a:15 u_c.b:16 u_c.c:17 + └── select + ├── columns: u_a.a:6!null u_a.b:7 u_a.c:8!null u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 u_c.a:15!null u_c.b:16 u_c.c:17 u_c.crdb_internal_mvcc_timestamp:18 u_c.tableoid:19 + ├── inner-join (cross) + │ ├── columns: u_a.a:6!null u_a.b:7 u_a.c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 u_c.a:15!null u_c.b:16 u_c.c:17 u_c.crdb_internal_mvcc_timestamp:18 u_c.tableoid:19 + │ ├── scan u_a + │ │ └── columns: u_a.a:6!null u_a.b:7 u_a.c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 + │ ├── inner-join (cross) + │ │ ├── columns: u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 u_c.a:15!null u_c.b:16 u_c.c:17 u_c.crdb_internal_mvcc_timestamp:18 u_c.tableoid:19 + │ │ ├── scan u_b + │ │ │ └── columns: u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + │ │ ├── scan u_c + │ │ │ └── columns: u_c.a:15!null u_c.b:16 u_c.c:17 u_c.crdb_internal_mvcc_timestamp:18 u_c.tableoid:19 + │ │ └── filters (true) + │ └── filters (true) + └── filters + └── (u_a.c:8 = u_b.a:11) AND (u_a.c:8 = u_c.a:15) + +# Test if USING works well with RETURNING expressions that reference +# the USING table +build +DELETE FROM + u_a +USING + u_c +WHERE + u_a.c > u_c.a AND u_c.c <= 4 +RETURNING + u_c.a, u_c.b, u_c.c +---- +project + ├── columns: a:11 b:12 c:13 + └── delete u_a + ├── columns: u_a.a:1!null u_a.b:2 u_a.c:3!null u_c.a:11 u_c.b:12 u_c.c:13 u_c.crdb_internal_mvcc_timestamp:14 u_c.tableoid:15 + ├── fetch columns: u_c.a:11 u_c.b:12 u_c.c:13 + └── select + ├── columns: u_a.a:6!null u_a.b:7 u_a.c:8!null u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_c.a:11!null u_c.b:12 u_c.c:13!null u_c.crdb_internal_mvcc_timestamp:14 u_c.tableoid:15 + ├── inner-join (cross) + │ ├── columns: u_a.a:6!null u_a.b:7 u_a.c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_c.a:11!null u_c.b:12 u_c.c:13 u_c.crdb_internal_mvcc_timestamp:14 u_c.tableoid:15 + │ ├── scan u_a + │ │ └── columns: u_a.a:6!null u_a.b:7 u_a.c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 + │ ├── scan u_c + │ │ └── columns: u_c.a:11!null u_c.b:12 u_c.c:13 u_c.crdb_internal_mvcc_timestamp:14 u_c.tableoid:15 + │ └── filters (true) + └── filters + └── (u_a.c:8 > u_c.a:11) AND (u_c.c:13 <= 4) + +# Test if RETURNING * returns everything +build +DELETE FROM u_a USING u_b WHERE c = u_b.a AND u_b.b = 'd' RETURNING * +---- +project + ├── columns: a:1!null b:2!null c:3!null a:11 b:12 + └── delete u_a + ├── columns: u_a.a:1!null u_a.b:2!null c:3!null u_b.a:11 u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + ├── fetch columns: u_b.a:11 u_b.b:12 c:8 + └── select + ├── columns: u_a.a:6!null u_a.b:7 c:8!null u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_b.a:11!null u_b.b:12!null u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + ├── inner-join (cross) + │ ├── columns: u_a.a:6!null u_a.b:7 c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + │ ├── scan u_a + │ │ └── columns: u_a.a:6!null u_a.b:7 c:8 u_a.crdb_internal_mvcc_timestamp:9 u_a.tableoid:10 + │ ├── scan u_b + │ │ └── columns: u_b.a:11!null u_b.b:12 u_b.crdb_internal_mvcc_timestamp:13 u_b.tableoid:14 + │ └── filters (true) + └── filters + └── (c:8 = u_b.a:11) AND (u_b.b:12 = 'd') + +# Test aliased table names and sorting +build +DELETE FROM abcde AS foo USING xyz AS bar WHERE bar.y > 0 ORDER BY bar.y LIMIT 5 +---- +delete abcde [as=foo] + ├── columns: + ├── fetch columns: x:17 y:18 z:19 d:12 e:13 rowid:14 + └── limit + ├── columns: a:9!null b:10 c:11 d:12 e:13 rowid:14!null foo.crdb_internal_mvcc_timestamp:15 foo.tableoid:16 x:17!null y:18!null z:19 bar.crdb_internal_mvcc_timestamp:20 bar.tableoid:21 + ├── internal-ordering: +18 + ├── sort + │ ├── columns: a:9!null b:10 c:11 d:12 e:13 rowid:14!null foo.crdb_internal_mvcc_timestamp:15 foo.tableoid:16 x:17!null y:18!null z:19 bar.crdb_internal_mvcc_timestamp:20 bar.tableoid:21 + │ ├── ordering: +18 + │ ├── limit hint: 5.00 + │ └── select + │ ├── columns: a:9!null b:10 c:11 d:12 e:13 rowid:14!null foo.crdb_internal_mvcc_timestamp:15 foo.tableoid:16 x:17!null y:18!null z:19 bar.crdb_internal_mvcc_timestamp:20 bar.tableoid:21 + │ ├── inner-join (cross) + │ │ ├── columns: a:9!null b:10 c:11 d:12 e:13 rowid:14!null foo.crdb_internal_mvcc_timestamp:15 foo.tableoid:16 x:17!null y:18 z:19 bar.crdb_internal_mvcc_timestamp:20 bar.tableoid:21 + │ │ ├── scan abcde [as=foo] + │ │ │ ├── columns: a:9!null b:10 c:11 d:12 e:13 rowid:14!null foo.crdb_internal_mvcc_timestamp:15 foo.tableoid:16 + │ │ │ └── computed column expressions + │ │ │ ├── d:12 + │ │ │ │ └── (b:10 + c:11) + 1 + │ │ │ └── e:13 + │ │ │ └── a:9 + │ │ ├── scan xyz [as=bar] + │ │ │ └── columns: x:17!null y:18 z:19 bar.crdb_internal_mvcc_timestamp:20 bar.tableoid:21 + │ │ └── filters (true) + │ └── filters + │ └── y:18 > 0 + └── 5 + +# Test if DELETE FROM ... USING can return hidden columns +build +DELETE FROM + u_a +USING + u_b, u_c +WHERE + u_a.c >= u_b.a +AND + u_a.c <= u_c.a +RETURNING + *, u_b.rowid, u_c.rowid +---- +error (42703): column "u_b.rowid" does not exist + +# Test if DELETE FROM ... USING works with LATERAL + +# Test if DELETE FROM ... USING works with partial indexes diff --git a/pkg/sql/sem/tree/pretty.go b/pkg/sql/sem/tree/pretty.go index 3a0fbc2f44ba..b56c73517a72 100644 --- a/pkg/sql/sem/tree/pretty.go +++ b/pkg/sql/sem/tree/pretty.go @@ -1161,7 +1161,7 @@ func (node *Update) doc(p *PrettyCfg) pretty.Doc { } func (node *Delete) doc(p *PrettyCfg) pretty.Doc { - items := make([]pretty.TableRow, 0, 6) + items := make([]pretty.TableRow, 0, 7) items = append(items, node.With.docRow(p), p.row("DELETE FROM", p.Doc(node.Table)))