Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
f9340ed
update
joechenrh May 27, 2025
b47d075
update
joechenrh May 27, 2025
8d6872c
update
joechenrh May 28, 2025
2fef8fa
fix SQL
joechenrh May 29, 2025
a90c62f
change implement
joechenrh May 29, 2025
756e9b5
fix check SQL
joechenrh May 29, 2025
7b161dd
update comment
joechenrh May 29, 2025
86096c7
add limit to fast mv index check
joechenrh May 29, 2025
bf7d299
fix check
joechenrh May 29, 2025
4237c5d
fix and add test
joechenrh May 30, 2025
bd42a3b
update
joechenrh May 30, 2025
8c6b234
add double type
joechenrh May 30, 2025
02f97a7
fix
joechenrh May 30, 2025
cfd0b99
update
joechenrh Aug 8, 2025
75e8433
remove some code
joechenrh Aug 12, 2025
04ee5dd
Merge branch 'master' into test_mv_index
joechenrh Aug 12, 2025
594357f
update bazel
joechenrh Aug 12, 2025
07651fc
update test
joechenrh Aug 12, 2025
463d7ed
fixup
joechenrh Aug 13, 2025
624cf0d
fixup
joechenrh Aug 13, 2025
586a20b
update
joechenrh Aug 13, 2025
40afe4f
update generated file
joechenrh Aug 14, 2025
f11651f
Merge branch 'master' into test_mv_index
joechenrh Aug 18, 2025
6eceef0
fix after merge
joechenrh Aug 18, 2025
0cb2fdc
update
joechenrh Aug 18, 2025
ac55da3
update
joechenrh Aug 18, 2025
64f7068
remove test
joechenrh Aug 18, 2025
a93bec0
update
joechenrh Aug 19, 2025
ef9a580
fix JSON tests
joechenrh Aug 19, 2025
8d4de37
fix tests
joechenrh Aug 19, 2025
5555bba
remove unnecessary variables
joechenrh Aug 20, 2025
8bd6c3c
update
joechenrh Aug 20, 2025
be9e1d9
move test
joechenrh Aug 20, 2025
c350c78
fix unstable global index test
joechenrh Aug 20, 2025
bc3cc7b
Merge branch 'master' into test_mv_index
joechenrh Aug 25, 2025
1ee37e6
update
joechenrh Aug 25, 2025
b1b5ccc
make admin check for global index stable
joechenrh Aug 25, 2025
642b847
Merge branch 'master' into test_mv_index
joechenrh Mar 18, 2026
241f361
docs: add design spec for MV index fast admin check refactoring
joechenrh Mar 18, 2026
d7a65eb
docs: update spec - builder builds complete SQL, no template placehol…
joechenrh Mar 18, 2026
6d49946
docs: add implementation plan for MV index fast check refactoring
joechenrh Mar 18, 2026
191536f
executor: add indexCheckBuilder interface and normalCheckBuilder
joechenrh Mar 18, 2026
0eecbb8
executor: add mvCheckBuilder and newIndexCheckBuilder constructor
joechenrh Mar 18, 2026
7ef0dcf
executor: refactor handleTask to use indexCheckBuilder interface
joechenrh Mar 18, 2026
2220a05
executor: remove old package-level SQL build functions
joechenrh Mar 18, 2026
5defddd
executor: fix check_table_index.go compilation errors after master merge
joechenrh Mar 19, 2026
b415af9
executor: fix builder.go compilation errors after master merge
joechenrh Mar 19, 2026
beaffdc
executor: fix compilation errors after master merge
joechenrh Mar 19, 2026
db88368
docs: add Phase C design spec for MV index BIT_XOR unification
joechenrh Mar 19, 2026
081f919
docs: simplify Phase C function signature to 2 params (array, prefix)
joechenrh Mar 19, 2026
76893df
docs: add type compatibility analysis to Phase C design
joechenrh Mar 19, 2026
e413c5b
docs: rewrite Phase C spec with type safety as core design constraint
joechenrh Mar 19, 2026
c1e6f9e
docs: fix Phase C spec issues from review
joechenrh Mar 19, 2026
7745aaa
docs: switch Phase C to Datum.ConvertTo approach for type conversion
joechenrh Mar 19, 2026
49f3d31
docs: add Phase C implementation plan
joechenrh Mar 19, 2026
28f6ab5
parser: add JSON_ARRAY_XOR_CRC32 AST node and parsing rule
joechenrh Mar 19, 2026
392a551
expression: add JSON_ARRAY_XOR_CRC32 builtin function
joechenrh Mar 19, 2026
e1c64b9
planner: add expression rewriter for JSON_ARRAY_XOR_CRC32
joechenrh Mar 19, 2026
922b7cd
executor: add mvXorCheckBuilder with BIT_XOR-based checksums
joechenrh Mar 19, 2026
4d1b67d
executor: swap to mvXorCheckBuilder, remove MV index type restriction
joechenrh Mar 19, 2026
42cf244
executor: fix mvXorCheckBuilder index-side check row query
joechenrh Mar 19, 2026
d934675
executor,expression,planner: delete JSON_SUM_CRC32, mvCheckBuilder, a…
joechenrh Mar 19, 2026
04b14f3
executor,expression,planner: delete JSON_SUM_CRC32 and mvCheckBuilder
joechenrh Mar 19, 2026
5a275a3
fix bazel
joechenrh Mar 19, 2026
f8af198
executor: add per-type edge case tests for MV index fast check
joechenrh Mar 19, 2026
3cc177a
parser,expression: delete JSON_SUM_CRC32 parser definitions and test …
joechenrh Mar 19, 2026
d10b9e2
executor: fix PR diff by restoring unrelated master changes
joechenrh Mar 19, 2026
e66923f
executor: fix admin_test.go PR diff - restore master content, add MV …
joechenrh Mar 19, 2026
332261d
executor,expression,planner: address CodeRabbit review comments
joechenrh Mar 19, 2026
e2073dd
tests: explicitly enable fast table check in MV index integration test
joechenrh Mar 19, 2026
d414ef7
Merge remote-tracking branch 'origin/master' into test_mv_index
joechenrh Mar 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
410 changes: 410 additions & 0 deletions docs/superpowers/plans/2026-03-18-mv-index-fast-check-refactor.md

