Skip to content

Commit

Permalink
Merge #111258
Browse files Browse the repository at this point in the history
111258: optbuilder: refactor semantic analysis of FOR UPDATE r=DrewKimball,msirek a=michae2

**optbuilder: a few minor tweaks to building of FOR UPDATE**

Make a few minor tweaks to semantic analysis of FOR UPDATE locking
clauses.

1. Disallow multiple FOR UPDATE clauses on parenthesized queries. We do
   not currently handle scopes of parenthesized queries correctly.
   Because of this, we disallow multiple ORDER BY and LIMIT clauses on
   parenthesized queries. The previous implementation of FOR UPDATE was
   simple enough that we could work around this, but the upcoming
   changes make it impossible to support.

2. Allow FOR UPDATE on statements with VALUES in the FROM list (but
   continue to disallow FOR UPDATE on VALUES directly). This matches
   Postgres.

Release note (sql change): Two minor changes to FOR UPDATE clauses:

1. Multiple FOR UPDATE clauses on fully-parenthesized queries are now
disallowed. For example, the following statements are now disallowed:

```
(SELECT 1 FOR UPDATE) FOR UPDATE;
SELECT * FROM ((SELECT 1 FOR UPDATE) FOR UPDATE) AS x;
```

whereas statements like the following are still allowed:

```
SELECT * FROM (SELECT 1 FOR UPDATE) AS x FOR UPDATE;
SELECT (SELECT 1 FOR UPDATE) FOR UPDATE;
```

This does not match PostgreSQL, which allows all of these, but does
match our behavior for ORDER BY and LIMIT.

2. FOR UPDATE is now allowed on statements with VALUES in the FROM list
or as a subquery. For example, the following statements are now allowed:

```
SELECT (VALUES (1)) FOR UPDATE;
SELECT * FROM (VALUES (1)) AS x FOR UPDATE;
```

Using FOR UPDATE directly on VALUES is still disallowed:

```
VALUES (1) FOR UPDATE;
(VALUES (1)) FOR UPDATE;
INSERT INTO t VALUES (1) FOR UPDATE;
```

This matches PostgreSQL.

**optbuilder: refactor semantic analysis of FOR UPDATE**

Locking clauses such as FOR UPDATE and FOR SHARE apply to some or all of
the data sources in a query's FROM list, depending on whether they have
targets (FOR UPDATE OF x). Without targets, they always apply within
subqueries in the FROM list. With targets, they apply within subqueries
if the subquery alias matches the target.

Because of this scope-like nature of FOR UPDATE and FOR SHARE, we
implement semantic analysis using a stack of locking items that grow as
we build inner subqueries deeper in the recursive optbuilder calls.

Prior to this change, we only used the stack of locking items during
buildScan, at the very bottom of the recursion. Because of this, calls
to `lockingSpec.filter` could afford to compress the stack into a single
locking item on our way deeper in the recursion.

As part of the upcoming fix for #75457, however, we will need to build a
new Lock operator when popping off locking items after returning from
the recursion. That Lock operator will need some information gathered
from buildScan at the bottom of the recursion.

To support this, we refactor the stack of locking items to be two
stacks: one that tracks all locking items in scope, and a second that
tracks which locking items currently apply. This will allow buildScan to
associate table information with the correct locking item(s), which can
then be used to build Lock operators when popping the locking items.

As a bonus, by using only the applied locking item stack in
`validateLockingInFrom` we can make validation of SELECT FOR UPDATE
queries a little more precise, which allows some queries we were
incorrectly disallowing.

Informs: #57031, #75457

Epic: CRDB-25322

Release note (sql change): Allow FOR UPDATE on some queries that were
previously disallowed. Queries that use the following operations are now
allowed to have FOR UPDATE OF as long as the prohibited operation is in
a subquery not locked by the FOR UPDATE OF:

- UNION
- INTERSECT
- EXCEPT
- DISTINCT
- GROUP BY
- HAVING
- aggregations
- window functions

For example, the following query is now allowed because the subquery
using the prohibited operations is not affected by the FOR UPDATE OF:

```
SELECT * FROM t,
  (SELECT DISTINCT 0, 0 UNION SELECT a, count(*) FROM t GROUP BY a HAVING a > 0) AS u
  FOR UPDATE OF t;
```

