pingcap / pingcap/tidb

executor: pessimistic unchanged-key locking mishandles partial and multi-valued unique indexes

Open
#71,212 2 comments 0 reactions 0 assignees View on GitHub
affects-7.1 affects-7.5 affects-8.1 affects-8.5 found-by-ai severity/major sig/sql-infra sig/transaction type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

### Summary

When `tidb_lock_unchanged_keys=ON` (the default), `addUnchangedKeysForLockByRow` does not correctly collect unchanged unique-index keys for either partial indexes or multi-valued indexes.

These are two independent failure modes; a partial multi-valued index is **not** required:

1. **Partial index only:** the partial-index predicate is not evaluated, so a row excluded from the index still locks an invented unique-index key. This causes false contention between rows that legally have the same value outside the predicate.
2. **Multi-valued index only:** the code calls `GenIndexKey` once for the whole JSON array instead of iterating the actual per-element keys. It therefore locks an invented whole-array key and does not lock the real unchanged element keys.

The affected code is:

https://github.com/pingcap/tidb/blob/f343ecdadb0d4bc3919ffa4640370c313e7b7986/pkg/executor/write.go#L390-L428

### 1. Minimal reproduce step (Required)

#### Case A: partial unique index without an MV index

Setup:

```sql
CREATE TABLE partial_t (
pk INT PRIMARY KEY,
k INT,
flag INT,
UNIQUE INDEX ux(k) WHERE flag = 1
);

INSERT INTO partial_t VALUES (1, 7, 0), (2, 7, 0);
```

Session 1:

```sql
SET @@tidb_lock_unchanged_keys = ON;
BEGIN PESSIMISTIC;
UPDATE partial_t SET flag = flag WHERE pk = 1;
```

Session 2:

```sql
SET @@tidb_lock_unchanged_keys = ON;
BEGIN PESSIMISTIC;
UPDATE partial_t SET flag = flag WHERE pk = 2;
```

The Session 2 UPDATE blocks until Session 1 commits or rolls back, even though the two rows are different and neither row has an entry in `ux`.

#### Case B: unique MV index without a partial predicate

Setup:

```sql
CREATE TABLE mv_t (
pk INT PRIMARY KEY,
j JSON,
UNIQUE INDEX ux((CAST(j AS SIGNED ARRAY)))
);

INSERT INTO mv_t VALUES (1, '[7]');
```

Session 1:

```sql
SET @@tidb_lock_unchanged_keys = ON;
BEGIN PESSIMISTIC;
UPDATE mv_t SET j = j WHERE pk = 1;
```

Session 2:

```sql
INSERT INTO mv_t VALUES (2, '[7]');
```

Session 2 immediately returns a duplicate-key error instead of waiting for Session 1. With a scalar unique index, the same INSERT waits for the transaction performing the no-op UPDATE.

#### Observable isolation anomaly

The missed MV element lock can participate in a history that cannot be explained by one serial order when the duplicate error is treated as an observable result:

1. T1 updates `counter_t.v` from `0` to `1`, then performs the no-op UPDATE on `mv_t` shown above.
2. T2 attempts to insert the duplicate MV element and immediately receives a duplicate-key error. This observes the old `mv_t` row before T1 deletes it.
3. T2 then updates the same `counter_t` row and waits for T1.
4. T1 deletes the old `mv_t` row and commits.
5. T2 resumes, increments the value written by T1 from `1` to `2`, and commits.

This creates dependencies in both directions: T2 must be before T1 to explain the duplicate observation, while T1 must be before T2 to explain the counter update.

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

- Rows that do not satisfy a partial-index predicate should not contribute any unchanged index key to the pessimistic lock set.
- For an MV index, all actual per-element unique-index keys should be collected and locked.
- With `tidb_lock_unchanged_keys=ON`, a competing write to an unchanged unique MV element should wait and retry in the same way as it does for a scalar unique index.

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

- Partial-index rows outside the predicate acquire false unique-key locks and contend with each other.
- An unchanged unique MV index locks one whole-array encoding rather than its actual element keys, so competing writes do not wait on the intended key.
- The behavior can change user-visible duplicate errors and transaction ordering.

No violation of transaction atomicity or unique-index consistency was reproduced: conflicting multi-row statements rolled back completely, and newly inserted or predicate-transitioned MV keys were correctly locked. The confirmed impact is incorrect pessimistic-lock/isolation behavior, plus unnecessary contention for partial indexes.

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

Current `master` at `f343ecdadb0d4bc3919ffa4640370c313e7b7986`.

Both independent cases were reproduced deterministically with executor testkit on MockStore/Unistore.

Contributor guide

Open the contributing guide

Research direction

Start in pkg/executor/write.go around addUnchangedKeysForLockByRow and reproduce both SQL cases with executor testkit on MockStore or Unistore. Add regression coverage showing that rows outside partial predicates add no keys and that every multi-valued index element is locked; done means competing writes have the expected waiting behavior without false contention.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.