Large diffs are not rendered by default.

573 changes: 573 additions & 0 deletions docs/superpowers/plans/2026-03-19-mv-index-fast-check-phase-c.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
# MV Index Fast Admin Check — Phase C: Unify to BIT_XOR

## Goal

Replace the MV index checksum from `SUM(handle_crc * element_crc)` to `BIT_XOR(per_entry_crc)`, unifying the aggregation with normal indexes. This eliminates `JSON_SUM_CRC32`, `JSONCRC32Mod`, and fixes a latent type-safety bug.

## Background

After Phase A, the `indexCheckBuilder` interface cleanly separates MV and normal index logic. The two builders currently use different aggregation:
- **normalCheckBuilder**: `BIT_XOR(CRC32(MD5(CONCAT_WS(handle, idx_cols))))`
- **mvCheckBuilder**: `SUM(handle_crc * JSON_SUM_CRC32(array))` with `MOD 1024`

Phase C unifies both to BIT_XOR and fixes a type-safety issue in the process.

## Math

For a table row with handle `h`, non-array index cols `n`, and MV index array `[a, b]` (after dedup):

**Index side** (each MV entry is independent):
```
BIT_XOR(CRC32(MD5(CONCAT_WS(0x2, h, n, a))), CRC32(MD5(CONCAT_WS(0x2, h, n, b))))
```

**Table side** (per-handle XOR, then cross-handle BIT_XOR):
```
BIT_XOR( CRC32(h||n||a) ⊕ CRC32(h||n||b) )
```

These are equal because XOR is associative: `(A ⊕ B) ⊕ C = A ⊕ B ⊕ C`.

