pingcap / pingcap/tidb

FOR UPDATE OF can bind different lock targets between v8.5.5 and v8.5.6

Open
#67,705 3 comments 0 reactions 0 assignees View on GitHub
may-affects-7.1 may-affects-7.5 may-affects-8.1 may-affects-8.5 severity/major sig/planner 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)

This issue was found while validating the `v8.5.5 -> v8.5.6` rolling upgrade window for #67130, which cherry-picked #65532 to `release-8.5`.

The same SQL has different lock targets between `v8.5.5` and the `v8.5.6-pre` build at commit `499c8777ea5c5002c9bc7d969d0a37ca5efb55d1`.

The behavior can be reproduced with two separate single-version playground clusters. They do not need to be a mixed cluster to show the version split.

First, start a `v8.5.5` playground:

```bash
tiup playground v8.5.5 \
--db 1 --pd 1 --kv 1 --tiflash 0 --without-monitor \
--tag single-v855 \
--port-offset 34000
```

Then initialize the schema:

```sql
DROP DATABASE IF EXISTS lock_db1;
DROP DATABASE IF EXISTS lock_db2;
CREATE DATABASE lock_db1;
CREATE DATABASE lock_db2;

USE lock_db1;
CREATE TABLE t(id INT PRIMARY KEY, v INT);
INSERT INTO t VALUES (1, 100);

USE lock_db2;
CREATE TABLE t(id INT PRIMARY KEY, v INT);
INSERT INTO t VALUES (1, 200);
```

In session A, keep this transaction open:

```sql
SET SESSION tidb_txn_mode = 'pessimistic';
USE lock_db2;
BEGIN PESSIMISTIC;
SELECT * FROM lock_db1.t ue1_0, lock_db2.t ue1_1 FOR UPDATE OF t;
SHOW WARNINGS;
SELECT SLEEP(15);
ROLLBACK;
```

While session A is sleeping, probe both tables from session B:

```sql
SET SESSION innodb_lock_wait_timeout = 1;
BEGIN PESSIMISTIC;
UPDATE lock_db1.t SET v = v + 1 WHERE id = 1;
ROLLBACK;
```

```sql
SET SESSION innodb_lock_wait_timeout = 1;
BEGIN PESSIMISTIC;
UPDATE lock_db2.t SET v = v + 1 WHERE id = 1;
ROLLBACK;
```

Repeat the same steps on a separate `v8.5.6-pre` playground:

```bash
tiup playground v8.5.6-pre \
--db 1 --pd 1 --kv 1 --tiflash 0 --without-monitor \
--tag single-v856pre \
--port-offset 41000
```

Negative control:

```sql
SET SESSION tidb_txn_mode = 'pessimistic';
USE lock_db2;
BEGIN PESSIMISTIC;
SELECT * FROM lock_db1.t ue1_0, lock_db2.t ue1_1 FOR UPDATE OF lock_db1.t;
SHOW WARNINGS;
SELECT SLEEP(15);
ROLLBACK;
```

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

The same `SELECT ... FOR UPDATE OF ...` statement should not silently change which table it locks between two patch versions that can coexist during a rolling upgrade.

For:

```sql
USE lock_db2;
SELECT * FROM lock_db1.t ue1_0, lock_db2.t ue1_1 FOR UPDATE OF t;
```

TiDB should either:

- lock the same table on both `v8.5.5` and `v8.5.6`, or
- reject the ambiguous `OF t` consistently, instead of locking different tables.

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

On standalone `v8.5.5`:

- the holder query succeeded;
- `UPDATE lock_db1.t ...` succeeded;
- `UPDATE lock_db2.t ...` failed with `ERROR 1205 (HY000): Lock wait timeout exceeded`;
- conclusion: `v8.5.5` locked `lock_db2.t`.

On standalone `v8.5.6-pre`:

- the holder query succeeded;
- `SHOW WARNINGS` returned:

```text
FOR UPDATE OF references the base table name while the table is aliased. Use the alias 'ue1_0' in OF to make the lock target explicit.
```

- `UPDATE lock_db1.t ...` failed with `ERROR 1205 (HY000): Lock wait timeout exceeded`;
- `UPDATE lock_db2.t ...` succeeded;
- conclusion: `v8.5.6-pre` locked `lock_db1.t`.

The negative control `FOR UPDATE OF lock_db1.t` locked `lock_db1.t` on both versions. This isolates the split to ambiguous unqualified `OF t` binding.

This means that during a rolling upgrade with both `v8.5.5` and `v8.5.6` TiDB frontends serving traffic, the same SQL text can lock different tables depending on which frontend handles the session.

There is also a related alias success/error split:

```sql
USE lock_db1;
SELECT ue1_0.id
FROM t ue1_0
WHERE ue1_0.id = 1
FOR UPDATE OF ue1_0;
```

- `v8.5.5`: `ERROR 1146 (42S02): Table 'lock_db1.ue1_0' doesn't exist`
- `v8.5.6-pre`: succeeds and returns the row

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

Low version:

```text
Release Version: v8.5.5
Git Commit Hash: 1fa258b833ff113883beeba40bc130be7ce66610
```

High version:

```text
Release Version: v8.5.6
Git Commit Hash: 499c8777ea5c5002c9bc7d969d0a37ca5efb55d1
```

## Compatibility addendum

This looks like a patch-window SQL semantic compatibility issue introduced by the alias-aware `FOR UPDATE OF` fix in #67130 / #65532.

Observed root-cause direction:

- before #67130, an unqualified `OF t` target is resolved through the old preprocess path, which fills the empty schema from `currentDB`; with `USE lock_db2`, this resolves to `lock_db2.t`;
- after #67130, lock-clause binding is deferred until after collecting `FROM` references and aliases; the compatibility fallback for base table name `t` resolves against the `FROM` list, so it picks `lock_db1.t AS ue1_0` in the reproducer above;
- the resolved target feeds `StmtCtx.LockTableIDs`, so the difference is not only a warning or parser-level drift; it changes the actual pessimistic lock keys selected by `SELECT ... FOR UPDATE OF ...`.

User impact:

- the trigger surface is narrow: `SELECT ... FOR UPDATE OF ...` with aliases or ambiguous base table names during `v8.5.5 -> v8.5.6` rolling upgrade / rollback;
- if an application relies on this lock to protect a later update, one TiDB frontend may lock the intended business row while another frontend locks a different table's row, causing application-level concurrency protection to fail;
- this is not TiDB storage corruption, but it can lead to application-level data consistency problems for affected SQL patterns.

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.