pingcap / pingcap/tidb

[tikv] TiKV-pushed WEEK can delete valid rows when the mode column is NULL

Open
#70,579 0 comments 0 reactions 0 assignees View on GitHub
component/tikv found-by-ai may-affects-25.10 may-affects-26.3 may-affects-7.5 may-affects-8.1 may-affects-8.5 severity/critical type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

# [tikv] TiKV-pushed WEEK can delete valid rows when the mode column is NULL

## Bug Report

### 1. Minimal reproduce step (Required)

This is distinct from `#70098`.

`#70098` is about the one-argument form `WEEK(date)` ignoring `default_week_format` after pushdown. This issue is about the two-argument form `WEEK(date, mode)` when `mode` is nullable and `NULL` semantically means “inherit mode 0”.

TiDB and TiKV currently disagree on that terminal:

- TiDB local evaluation treats `WEEK(date, NULL)` as `WEEK(date, 0)`.
- TiKV pushdown returns `NULL` when the `mode` argument is `NULL`.

Minimal shape:

```sql
DROP TABLE IF EXISTS t;
CREATE TABLE t (
id INT PRIMARY KEY,
d DATE NOT NULL,
mode INT NULL
);

INSERT INTO t VALUES
(1, '2023-01-01', NULL),
(2, '2023-01-08', 0),
(3, '2023-01-08', 1);
```

Local projection shows TiDB's contract:

```sql
SELECT id, WEEK(d, mode) AS w FROM t ORDER BY id;
```

Expected result:

```text
1 2
2 2
3 3
```

Now run a pushed cleanup predicate whose business meaning is “remove rows whose computed week is unknown”:

```sql
EXPLAIN FORMAT='brief'
DELETE FROM t WHERE WEEK(d, mode) IS NULL;

DELETE FROM t WHERE WEEK(d, mode) IS NULL;
SELECT * FROM t ORDER BY id;
```

In the reproduced plan, `isnull(week(d, mode))` stays in `Selection` on `cop[tikv]`.

For maintainers, the deterministic real-TiKV regression is the current-master witness:

```bash
go test --tags=intest ./tests/realtikvtest/pushdowntest \
-run 'TestPushdownLogic/TestWeekWithNullModeDelete$' \
-count=1 -v
```

The same evidence also contains a still-pushed GREEN counterfactual:

```sql
DELETE FROM t WHERE WEEK(d, IFNULL(mode, 0)) IS NULL;
```

That normalization keeps pushdown ownership on TiKV but restores the correct survivor set.

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

`WEEK(date, mode)` must preserve the same NULL/default semantics whether it is evaluated in TiDB or after TiKV pushdown.

For the rows above, `WEEK(d, mode) IS NULL` should match no row, so the `DELETE` should affect `0` rows and all three rows should remain.

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

On the reproduced real-TiKV path:

- local projection is `{1: 2, 2: 2, 3: 3}`
- pushed equality witness selects only `id=2`
- root equality witness selects `id=1,2`
- pushed `DELETE ... WHERE WEEK(d, mode) IS NULL` succeeds with `affected=1`
- survivors become `id=2,3`
- the same root-only `DELETE` affects `0` rows and keeps `id=1,2,3`

So TiKV interprets `mode=NULL` as a true `NULL` terminal, while TiDB interprets it as `mode=0`. A valid `DATE` row is deleted successfully under default settings.

The still-pushed counterfactual:

```sql
DELETE FROM t WHERE WEEK(d, IFNULL(mode, 0)) IS NULL;
```

is GREEN:

- plan ownership remains `cop[tikv]`
- affected rows become `0`
- survivors stay `id=1,2,3`
- `ADMIN CHECK TABLE` stays green

This isolates the bug to NULL-mode default materialization rather than pushdown enablement itself.

#### Likely root cause

Current source owners disagree on the optional-argument policy:

- TiDB `builtinWeekWithModeSig.evalInt` maps `NULL` mode to `0`
- TiKV `week_with_mode` returns `None` if `mode` is `None`

Both sides expose the same pushed `WeekWithMode` signature, but only TiDB applies the default-materialization rule.

Because the pushed terminal feeds a row-admission predicate for `DELETE`, the semantic mismatch becomes successful wrong-row data loss.

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

Reproduced product build:

```text
TiDB: ed2376acc6e0feeff9f3e2c38db489727933aa80
TiKV: 730be34f959185c934b7d3db730ca1dbeb3949f8
PD: f7db42521223b92fa30d68352b15e6962b699b7e
```

Latest upstream source audited at the time of validation still showed the same rule split:

```text
TiDB source: 05b396fb6636f73b3bc06b09107cf43f2c725c35
TiKV source: 91ccfb212677a43fd5255183ccf2afa4e3cec23e
```

Environment:

- one TiDB, one PD, one real TiKV
- default strict SQL mode
- vectorization ON
- MDL ON
- no failpoint or fault injection

### Impact

This is reachable with ordinary valid data:

1. a table stores a nullable week-mode policy,
2. `NULL` means “inherit the default 0-mode behavior”,
3. a retention/reconciliation predicate uses `WEEK(date, mode) IS NULL`,
4. TiKV pushdown turns those rows into a different preimage set and deletes a valid row.

No malformed date, nondefault SQL mode, restart, retry, concurrency, failpoint, or metadata corruption is required.

`ADMIN CHECK TABLE` remains green because the bug is semantic wrong-row deletion, not physical index inconsistency.

Contributor guide

Open the contributing guide

Research direction

Start with tests/realtikvtest/pushdowntest and run TestPushdownLogic/TestWeekWithNullModeDelete. Compare TiDB's builtinWeekWithModeSig.evalInt behavior with TiKV's week_with_mode handling of a NULL mode, then add regression coverage showing matching local and pushed results and that the DELETE affects zero rows.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.