**Why non-array cols must be in the CRC**: Without `n`, corruption in non-array columns goes undetected:
```
Index mvi(a, CAST(j AS UNSIGNED ARRAY)), row: handle=1, a=10, j=[1,2]
If a is corrupted to 99 in the index:
Without n: CRC32("1,1") ⊕ CRC32("1,2") — same on both sides → missed ✗
With n: CRC32("1,99,1") vs CRC32("1,10,1") — different → detected ✓
```

## New Builtin Function: `JSON_ARRAY_XOR_CRC32`

**Signature**: `JSON_ARRAY_XOR_CRC32(array_expr, prefix_string)`

The prefix is pre-computed in SQL:
```sql
JSON_ARRAY_XOR_CRC32(array_expr, CONCAT_WS(0x2, handle_cols, non_array_cols))
```

The target type for casting (e.g., DOUBLE, CHAR(10)) is carried in the function class metadata (same pattern as `JSON_SUM_CRC32`), not as an SQL argument. The builder constructs this from `tblCol.FieldType.ArrayType()`.

**Semantics**:
1. Evaluate `array_expr` as a JSON array
2. If `array_expr` is NULL: return `CRC32(MD5(prefix_string))` — matches the index-side behavior where `CONCAT_WS(0x2, prefix, NULL)` skips the NULL argument, leaving only the prefix
3. For each element `e`:
a. Cast `e` to the target type, then convert to string via `Datum.ConvertTo(sc, targetFieldType)` → `Datum.ConvertTo(sc, stringFieldType)`
b. Compute `CRC32(MD5(CONCAT_WS(0x2, prefix_string, str)))`
4. Deduplicate by CRC32 result, then XOR all unique CRC32s together (dedup must happen after casting, because different JSON representations like int `1` and float `1.0` may produce the same cast result — matching index dedup semantics)
5. Return the result as UNSIGNED INT

### Type Safety

The checksum is correct only when the table-side and index-side string representations of array elements are **identical**.

**Current problem**: `JSON_SUM_CRC32` uses a custom `convertJSON2String` (builtin_json.go:234–305) with per-type formatting logic that may diverge from the index side's implicit CAST-to-STRING path. For example, ETReal uses `strconv.FormatFloat(val, 'f', -1, digit)` without `ProduceStrWithSpecifiedTp`, while the index-side CAST may include it.

**Phase C approach**: Use `Datum.ConvertTo` for type conversion — this goes through TiDB's standard type conversion system. While not 100% guaranteed to match the expression layer's `builtinCastXXXAsStringSig` in all edge cases, it is far more aligned than the current custom `convertJSON2String`, and covers all supported CAST ARRAY types.

```go
// Pseudocode for element processing:
for each element in json_array {
// 1. Convert JSON element to target type datum
datum := convertJSONToDatum(element, targetFieldType)
// 2. Convert datum to string via standard type system
strDatum, _ := datum.ConvertTo(sc, stringFieldType)
str := strDatum.GetString()
// 3. Compute CRC with prefix
crc := crc32.ChecksumIEEE([]byte(CONCAT_WS(0x2, prefix, str)))
result ^= crc // dedup by CRC value, then XOR unique CRCs
}
```

**Correctness is verified by per-type tests**, not by code-path analysis alone. Each supported type must have a test that inserts data, runs `admin check table` (fast), and verifies no false positives.

With `Datum.ConvertTo` + per-type tests, all CAST ARRAY types should be safe, including ETDecimal and ETReal. The `indexSupportFastCheck` type restriction in builder.go can be removed once tests pass for each type.

## SQL Changes

### Checksum Phase

**Table side**:
```sql
SELECT BIT_XOR(JSON_ARRAY_XOR_CRC32(array_expr, CONCAT_WS(0x2, handle, non_array))),
bucket, COUNT(*)
FROM t USE INDEX()
WHERE json_length_filter AND whereKey = 0
GROUP BY bucket
```

**Index side**:
```sql
SELECT BIT_XOR(CRC32(MD5(CONCAT_WS(0x2, handle, non_array, hidden_col)))),
bucket, COUNT(*)
FROM t USE INDEX(idx)
WHERE whereKey = 0
GROUP BY bucket
```