This matches PostgreSQL.

Co-authored-by: Michael Erickson <michae2@cockroachlabs.com>
  • Loading branch information
craig[bot] and michae2 committed Oct 6, 2023
2 parents d926f1e + fb58047 commit 4ab430e
Show file tree
Hide file tree
Showing 10 changed files with 461 additions and 262 deletions.
134 changes: 134 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/select_for_update
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,29 @@ query I
----
1

# Unlike Postgres, we do not support multiple locking clauses on parenthesized
# queries.

statement error pgcode 42601 multiple FOR UPDATE clauses not allowed
(SELECT 1 FOR UPDATE) FOR UPDATE

statement error pgcode 42601 multiple FOR UPDATE clauses not allowed
((SELECT 1 FOR UPDATE)) FOR UPDATE

statement error pgcode 42601 multiple FOR UPDATE clauses not allowed
((SELECT 1) FOR UPDATE) FOR UPDATE

# But we do support locking clauses both inside and outside subqueries.

statement ok
SELECT (SELECT 1 FOR UPDATE) FOR UPDATE

statement ok
SELECT * FROM (SELECT 1 FOR UPDATE) AS x FOR UPDATE

statement ok
SELECT * FROM (SELECT 1 FOR UPDATE) AS x WHERE EXISTS (SELECT 1 FOR UPDATE) FOR UPDATE

# FOR READ ONLY is ignored, like in Postgres.
query I
SELECT 1 FOR READ ONLY
Expand All @@ -140,54 +163,159 @@ SELECT 1 FOR READ ONLY
# Various operations are not supported when locking clauses are provided.
# FeatureNotSupported errors are thrown for each of them.

statement ok
CREATE TABLE i (i PRIMARY KEY) AS SELECT 1

statement error pgcode 0A000 FOR UPDATE is not allowed with UNION/INTERSECT/EXCEPT
SELECT 1 UNION SELECT 1 FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with UNION/INTERSECT/EXCEPT
SELECT * FROM (SELECT 1 UNION SELECT 1) a FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with UNION/INTERSECT/EXCEPT
SELECT * FROM (SELECT 1 UNION SELECT 1) a, i FOR UPDATE

query II
SELECT * FROM (SELECT 1 UNION SELECT 1) a, i FOR UPDATE OF i
----
1 1

statement error pgcode 0A000 FOR UPDATE is not allowed with VALUES
VALUES (1) FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with VALUES
(VALUES (1)) FOR UPDATE

# VALUES within a subquery is allowed, like in Postgres.

query I
SELECT (VALUES (1)) FOR UPDATE
----
1

query I
SELECT * FROM (VALUES (1)) a FOR UPDATE
----
1

query II
SELECT * FROM (VALUES (1)) a, i FOR UPDATE
----
1 1

query II
SELECT * FROM (VALUES (1)) a, i FOR UPDATE OF a
----
1 1

query II
SELECT * FROM (VALUES (1)) a, i FOR UPDATE OF i
----
1 1

statement error pgcode 0A000 FOR UPDATE is not allowed with DISTINCT clause
SELECT DISTINCT 1 FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with DISTINCT clause
SELECT * FROM (SELECT DISTINCT 1) a FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with DISTINCT clause
SELECT * FROM (SELECT DISTINCT 1) a, i FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with DISTINCT clause
SELECT * FROM (SELECT DISTINCT 1) a, i FOR UPDATE OF a

query II
SELECT * FROM (SELECT DISTINCT 1) a, i FOR UPDATE OF i
----
1 1

statement error pgcode 0A000 FOR UPDATE is not allowed with GROUP BY clause
SELECT 1 GROUP BY 1 FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with GROUP BY clause
SELECT * FROM (SELECT 1 GROUP BY 1) a FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with GROUP BY clause
SELECT * FROM (SELECT 1 GROUP BY 1) a, i FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with GROUP BY clause
SELECT * FROM (SELECT 1 GROUP BY 1) a, i FOR UPDATE OF a

query II
SELECT * FROM (SELECT 1 GROUP BY 1) a, i FOR UPDATE OF i
----
1 1

