pingcap / pingcap/tidb

ddl: PRE_SPLIT_REGIONS encodes TIMESTAMP split values in the DDL worker session's time zone

Open
#71,008 0 comments 0 reactions 1 assignee Claimed by @mjonss View on GitHub
affects-9.0 component/ddl severity/moderate 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)

`PRE_SPLIT_REGIONS = (BY ...)` and `PRE_SPLIT_REGIONS = (BETWEEN ... AND ...)` on `ADD INDEX` /
`CREATE INDEX` encode a TIMESTAMP split value in the wrong time zone when the DDL owner's internal
session time zone differs from the time zone of the session that submitted the statement.

```sql
SET @@session.time_zone = '+08:00';
CREATE TABLE t (a BIGINT, b TIMESTAMP);
ALTER TABLE t ADD INDEX idx(b) PRE_SPLIT_REGIONS = (BY ('2024-04-08 10:00:00'));
SHOW TABLE t INDEX idx REGIONS;
```

The split value is evaluated with the reorg expression context, which carries the submitting
session's time zone from `DDLReorgMeta.Location`, so the datum holds `10:00:00` as a `+08:00` wall
clock. The index key is then encoded with `sc.TimeZone()` taken from the DDL worker session that
runs the job (`w.sess.Context`, passed at `pkg/ddl/index.go:1240`), which is a different session
entirely. `DDLReorgMeta` captures `Location`, `SQLMode` and type flags precisely because the worker
session cannot reproduce the submitter's semantics.

Where the two zones come from

- Evaluation: `preSplitIndexRegions` builds its expression context with
`newReorgExprCtxWithReorgMeta` (`pkg/ddl/reorg.go:116-136`), which applies
`exprstatic.WithLocation(loc)` from `reorgTimeZoneWithTzLoc(reorgMeta.Location)`. With a nil
`reorgMeta.Location` it falls back to `timeutil.SystemLocation()`, which is still not the worker
session's zone.
- Encoding: `getSplitIdxPhysicalKeysFromValueList` and `getSplitIdxPhysicalKeysFromBound`
(`pkg/ddl/index_presplit.go`) call
`index.GenIndexKey(sc.ErrCtx(), sc.TimeZone(), ...)` with
`sc := sctx.GetSessionVars().StmtCtx` from the DDL worker session.

An index key always holds a TIMESTAMP in UTC, and `GenIndexKey` gets there by converting *from* the
zone it is given, so handing it the worker session's zone shifts a value that was expressed in the
submitter's zone by the difference between the two.

The same mismatch applies to `ErrCtx`: the evaluation applies
`reorgErrLevelsWithSQLMode(reorgMeta.SQLMode)` while the encoding takes its error levels from the
worker session. Lower stakes, but the same cause.

Reproduced at unit level by driving `preSplitIndexRegions` with `reorgMeta.Location` fixed at
`+08:00` and the DDL worker session at UTC, `-05:00` and `+09:30`: the three runs produce three
different sets of split keys, and none of them matches the instant the submitter named. Only the
`+08:00` worker session, that is the case where the two zones coincide, produces the correct key.

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

The pre-split boundary lands at the instant the submitting session named, `2024-04-08 10:00:00
+08:00` = `2024-04-08 02:00:00 UTC`, so the region boundary matches the index entries actually
written for that value.

Split points should be a function of the statement and the data, not of which TiDB node happens to
own the DDL job or how that node's internal session is configured.

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

The boundary is offset by the difference between the two zones, so regions are split at instants
nobody asked for. With the worker session at UTC and the submitter at `+08:00`, the split key is 8
hours away from the intended one.

This does not corrupt data or return wrong query results: index entries are written correctly, and
only the region boundaries are misplaced. The effect is that pre-splitting fails to do its job, so
the write hotspot it was meant to avoid is not avoided, and the resulting regions are unbalanced.

Because the DDL owner's session and the user's session often do share a time zone, this can go
unnoticed until the owner moves to a node configured differently, at which point the same statement
produces different split points.

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

Present on `master`. Introduced by #57553 (2024-12-18, "ddl: support pre-split index regions before
creating index"), which used the reorg expression context for evaluation and the worker session for
encoding from the start.

Not present in v8.5 or earlier: `pkg/ddl/index_presplit.go` does not exist on `release-8.5`,
`release-8.1` or `release-7.5`. The affected code is on `release-9.0-beta.1`, `release-9.0-beta.2`
and `release-nextgen-202603`.

Note that the `SPLIT TABLE ... INDEX ... BY` *statement* (`pkg/executor/split.go`) is a separate
code path and is not affected: it evaluates and encodes in the same session.

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.