**Note on COUNT(*)**: For MV indexes, COUNT(*) differs between table side (one row per handle) and index side (N entries per handle). The algorithm only uses count for drill-down termination (`tableRowCntToCheck`), not for correctness checks.

### Detail (Check Row) Phase

**Table side**: `JSON_ARRAY_XOR_CRC32` per row as the checksum column, ordered by handle.

**Index side**: Subquery with `GROUP BY handle`, using `BIT_XOR(CRC32(MD5(CONCAT_WS(...))))` per handle as the checksum, with `JSON_ARRAYAGG` for array values.

## Implementation: `mvXorCheckBuilder`

New builder implementing `indexCheckBuilder`, replacing `mvCheckBuilder`:

- `handleChecksum()`: `CRC32(MD5(CONCAT_WS(handle_cols)))` — same as `normalCheckBuilder`. **Note**: unlike `mvCheckBuilder`, this uses only handle columns (not non-array index columns), because non-array columns are now included in the per-entry CRC instead.
- `buildChecksumQuery()`: table side uses `BIT_XOR(JSON_ARRAY_XOR_CRC32(...))`, index side uses `BIT_XOR(CRC32(MD5(CONCAT_WS(...))))`
- `buildCheckRowQuery()`: table side uses `JSON_ARRAY_XOR_CRC32` per row; index side uses subquery + GROUP BY handle
- `getRecords()`: same as current `mvCheckBuilder`

## Migration

1. Add `JSON_ARRAY_XOR_CRC32` in parser/planner/expression (same restricted-context approach as `JSON_SUM_CRC32`)
2. Add `mvXorCheckBuilder` implementing `indexCheckBuilder`
3. Swap `newIndexCheckBuilder` to use `mvXorCheckBuilder`
4. Remove type restriction in `indexSupportFastCheck`
5. Delete `mvCheckBuilder`, `JSON_SUM_CRC32`, `JSONCRC32Mod`, and `convertJSON2String`

## Verification

Each supported type needs an explicit test: insert data → `admin check table` (fast) → must pass. Edge cases:

- Float: `[1.0, 2.5, 0.1]` (formatting precision)
- Decimal: `[1.10, 2.00]` (trailing zeros)
- Datetime: `['2024-01-01 00:00:00']` (with/without time)
- Duration: `['12:30:00']` (with/without fractional seconds)
- Empty array: `[]` (no index entries)
- NULL JSON (creates one NULL index entry)
- Duplicates: `[1, 1, 2]` (dedup must match index behavior)

## Benefits

- Unified aggregation: both MV and normal indexes use `BIT_XOR`
- No `MOD 1024` overflow protection (XOR is bounded to 32 bits)
- Type-safe: `Datum.ConvertTo`-based conversion replaces custom `convertJSON2String`, supports all CAST ARRAY types (verified by per-type tests)
- `handleChecksum()` unified between builders
- Correctness fix: eliminates `convertJSON2String` divergence risk

## Trade-offs

- Still needs one internal-only builtin function (replacement, not addition)
- Still can't be pushed to TiKV (same limitation as `JSON_SUM_CRC32`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# MV Index Fast Admin Check Refactoring

## Problem

The current fast admin check (`FastCheckTableExec`) has separate code paths for normal indexes and multi-valued (MV) indexes. The MV-specific logic is interleaved in `handleTask`, making the code hard to maintain and extend. Key issues:

1. `handleTask` branches on `idxInfo.MVIndex` to choose between 4 different SQL builder functions
2. MV indexes use `SUM(handle_crc * element_crc)` while normal indexes use `BIT_XOR(CRC32(...))` — two different checksum models
3. A custom builtin function `JSON_SUM_CRC32` was added to the expression layer solely for this use case (and cannot be pushed down to TiKV)
4. SQL templates use heavy `fmt.Sprintf` with `%s` placeholders, reducing readability

## Design: Phase A — Strategy Abstraction

### Core Abstraction: `indexCheckBuilder`