statement error pgcode 0A000 FOR UPDATE is not allowed with HAVING clause
SELECT 1 HAVING TRUE FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with HAVING clause
SELECT * FROM (SELECT 1 HAVING TRUE) a FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with HAVING clause
SELECT * FROM (SELECT 1 HAVING TRUE) a, i FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with HAVING clause
SELECT * FROM (SELECT 1 HAVING TRUE) a, i FOR UPDATE OF a

query II
SELECT * FROM (SELECT 1 HAVING TRUE) a, i FOR UPDATE OF i
----
1 1

statement error pgcode 0A000 FOR UPDATE is not allowed with aggregate functions
SELECT count(1) FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with aggregate functions
SELECT * FROM (SELECT count(1)) a FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with aggregate functions
SELECT * FROM (SELECT count(1)) a, i FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with aggregate functions
SELECT * FROM (SELECT count(1)) a, i FOR UPDATE OF a

query II
SELECT * FROM (SELECT count(1)) a, i FOR UPDATE OF i
----
1 1

statement error pgcode 0A000 FOR UPDATE is not allowed with window functions
SELECT count(1) OVER () FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with window functions
SELECT * FROM (SELECT count(1) OVER ()) a FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with window functions
SELECT * FROM (SELECT count(1) OVER ()) a, i FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with window functions
SELECT * FROM (SELECT count(1) OVER ()) a, i FOR UPDATE OF a

query II
SELECT * FROM (SELECT count(1) OVER ()) a, i FOR UPDATE OF i
----
1 1

statement error pgcode 0A000 FOR UPDATE is not allowed with set-returning functions in the target list
SELECT generate_series(1, 2) FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with set-returning functions in the target list
SELECT * FROM (SELECT generate_series(1, 2)) a FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with set-returning functions in the target list
SELECT * FROM (SELECT generate_series(1, 2)) a, i FOR UPDATE

statement error pgcode 0A000 FOR UPDATE is not allowed with set-returning functions in the target list
SELECT * FROM (SELECT generate_series(1, 2)) a, i FOR UPDATE OF a

query II nosort
SELECT * FROM (SELECT generate_series(1, 2)) a, i FOR UPDATE OF i
----
1 1
2 1

# Set-returning functions in the from list are allowed.
query I nosort
SELECT * FROM generate_series(1, 2) FOR UPDATE
Expand All @@ -201,6 +329,12 @@ SELECT * FROM (SELECT * FROM generate_series(1, 2)) a FOR UPDATE
1
2

query II nosort
SELECT * FROM (SELECT * FROM generate_series(1, 2)) a, i FOR UPDATE
----
1 1
2 1

# Use of SELECT FOR UPDATE/SHARE requires SELECT and UPDATE privileges.

statement ok
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/optbuilder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,10 @@ func (b *Builder) buildStmt(

switch stmt := stmt.(type) {
case *tree.Select:
return b.buildSelect(stmt, noRowLocking, desiredTypes, inScope)
return b.buildSelect(stmt, noLocking, desiredTypes, inScope)

case *tree.ParenSelect:
return b.buildSelect(stmt.Select, noRowLocking, desiredTypes, inScope)
return b.buildSelect(stmt.Select, noLocking, desiredTypes, inScope)

case *tree.Delete:
return b.processWiths(stmt.With, inScope, func(inScope *scope) *scope {
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/opt/optbuilder/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ import (
// See Builder.buildStmt for a description of the remaining input and
// return values.
func (b *Builder) buildJoin(
join *tree.JoinTableExpr, locking lockingSpec, inScope *scope,
join *tree.JoinTableExpr, lockCtx lockingContext, inScope *scope,
) (outScope *scope) {
leftScope := b.buildDataSource(join.Left, nil /* indexFlags */, locking, inScope)
leftScope := b.buildDataSource(join.Left, nil /* indexFlags */, lockCtx, inScope)

inScopeRight := inScope
isLateral := b.exprIsLateral(join.Right)
Expand All @@ -45,7 +45,7 @@ func (b *Builder) buildJoin(
inScopeRight.context = exprKindLateralJoin
}

rightScope := b.buildDataSource(join.Right, nil /* indexFlags */, locking, inScopeRight)
rightScope := b.buildDataSource(join.Right, nil /* indexFlags */, lockCtx, inScopeRight)

// Check that the same table name is not used on both sides.
b.validateJoinTableNames(leftScope, rightScope)
Expand Down
Loading

0 comments on commit 4ab430e

Please sign in to comment.