[tikv] TiKV-pushed WEEK can delete valid rows when the mode column is NULL
- 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
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