Extract the difference between MV and normal index checking into a builder interface:

```go
// indexCheckBuilder abstracts SQL generation and row parsing for fast admin check.
// handleTask depends only on this interface — it never checks idxInfo.MVIndex.
type indexCheckBuilder interface {
// handleChecksum returns the SQL expression used for bucketing.
// e.g. "CRC32(MD5(CONCAT_WS(0x2, `id`)))"
handleChecksum() string

// buildChecksumQuery returns complete SQL for the checksum comparison phase.
// The builder handles all internal formatting — caller only passes logical parameters.
buildChecksumQuery(groupByKey, whereKey string) (tableSQL, indexSQL string)

// buildCheckRowQuery returns complete SQL for the detail row comparison phase.
buildCheckRowQuery(groupByKey string) (tableSQL, indexSQL string)

// getRecords parses query results into records with checksums for row comparison.
// Each builder knows its own column layout (e.g. MV indexes map array columns to TypeJSON).
getRecords(ctx context.Context, se sessionctx.Context, sql string) ([]*recordWithChecksum, error)
}
```

Construction:

```go
func newIndexCheckBuilder(tblName string, handleCols []string, pkTypes []*types.FieldType,
idxInfo *model.IndexInfo, tblInfo *model.TableInfo,
) indexCheckBuilder {
if idxInfo.MVIndex {
return &mvCheckBuilder{...}
}
return &normalCheckBuilder{...}
}
```

### `handleTask` Refactoring

The only line that is index-type-aware becomes `newIndexCheckBuilder(...)`. Everything else uses the interface:

```go
builder := newIndexCheckBuilder(tblName, handleCols, pkTypes, idxInfo, tblInfo)
md5Handle := builder.handleChecksum()

// Checksum phase loop — identical for MV and normal
for tableRowCntToCheck > LookupCheckThreshold || checkTimes == 0 {
tableQuery, indexQuery := builder.buildChecksumQuery(groupByKey, whereKey)
// ... compare checksums, narrow down bucket ...
}

// Detail phase — also identical
if meetError {
tableQuery, indexQuery := builder.buildCheckRowQuery(groupByKey)
idxRecords, _ := builder.getRecords(ctx, se, indexQuery)
tblRecords, _ := builder.getRecords(ctx, se, tableQuery)
// ... compare handle by handle ...
}
```

Unchanged parts:
- Checksum phase bucketed binary search logic (~80 lines)
- Detail phase handle comparison logic (~50 lines)
- `getGroupChecksum`, `queryToRow`, `getReporter`, `initSessCtx`, and other utility functions

### Builder Implementations

**`normalCheckBuilder`**: wraps existing `buildChecksumSQLForNormalIndex` and `buildCheckRowSQLForNormalIndex` logic. No behavior change.

- `handleChecksum()` → `crc32FromCols(handleCols)`
- `buildChecksumQuery(groupByKey, whereKey)` → `BIT_XOR(CRC32(MD5(CONCAT_WS(handle, idx_cols))))` grouped by bucket
- `buildCheckRowQuery(groupByKey)` → fetches individual rows with handle + index columns + per-row checksum
- `getRecords()` → calls shared `getRecordWithChecksum` with standard `indexColTypes`

**`mvCheckBuilder`**: wraps existing `buildChecksumSQLForMVIndex` and `buildCheckRowSQLForMVIndex` logic. No behavior change.

- `handleChecksum()` → `crc32FromCols(handleCols + non-array index cols)`
- `buildChecksumQuery(groupByKey, whereKey)` → `SUM(handle_crc * JSON_SUM_CRC32(array_expr))` on table side, `SUM(handle_crc * CRC32(hidden_col))` on index side
- `buildCheckRowQuery(groupByKey)` → table side fetches JSON arrays directly; index side uses subquery + `JSON_ARRAYAGG` + `GROUP BY handle`
- `getRecords()` → calls shared `getRecordWithChecksum` with array columns mapped to `TypeJSON`

The existing 4 package-level `buildXxxForMVIndex` / `buildXxxForNormalIndex` functions become builder private methods.

