pingcap / pingcap/tidb

Deferred lock keys error might break the statement atomicity with foreign key trigger

Open
#70,489 1 comment 0 reactions 1 assignee Claimed by @YangKeao View on GitHub
affects-25.10 affects-26.3 affects-7.1 affects-7.5 affects-8.1 affects-8.5 severity/major sig/sql-infra type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

Please answer these questions before submitting your issue. Thanks!

### 1. Minimal reproduce step (Required)

Add the following reproducer to `pkg/executor/test/fktest/foreign_key_test.go`:

```go
func TestReproduceFKCascadeDeferredLockRetry(t *testing.T) {
store := testkit.CreateMockStore(t)
tk1 := testkit.NewTestKit(t, store)
tk2 := testkit.NewTestKit(t, store)

tk1.MustExec("set @@global.tidb_enable_foreign_key=1")
tk1.MustExec("set @@foreign_key_checks=1")
tk1.MustExec("use test")
tk2.MustExec("set @@foreign_key_checks=1")
tk2.MustExec("use test")

tk1.MustExec("create table fk_guard (id int primary key)")
tk1.MustExec("create table fk_parent (" +
"id int primary key, guard_id int, v int, " +
"foreign key (guard_id) references fk_guard(id))")
tk1.MustExec("create table fk_child (" +
"id int primary key, parent_id int, " +
"foreign key (parent_id) references fk_parent(id) on update cascade)")
tk1.MustExec("insert into fk_guard values (1), (2)")
tk1.MustExec("insert into fk_parent values (1, 1, 0)")
tk1.MustExec("insert into fk_child values (1, 1)")

tk2.MustExec("begin pessimistic")
tk2.MustQuery("select * from fk_guard where id = 2 for update").Check(testkit.Rows("2"))

tk1.MustExec("begin pessimistic")
done := make(chan error, 1)
go func() {
done <- tk1.ExecToErr("update fk_parent set id = 2, guard_id = 2, v = v + 1 where id = 1")
}()

time.Sleep(200 * time.Millisecond)
tk2.MustExec("delete from fk_guard where id = 2")
tk2.MustExec("commit")

require.NoError(t, <-done)
tk1.MustQuery("select * from fk_parent order by id").Check(testkit.Rows("2 2 1"))
tk1.MustQuery("select * from fk_child order by id").Check(testkit.Rows("1 2"))
tk1.MustExec("commit")

tk2.MustQuery("select * from fk_guard order by id").Check(testkit.Rows("1"))
tk2.MustQuery("select * from fk_parent order by id").Check(testkit.Rows("2 2 1"))
tk2.MustQuery("select * from fk_child order by id").Check(testkit.Rows("1 2"))
}
```

Then run the test with failpoint

On the affected version, the test passes and demonstrates that TiDB can commit `fk_parent.guard_id = 2` even though `fk_guard(id = 2)` has been deleted.

### 2. What did you expect to see? (Required)

The update should not be able to commit data that violates the foreign key constraint.

After `fk_guard(id = 2)` is deleted by the concurrent pessimistic transaction, the retried update should observe that the referenced row no longer exists and fail with a foreign key violation. The writes from the failed
attempt, including FK cascade writes, should be rolled back before retrying.

### 3. What did you see instead (Required)

The update returns success and the transaction commits successfully.

The final data violates the foreign key constraint:

```text
fk_guard:
1

fk_parent:
2 2 1

fk_child:
1 2
```

`fk_parent.guard_id = 2` references a missing row in `fk_guard`.

A likely cause is that FK-related deferred locks are acquired after the main DML and cascade writes have already been flushed into the transaction mem-buffer. When the deferred lock hits a conflict and the statement
retries, the retry rollback does not clean all writes from the previous FK/cascade attempt, so dirty in-transaction state can be committed.

### 4. What is your TiDB version? (Required)

Reproduced on current master build:

```text
Git Commit Hash: 38ff90883244229032900b0fe35cc44e3c82943a
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.