cockroachdb / cockroachdb/cockroach
sql/schemachanger: ADD COLUMN with a failing CHECK validates past the non-revertible boundary and leaves the table unusable for DDL
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
`ALTER TABLE ... ADD COLUMN c INT NOT NULL DEFAULT 1 CHECK (c > 10)` on a table that already has rows is planned so that the CHECK constraint is added and validated in `PostCommitNonRevertiblePhase`, after the primary index swap. The NOT NULL and index validations run in the revertible phase, but the `CheckConstraint` element only transitions `ABSENT -> WRITE_ONLY` in non-revertible stage 1 and is validated in stage 2. When that validation fails on existing rows, which is a perfectly legitimate outcome, there is no legal rollback: every rollback attempt fails descriptor validation with
```
error executing PostCommitNonRevertiblePhase stage 1 of 4 with 13 MutationType ops: relation "t" (104): primary index "t_pkey" must contain column ID 3 in either key or store columns
```
What the user sees depends on the version, but the table ends up in the same bad state in all of them:
| Version | First ALTER | Job | Later DDL on the table |
|---|---|---|---|
| v26.3.1 | blocks indefinitely | `reverting` forever | blocks behind the reverting job |
| master dev build from late July 2026 | returns the CHECK error after ~1 min | `failed` (rollback skipped) | hangs forever in the concurrent-schema-change wait, no job is created |
| master as of 2026-09-14 | blocks indefinitely | `reverting` forever | blocks behind the reverting job |
The July build behaved differently because of the fix for #172229. Before that fix, the failure at the non-revertible stage was misrouted to `OnFailOrCancel` because the resumer's cached payload still said the job was cancelable, and the CHECK violation was not yet classified as a permanent job error, so the invalid rollback was retried forever; that is the loop 26.3.x still shows, since the fix is not on release-26.3. With the fix, the violation is permanent and the then-current `OnFailOrCancel` gate skipped the rollback once the job was already reverting, so the job went to `failed` without any rollback. A later change (August 2026) keys that gate on the rollback's own error instead; the descriptor validation error is not classified permanent, so current master retries the invalid rollback forever, by a different route than 26.3.x.
Descriptor state after the failed validation, identical across versions since a failed rollback never commits:
- column `c INT8 NOT NULL DEFAULT 1` is public and every row was backfilled with `c = 1`;
- the new primary index (id 2, `t_pkey`, storing `v, c`) is public; the old primary index remains as a `DROP DELETE_ONLY` mutation with a placeholder name and is never garbage collected;
- `check_c CHECK (c > 10)` is present with validity `Validating`, shown by `SHOW CREATE TABLE` as `NOT VALID`, and enforced on writes. Inserts that rely on the column default are rejected by the constraint, while the two existing rows violate it;
- `declarativeSchemaChangerState` still names the job (25 targets). On the build where the job is `failed`, any subsequent `ALTER TABLE` enters `waitForDescriptorSchemaChanges`, which only checks `desc.HasConcurrentSchemaChanges()` and never consults the jobs table, so it spins forever and never creates a job;
- `crdb_internal.invalid_objects` reports nothing.
**To Reproduce**
Single node, all default settings:
```sql
CREATE TABLE t (id INT PRIMARY KEY, v INT);
INSERT INTO t VALUES (1, 1), (2, 2);
ALTER TABLE t ADD COLUMN c INT NOT NULL DEFAULT 1 CHECK (c > 10);
-- blocks (26.3.1, current master) or returns
-- "validation of CHECK "c > 10:::INT8" failed on row: id=1, v=1, c=1" (July master)
```
In another session:
```sql
SELECT status, running_status FROM [SHOW JOBS] WHERE job_type = 'NEW SCHEMA CHANGE';
-- reverting | Pending: Validating CHECK constraint (1 operation) — PostCommitNonRevertible phase (stage 2 of 4).
SHOW CREATE TABLE t; -- c is present, check_c is NOT VALID
INSERT INTO t (id, v) VALUES (3, 3); -- fails on check_c
ALTER TABLE t ADD COLUMN d INT; -- never returns
```
`EXPLAIN (DDL) ALTER TABLE t ADD COLUMN c INT NOT NULL DEFAULT 1 CHECK (c > 10)` shows the CheckConstraint transitions confined to `PostCommitNonRevertiblePhase` stages 1 to 3, while `ValidateColumnNotNull` and `ValidateIndex` run in `PostCommitPhase` stage 7.
**Expected behavior**
The statement fails with the CHECK violation, the job reaches `failed`, and the table is left exactly as it was: no column `c`, no constraint, no lingering index mutation, no job state on the descriptor, and later DDL on the table works.
Two things need to change:
1. Planning: a validation that can fail on user data must not be scheduled past the non-revertible boundary. The CHECK on a newly added column should be added and validated in the revertible phase, before the primary index swap, the way the NOT NULL constraint already is. Dependency rules for `CheckConstraint` relative to the column and primary index transitions need adjusting, and an end-to-end fixture for ADD COLUMN with a failing inline CHECK should be added (none exists today).
2. Job handling: `OnFailOrCancel` should never let a job reach a terminal state while the descriptor still points at it. If a rollback is abandoned, the job state must be cleared from the descriptor or the job must not be abandoned.
**Environment:**
- Verified on v26.3.1, a master dev build from 2026-07-29, and master at 2026-09-14. The plan shape is not new, so older releases are expected to be affected; the branch labels for 26.2, 25.4 and 25.2 are for triage and have not been verified.
- Server OS: macOS arm64 (single node); nothing platform specific.
- Client: `cockroach sql`.
**Additional context**
Impact: one ordinary DDL statement with a CHECK that the column's own default violates leaves the table permanently unable to accept further schema changes, and in most versions also leaves the client session blocked. `crdb_internal.invalid_objects` does not flag the table.
Found while testing mixed-version behaviour of declarative CREATE TABLE for #175064; the failure is unrelated to upgrades. Closest existing issue is #172229, which covered the transient-error variant of the same rollback loop. See also #131405.
Jira issue: CRDB-68502
Epic CRDB-68473
Contributor guide
Research direction
Start with the reproduced ALTER TABLE statement and its EXPLAIN (DDL) output, then trace the CheckConstraint transitions through PostCommitNonRevertiblePhase and the existing ValidateColumnNotNull and ValidateIndex scheduling. Read the OnFailOrCancel and waitForDescriptorSchemaChanges paths described in the issue. Done means the failing CHECK leaves no schema-change state and later DDL succeeds, with an end-to-end fixture covering the failure.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100