pingcap / pingcap/tidb

[executor] Vectorized recursive CTE can turn a sign-only DECIMAL into zero and overwrite an existing row

Open
#70,118 1 comment 0 reactions 0 assignees View on GitHub
affects-7.5 affects-8.1 affects-8.5 component/executor component/expression found-by-ai severity/critical type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

### 1. Minimal reproduce step (Required)

**Environment**

TiDB master 05b396fb66; default vectorized expression; default strict mode; MDL ON; one TiDB and real TiKV

**Steps**

Create roots(node_id, amount DECIMAL(10,2) UNSIGNED), edges(node_id,parent_id,raw_amount VARCHAR), and matched result tables keyed by amount. Put 1.00 in the root, '-' in one child, and prepopulate amount=0 with marker protected. Run a recursive CTE that selects the typed root amount and the child raw_amount, then REPLACE the CTE rows into each result table once with tidb_enable_vectorized_expression=OFF and once with the default ON. Record the recursive Projection cast, terminals, affected rows, and complete destination rows.

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

The vectorized union-contract cast preserves scalar parsing. Under default strict mode, sign-only '-' produces error 1292, the REPLACE is atomic, and the protected zero-key row remains.

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

Scalar REPLACE returns error 1292, affects zero rows, and keeps 0.00/999/protected. Default vectorized REPLACE succeeds with affected_rows=3, stores 1.00/1/generated, fabricates 0.00 from '-', and replaces the protected row with 0.00/2/generated.

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

TiDB master 05b396fb66; default vectorized expression; default strict mode; MDL ON; one TiDB and real TiKV

Likely root cause and fix direction

**Likely root cause**

builtinCastStringAsDecimalSig.evalDecimal treats a value as a negative number only when len(val)>1, so '-' reaches MyDecimal.FromString and the strict truncate terminal. vecEvalDecimal uses len(val)>0, classifies '-' as an unsigned-union negative, skips parsing, and leaves the initialized decimal zero. The recursive CTE seed provides the unsigned DECIMAL type contract.

**Fix direction**

Make the vector negative classifier require len(val)>1, matching scalar evaluation. Add strict sign-only scalar/vector parity and a recursive CTE REPLACE regression. Audit every pre-parser classifier that can skip validation in scalar, vectorized, generated, batch, or remote twins.

Contributor guide

Open the contributing guide

Research direction

Start with builtinCastStringAsDecimalSig.evalDecimal and vecEvalDecimal, comparing how each classifies sign-only '-' before parsing. Run the minimal recursive CTE and REPLACE reproduction in scalar and vectorized modes; done means both paths return error 1292 atomically, preserve the protected zero row, and have regression coverage for the parity case and relevant classifier twins.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.