ddl: MODIFY COLUMN rolls back when column name contains backticks (unescaped identifier in internal check SQL)
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
**Affected versions**: master (since #63965) and release-8.5 v8.5.4+ (cherry-pick #64138). Found during a PR-by-PR review of release-8.5.
### 1. Minimal reproduce step (Required)
```sql
DROP DATABASE IF EXISTS d;
CREATE DATABASE d;
CREATE TABLE d.t (`a``b` INT NOT NULL); -- column name is a`b
INSERT INTO d.t VALUES (1);
ALTER TABLE d.t MODIFY COLUMN `a``b` TINYINT NOT NULL;
SHOW CREATE TABLE d.t; -- column type is unchanged (still INT)
```
### 2. What did you expect to see? (Required)
The existing value `1` fits in `TINYINT`, so the no-reorg MODIFY COLUMN path should validate the data and succeed:
```
`a``b` tinyint NOT NULL
```
### 3. What did you see instead (Required)
The DDL job rolls back with a syntax error from the internally generated check SQL:
```
DDL job rollback
You have an error in your SQL syntax ... near "` FROM `d`.`t` WHERE (`a`b` < -128 OR `a`b` > 127) LIMIT 1"
```
and the column type is unchanged (`int`). The generated internal SQL contains `` `a`b` `` — the backtick inside the identifier is not escaped, producing unparseable SQL.
### 4. What is your TiDB version? (Required)
Reproduced on v8.5.7 (release-8.5, git 1fdc13626a). Root cause confirmed by code inspection in master @ 6f5bfe198f (2026-08-01), which carries identical code.
### Root cause analysis
The no-row-reorg MODIFY COLUMN optimization (#63965, later widened by #63465) generates an internal SQL statement to check whether existing data fits the new column type. `buildCheckSQLFromModifyColumn` (`pkg/ddl/modify_column.go:851-852` on release-8.5, `:862-864` on master) wraps identifiers with plain string concatenation:
```go
checkColName := fmt.Sprintf("`%s`", oldCol.Name.O)
tableName := fmt.Sprintf("`%s`.`%s`", dbName.O, tblName.O)
```
Backticks inside the identifier are never escaped, so a column named `` a`b `` produces `` SELECT `a`b` FROM ... `` — invalid SQL. `checkModifyColumnData` (`modify_column.go:816`) executes it via `ExecRestrictedSQL` and any error aborts the job, causing the rollback. Note the schema/table names are concatenated the same way, so a database or table name containing a backtick is affected identically.
Introduced by b1f10c8ec5 on release-8.5 (master a9bb0133e2, PR #63965; cherry-pick #64138); 40946f950b (#63465, cherry-pick #64123) later extended this fast path to more cases while keeping the unescaped construction.
Suggested direction: escape embedded backticks when quoting (e.g. `strings.ReplaceAll(name, "`", "``")`), or build the check statement through the AST/SQL builder instead of raw string concatenation.
Related: #63965 (original PR), #64123 (path widening), #63595 (tracking issue for skip-reorg MODIFY COLUMN).
Contributor guide
Research direction
Start in pkg/ddl/modify_column.go, especially buildCheckSQLFromModifyColumn and checkModifyColumnData, and reproduce the ALTER TABLE case with a backtick-containing column name. Trace the generated check SQL and add regression coverage for escaped identifiers, including database and table names if appropriate. Done means the MODIFY COLUMN succeeds and SHOW CREATE TABLE reports the new type without rollback.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, mysql, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100