Skip to content

Commit bb857aa

Browse files
committed
sql: add implicit SELECT FOR SHARE locking to FK parent checks
Add SELECT FOR SHARE locking to FK parent checks. Under serializable isolation, this locking is only used when `enable_implicit_fk_locking_for_serializable` is set. Under weaker isolation levels (snapshot and read committed) this locking is always used. We only need to lock during the insertion-side FK checks, which verify the existence of a parent row. Deletion-side FK checks verify the non-existence of a child row, and these do not need to lock. Instead, to prevent concurrent inserts or updates to the child that would violate the FK constraint, we rely on the intent(s) created by the deletion conflicting with the FK locking of those concurrent inserts or updates. Fixes: cockroachdb#80683 Informs: cockroachdb#100156 Epic: CRDB-25322 Release note (sql change): Add a new session variable, `enable_implicit_fk_locking_for_serializable`, which controls locking during foreign key checks under serializable isolation. With this set to true, foreign key checks of the referenced (parent) table, such as those performed during an INSERT or UPDATE of the referencing (child) table, will lock the referenced row using SELECT FOR SHARE locking. (This is somewhat analogous to the existing `enable_implicit_select_for_update` variable but applies to the foreign key checks of a mutation statement instead of the initial row fetch.) Under weaker isolation levels such as read committed, SELECT FOR SHARE locking will always be used to ensure the database maintains the foreign key constraint, regardless of the current setting of `enable_implicit_fk_locking_for_serializable`.
1 parent 03a8d4a commit bb857aa

File tree

32 files changed

+1137
-27
lines changed

32 files changed

+1137
-27
lines changed

pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/sql/exec_util.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3612,6 +3612,10 @@ func (m *sessionDataMutator) SetOptimizerUseImprovedJoinElimination(val bool) {
36123612
m.data.OptimizerUseImprovedJoinElimination = val
36133613
}
36143614

3615+
func (m *sessionDataMutator) SetImplicitFKLockingForSerializable(val bool) {
3616+
m.data.ImplicitFKLockingForSerializable = val
3617+
}
3618+
36153619
// Utility functions related to scrubbing sensitive information on SQL Stats.
36163620

36173621
// quantizeCounts ensures that the Count field in the

pkg/sql/insert_fast_path.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"sync"
1616

