pingcap / pingcap/tidb

planner: reject SELECT FOR UPDATE / FOR SHARE on mview and mlog tables

Open
#66,588 2 comments 0 reactions 0 assignees View on GitHub
component/mview feature/developing sig/transaction type/new-feature
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Enhancement

### Background

PR #66396 added write control via `CheckMViewUpdatable()` (`pkg/planner/core/util.go`) to reject explicit DML (INSERT / UPDATE / DELETE / LOAD DATA / IMPORT INTO) on materialized view and MV log tables. However, `SELECT ... FOR UPDATE`, `SELECT ... FOR SHARE`, and `SELECT ... LOCK IN SHARE MODE` are not checked — `buildSelectLock()` (`pkg/planner/core/planbuilder.go:1439`) does not call `CheckMViewUpdatable()`.

### Current behavior

```sql
CREATE TABLE t (a INT PRIMARY KEY, b INT);
CREATE MATERIALIZED VIEW LOG ON t (a, b);
INSERT INTO t VALUES (1, 2);

-- All succeed without error:
SELECT * FROM `$mlog$t` FOR UPDATE;
SELECT * FROM `$mlog$t` FOR SHARE;
SELECT * FROM `$mlog$t` LOCK IN SHARE MODE;

CREATE MATERIALIZED VIEW v (a, s, cnt) AS SELECT a, SUM(b), COUNT(1) FROM t GROUP BY a;

-- All succeed without error:
SELECT * FROM v FOR UPDATE;
SELECT * FROM v FOR SHARE;
SELECT * FROM v LOCK IN SHARE MODE;
```

### Expected behavior

These statements should be rejected, consistent with how explicit DML is rejected.

### Impact

`REFRESH MATERIALIZED VIEW` runs in a pessimistic transaction (`pkg/executor/materialized_view.go`). During refresh:

- **Complete refresh** executes `DELETE FROM ` followed by `INSERT INTO `.
- **Fast refresh** writes incremental changes to the MV table.

`SELECT ... FOR UPDATE` acquires pessimistic locks in TiKV. If a user session holds `FOR UPDATE` locks on MV or MV log table rows, the refresh transaction may encounter lock conflicts when it tries to write to those rows. This can cause the refresh to block or fail with a lock wait timeout.

Note: `FOR SHARE` / `LOCK IN SHARE MODE` are noop in TiDB (gated behind `tidb_enable_noop_functions`), so they do not acquire real locks and would not cause refresh conflicts in practice. However, for consistency with the DML rejection, they should also be rejected.

ref #18023

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.