executor: pessimistic unchanged-key locking mishandles partial and multi-valued unique indexes
- 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
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