pingcap / pingcap/tidb

MODIFY COLUMN can leave rows that violate an existing enforced CHECK constraint

Open
#69,649 0 comments 0 reactions 0 assignees View on GitHub
affects-9.0 component/ddl found-by-ai may-affects-7.5 may-affects-8.1 may-affects-8.5 severity/major sig/sql-infra 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)

`ALTER TABLE ... MODIFY COLUMN` rewrites existing rows through the DDL backfill path, and that path does not re-check `CHECK` constraints on the converted values. A type conversion that changes a value's truth value (e.g. `DECIMAL 0.40 -> INT 0`) leaves rows that violate a published, still-enforced `CHECK` constraint.

```sql
-- CHECK constraints must be enabled; a session reads this at connect time, so reconnect after:
SET GLOBAL tidb_enable_check_constraint = 1;

CREATE TABLE t(a DECIMAL(10,2), CONSTRAINT c CHECK (a > 0));
INSERT INTO t VALUES (0.4), (1.2); -- both satisfy a > 0

ALTER TABLE t MODIFY a INT; -- succeeds, no warning
SELECT a, a > 0 AS ok FROM t; -- 0 -> ok = 0 : row now violates CHECK (a > 0)
SHOW CREATE TABLE t; -- still declares CONSTRAINT `c` CHECK ((`a` > 0))
```

The same result reproduces with `DOUBLE` and `VARCHAR('0.4')` source columns converted to `INT`.

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

`MODIFY COLUMN` must not produce rows that violate an existing enforced `CHECK` constraint. It should either reject the conversion or validate the post-conversion rows, consistent with the sibling paths that already enforce the same constraint:

```sql
-- ADD CHECK scans existing rows and rejects the same bad value:
CREATE TABLE ref(a INT); INSERT INTO ref VALUES (0),(1);
ALTER TABLE ref ADD CONSTRAINT c2 CHECK (a > 0); -- ERROR 3819: Check constraint 'c2' is violated.

-- ordinary DML rejects it too:
INSERT INTO t VALUES (0); -- ERROR 3819: Check constraint 'c' is violated.
```

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

`ALTER TABLE t MODIFY a INT` succeeds with an empty `SHOW WARNINGS`, and the final table is self-inconsistent: `SHOW CREATE TABLE` still declares `CHECK ((a > 0))`, yet `SELECT a, a > 0 FROM t` returns a row with `a = 0` and `a > 0 = 0`. `ADMIN CHECK TABLE t` also passes, because it verifies record/index consistency, not `CHECK` predicate validity — so the corruption is not surfaced anywhere.

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

```
Release Version: v9.0.0-beta.2.pre-1774-g81ec977cb8
Git Commit Hash: 81ec977cb8bf97e0c9805dfc0be8ffcbd4b0bbeb
UTC Build Time: 2026-05-28 03:35:30
Edition: Community
Store: tikv
```

Likely root cause — the MODIFY COLUMN backfill writer bypasses CHECK enforcement

Ordinary DML enforces `CHECK` constraints in `pkg/table/tables/tables.go` (`AddRecord` / `UpdateRecord` call `CheckRowConstraint`), and `ALTER TABLE ADD CHECK` scans existing rows in `pkg/ddl/constraint.go` before publishing. The `MODIFY COLUMN` reorg writer does neither: in `pkg/ddl/column.go`, `updateColumnWorker.getRowRecord` decodes the old row, casts the old value to the new column type, and encodes the new row, which the backfill transaction then writes with `txn.Set` directly — there is no `CheckRowConstraint` equivalent on this path. Evaluating writable `CHECK` constraints on the post-conversion row before the write (reusing the DML `table.CheckRowConstraint` semantics), or scanning rows the way `ADD CHECK` does before the new column goes public, would close the gap.

Contributor guide

Open the contributing guide

Research direction

Start with the SQL reproduction, then read pkg/ddl/column.go, especially updateColumnWorker.getRowRecord and the MODIFY COLUMN backfill write path. Compare it with CheckRowConstraint in pkg/table/tables.go and the existing validation scan in pkg/ddl/constraint.go. Done means MODIFY COLUMN rejects the conversion or validates post-conversion rows so no enforced CHECK constraint is violated.

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
Clearly specified
Newbie friendliness
56/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.