Skip to content

Commit

Permalink
sql: add support for DELETE FROM ... USING to optbuilder
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
faizaanmadhani committed Oct 3, 2022
1 parent b860d48 commit 1823ff3
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 22 deletions.
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/delete
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -337,3 +334,6 @@ SELECT * FROM a AS OF SYSTEM TIME $ts
3
4
5

# Test that USING works

7 changes: 4 additions & 3 deletions pkg/sql/opt/ops/mutation.opt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/opt/optbuilder/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
56 changes: 41 additions & 15 deletions pkg/sql/opt/optbuilder/mutation_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -375,11 +376,12 @@ func (mb *mutationBuilder) buildInputForUpdate(
// buildInputForDelete constructs a Select expression from the fields in
// the Delete operator, similar to this:
//
// SELECT <cols>
// FROM <table>
// WHERE <where>
// ORDER BY <order-by>
// LIMIT <limit>
// SELECT <cols>
// FROM <table>
// USING <tables>
// WHERE <where>
// ORDER BY <order-by>
// LIMIT <limit>
//
// All columns from the table to update are added to fetchColList.
// TODO(andyk): Do needed column analysis to project fewer columns if possible.
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
190 changes: 190 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/delete
Original file line number Diff line number Diff line change
Expand Up @@ -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.
# ------------------------------------------------------------------------------
Expand Down Expand Up @@ -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: <none>
├── 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: <none>
├── 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: <none>
├── 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: <none>
├── 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
2 changes: 1 addition & 1 deletion pkg/sql/sem/tree/pretty.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down

0 comments on commit 1823ff3

Please sign in to comment.