Skip to content

Commit

Permalink
opt: do not use LockDurabilityGuaranteed under serializable isolation
Browse files Browse the repository at this point in the history
This is a follow-up from cockroachdb#103734.

We do not want to use guaranteed-durable (a.k.a. replicated) locking
under serializable isolation, because it prevents pipelining and other
optimizations, and is unnecessary for correctness. This commit ammends
8cbc6d1 to only set durability for
`SELECT FOR UPDATE` locking under weaker isolation levels.

This means that query plans will be slightly different under different
isolation levels, and so we must add isolation level to the optimizer
memo staleness calculation.

Furthermore, this commit changes the error message added by
e633d5e to be about guaranteed-durable
locking rather than `SELECT FOR UPDATE`, because in a later commit this
specific error will also be triggered by foreign key checks under weaker
isolation levels.

Informs: cockroachdb#100144, cockroachdb#100156, cockroachdb#100193, cockroachdb#100194

Release note: None
  • Loading branch information
michae2 committed Jul 20, 2023
1 parent 759944b commit 243aeb0
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 172 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ COMMIT
statement ok
BEGIN TRANSACTION ISOLATION LEVEL READ COMMITTED

query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
SELECT aisle FROM supermarket WHERE person = 'matilda' FOR UPDATE

statement ok
Expand All @@ -49,7 +49,7 @@ SET CLUSTER SETTING sql.txn.snapshot_isolation.enabled = true
statement ok
BEGIN TRANSACTION ISOLATION LEVEL SNAPSHOT

query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under Snapshot isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
SELECT aisle FROM supermarket WHERE person = 'matilda' FOR UPDATE

statement ok
Expand All @@ -65,7 +65,7 @@ BEGIN TRANSACTION
statement ok
SET TRANSACTION ISOLATION LEVEL READ COMMITTED;

query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
UPDATE supermarket
SET aisle = (SELECT aisle FROM supermarket WHERE person = 'matilda' FOR UPDATE)
WHERE person = 'michael'
Expand All @@ -80,7 +80,7 @@ BEGIN TRANSACTION
statement ok
SET TRANSACTION ISOLATION LEVEL READ COMMITTED;

query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
WITH s AS
(SELECT aisle FROM supermarket WHERE person = 'matilda' FOR UPDATE)
SELECT aisle + 1 FROM s
Expand All @@ -98,7 +98,7 @@ CREATE FUNCTION wrangle (name STRING) RETURNS INT LANGUAGE SQL AS $$
$$

# But calling that function should fail.
query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
INSERT INTO supermarket (person, aisle) VALUES ('grandma', wrangle('matilda'))

statement ok
Expand All @@ -109,14 +109,14 @@ statement ok
PREPARE psa AS SELECT aisle FROM supermarket WHERE person = $1::STRING FOR UPDATE

# But execution should fail.
query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
EXECUTE psa('matilda')

statement ok
DEALLOCATE psa

# SELECT FOR UPDATE using a lookup join should also fail.
query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
WITH names AS MATERIALIZED
(SELECT 'matilda' AS person)
SELECT aisle
Expand All @@ -125,14 +125,14 @@ SELECT aisle
FOR UPDATE

# SELECT FOR UPDATE using an index join should also fail.
query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
SELECT aisle
FROM supermarket@supermarket_starts_with_idx
WHERE starts_with = 'm'
FOR UPDATE

# SELECT FOR UPDATE using a zigzag join should also fail.
query error pgcode 0A000 cannot execute SELECT FOR UPDATE statements under ReadCommitted isolation
query error pgcode 0A000 guaranteed-durable locking not yet implemented
SELECT aisle
FROM supermarket@{FORCE_ZIGZAG}
WHERE starts_with = 'm' AND ends_with = 'lda'
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/opt/exec/execbuilder/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder",
visibility = ["//visibility:public"],
deps = [
"//pkg/kv/kvserver/concurrency/isolation",
"//pkg/server/telemetry",
"//pkg/sql/catalog/colinfo",
"//pkg/sql/catalog/descpb",
Expand Down
7 changes: 2 additions & 5 deletions pkg/sql/opt/exec/execbuilder/relational.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"math"
"strings"

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -2896,11 +2895,9 @@ func (b *Builder) buildLocking(locking opt.Locking) (opt.Locking, error) {
"cannot execute %s in a read-only transaction", locking.Strength.String(),
)
}
if locking.Durability == tree.LockDurabilityGuaranteed &&
b.evalCtx.TxnIsoLevel != isolation.Serializable {
if locking.Durability == tree.LockDurabilityGuaranteed {
return opt.Locking{}, unimplemented.NewWithIssuef(
100144, "cannot execute SELECT FOR UPDATE statements under %s isolation",
b.evalCtx.TxnIsoLevel,
100193, "guaranteed-durable locking not yet implemented",
)
}
b.ContainsNonDefaultKeyLocking = true
Expand Down
Loading

0 comments on commit 243aeb0

Please sign in to comment.