## Scope

### In scope (Phase A)
- Introduce `indexCheckBuilder` interface + two implementations
- Refactor `handleTask` to be index-type-agnostic
- Remove `if idxInfo.MVIndex` branches from `handleTask`
- Remove debug TODO code (line 802-803 in current check_table_index.go)
- Internalize the 4 `buildXxx` package-level functions into builder methods

### Out of scope (future phases)
- **Phase C**: Unify aggregation to BIT_XOR, replace `JSON_SUM_CRC32` with a new `JSON_ARRAY_XOR` function. Migration path: add `mvXorCheckBuilder` implementing the same `indexCheckBuilder` interface, swap in `newIndexCheckBuilder`, delete old `mvCheckBuilder` and `JSON_SUM_CRC32`.
- **Type support**: Remove the type restriction in `indexSupportFastCheck` (builder.go TODO)
- **Master improvements**: Quick pass global checksum, session vars propagation, `verifyIndexSideQuery` with `intest.AssertFunc`, `execdetails.ContextWithInitializedExecDetails` — orthogonal to this refactoring, can be cherry-picked separately.

## Files Changed

| File | Change |
|------|--------|
| `pkg/executor/check_table_index.go` | Add `indexCheckBuilder` interface, `normalCheckBuilder`, `mvCheckBuilder`; refactor `handleTask`; internalize build functions |
| `pkg/executor/builder.go` | `indexSupportFastCheck` unchanged |
| `pkg/executor/test/admintest/admin_test.go` | Existing tests cover correctness; no new tests needed for pure refactoring |
1 change: 0 additions & 1 deletion pkg/executor/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ go_library(
"//pkg/dxf/framework/proto",
"//pkg/dxf/framework/storage",
"//pkg/dxf/importinto",
"//pkg/dxf/operator",
"//pkg/errctx",
"//pkg/errno",
"//pkg/executor/aggfuncs",
Expand Down
46 changes: 32 additions & 14 deletions pkg/executor/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -531,30 +531,48 @@ func buildIndexLookUpChecker(b *executorBuilder, p *physicalop.PhysicalIndexLook
}
}

// indexSupportFastCheck checks whether the index supports fast check.
func indexSupportFastCheck(tblInfo *model.TableInfo, idxInfo *model.IndexInfo) bool {
// Columnar index does not support fast check.
if idxInfo.IsColumnarIndex() {
return false
}

// Partial index (with WHERE condition) does not support fast check: the
// checksum query would scan all rows, producing mismatches for rows
// excluded by the condition.
if idxInfo.HasCondition() {
return false
}

for _, col := range idxInfo.Columns {
// Prefix index does not support fast check.
if col.Length != types.UnspecifiedLength {
return false
}

// MV Index array columns: all CAST ARRAY types are supported by
// JSON_ARRAY_XOR_CRC32. No type restriction needed.
}

return true
}

func (b *executorBuilder) buildCheckTable(v *plannercore.CheckTable) exec.Executor {
canUseFastCheck := true
supportFastAdminCheck := true
tblInfo := v.Table.Meta()
for _, idx := range v.IndexInfos {
if idx.MVIndex || idx.IsColumnarIndex() {
canUseFastCheck = false
break
}
for _, col := range idx.Columns {
if col.Length != types.UnspecifiedLength {
canUseFastCheck = false
break
}
}
if !canUseFastCheck {
if !indexSupportFastCheck(tblInfo, idx) {
supportFastAdminCheck = false
break
}
}
if b.ctx.GetSessionVars().FastCheckTable && canUseFastCheck {
if b.ctx.GetSessionVars().FastCheckTable && supportFastAdminCheck {
e := &FastCheckTableExec{
Comment thread
coderabbitai[bot] marked this conversation as resolved.
BaseExecutor: exec.NewBaseExecutor(b.ctx, v.Schema(), v.ID()),
dbName: v.DBName,
table: v.Table,
indexInfos: v.IndexInfos,
is: b.is,
}
return e
}
Expand Down
Loading
Loading