1717
"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
18+
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
1819
"github.com/cockroachdb/cockroach/pkg/roachpb"
1920
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
2021
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
@@ -188,10 +189,20 @@ func (r *insertFastPathRun) addFKChecks(
188189
if r.traceKV {
189190
log.VEventf(ctx, 2, "FKScan %s", span)
190191
}
192+
lockStrength := row.GetKeyLockingStrength(descpb.ToScanLockingStrength(c.Locking.Strength))
193+
lockWaitPolicy := row.GetWaitPolicy(descpb.ToScanLockingWaitPolicy(c.Locking.WaitPolicy))
194+
if r.fkBatch.Header.WaitPolicy != lockWaitPolicy {
195+
return errors.AssertionFailedf(
196+
"FK check lock wait policy %s did not match %s",
197+
lockWaitPolicy, r.fkBatch.Header.WaitPolicy,
198+
)
199+
}
191200
reqIdx := len(r.fkBatch.Requests)
192201
r.fkBatch.Requests = append(r.fkBatch.Requests, kvpb.RequestUnion{})
193202
r.fkBatch.Requests[reqIdx].MustSetInner(&kvpb.ScanRequest{
194203
RequestHeader: kvpb.RequestHeaderFromSpan(span),
204+
KeyLocking: lockStrength,
205+
// TODO(michae2): Once #100193 is finished, also include c.Locking.Durability.
195206
})
196207
r.fkSpanInfo = append(r.fkSpanInfo, insertFastPathFKSpanInfo{
197208
check: c,
@@ -248,6 +259,8 @@ func (n *insertFastPathNode) startExec(params runParams) error {
248259
}
249260
}
250261
maxSpans := len(n.run.fkChecks) * len(n.input)
262+
// Any FK checks using locking should have lock wait policy BLOCK.
263+
n.run.fkBatch.Header.WaitPolicy = lock.WaitPolicy_Block
251264
n.run.fkBatch.Requests = make([]kvpb.RequestUnion, 0, maxSpans)
252265
n.run.fkSpanInfo = make([]insertFastPathFKSpanInfo, 0, maxSpans)
253266
if len(n.input) > 1 {
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# LogicTest: !local-mixed-22.2-23.1
2+
3+
# Some foreign key checks are prohibited under weaker isolation levels until we
4+
# improve locking. See #80683, #100156, #100193.
5+
6+
statement ok
7+
CREATE TABLE jars (j INT PRIMARY KEY)
8+
9+
statement ok
10+
CREATE TABLE cookies (c INT PRIMARY KEY, j INT REFERENCES jars (j))
11+
12+
statement ok
13+
SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL READ COMMITTED
14+
15+
statement ok
16+
INSERT INTO jars VALUES (1), (2)
17+
18+
# Foreign key checks of the parent require durable shared locking under weaker
19+
# isolation levels, and are not yet supported.
20+
query error pgcode 0A000 guaranteed-durable locking not yet implemented
21+
INSERT INTO cookies VALUES (1, 1)
22+
23+
statement ok
24+
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE
25+
26+
statement ok
27+
INSERT INTO cookies VALUES (1, 1)
28+
29+
statement ok
30+
COMMIT
31+
32+
query error pgcode 0A000 guaranteed-durable locking not yet implemented
33+
UPDATE cookies SET j = 2 WHERE c = 1
34+
35+
# Foreign key checks of the child do not require locking.
36+
query error violates foreign key constraint
37+
UPDATE jars SET j = j + 4
38+
39+
query error violates foreign key constraint
40+
DELETE FROM jars WHERE j = 1
41+
42+
statement ok
43+
DELETE FROM cookies WHERE c = 1
44+
45+
statement ok
46+
DELETE FROM jars WHERE j = 1

pkg/sql/logictest/testdata/logic_test/information_schema

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5289,6 +5289,7 @@ enable_auto_rehoming off
52895289
enable_create_stats_using_extremes off
52905290
enable_drop_enum_value on
52915291
enable_experimental_alter_column_type_general off
5292+
enable_implicit_fk_locking_for_serializable off
52925293
enable_implicit_select_for_update on
52935294
enable_implicit_transaction_for_batch_statements on
52945295
enable_insert_fast_path on

pkg/sql/logictest/testdata/logic_test/pg_catalog

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2727,6 +2727,7 @@ distsql off N
27272727
enable_auto_rehoming off NULL NULL NULL string
27282728
enable_create_stats_using_extremes off NULL NULL NULL string
27292729
enable_experimental_alter_column_type_general off NULL NULL NULL string
2730+
enable_implicit_fk_locking_for_serializable off NULL NULL NULL string
27302731
enable_implicit_select_for_update on NULL NULL NULL string
27312732
enable_implicit_transaction_for_batch_statements on NULL NULL NULL string
27322733
enable_insert_fast_path on NULL NULL NULL string
@@ -2887,6 +2888,7 @@ distsql off N
28872888
enable_auto_rehoming off NULL user NULL off off
28882889
enable_create_stats_using_extremes off NULL user NULL off off
28892890
enable_experimental_alter_column_type_general off NULL user NULL off off
2891+
enable_implicit_fk_locking_for_serializable off NULL user NULL off off
28902892
enable_implicit_select_for_update on NULL user NULL on on
28912893
enable_implicit_transaction_for_batch_statements on NULL user NULL on on
28922894
enable_insert_fast_path on NULL user NULL on on
@@ -3044,6 +3046,7 @@ distsql_workmem NULL NULL NULL
30443046
enable_auto_rehoming NULL NULL NULL NULL NULL
30453047
enable_create_stats_using_extremes NULL NULL NULL NULL NULL
30463048
enable_experimental_alter_column_type_general NULL NULL NULL NULL NULL
3049+
enable_implicit_fk_locking_for_serializable NULL NULL NULL NULL NULL
30473050
enable_implicit_select_for_update NULL NULL NULL NULL NULL
30483051
enable_implicit_transaction_for_batch_statements NULL NULL NULL NULL NULL
30493052
enable_insert_fast_path NULL NULL NULL NULL NULL

pkg/sql/logictest/testdata/logic_test/read_committed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
subtest select_for_update
44

55
# SELECT FOR UPDATE is prohibited under weaker isolation levels until we improve
6-
# locking. See #57031, #75457, #100144.
6+
# locking. See #57031, #75457, #100144, #100193.
77

88
statement ok
99
CREATE TABLE supermarket (

pkg/sql/logictest/testdata/logic_test/show_source

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ distsql off
6262
enable_auto_rehoming off
6363
enable_create_stats_using_extremes off
6464
enable_experimental_alter_column_type_general off
65+
enable_implicit_fk_locking_for_serializable off
6566
enable_implicit_select_for_update on
6667
enable_implicit_transaction_for_batch_statements on
6768
enable_insert_fast_path on

pkg/sql/logictest/tests/fakedist-disk/generated_test.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/sql/logictest/tests/fakedist-vec-off/generated_test.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)