cockroachdb / cockroachdb/cockroach
backfill: addConstraints mutates range-loop copy for UniqueWithoutIndex constraints
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
In `pkg/sql/backfill.go:addConstraints`, the retry/rollback path for UniqueWithoutIndex (UWI) constraints mutates a range-loop copy of a value-type slice element instead of the actual descriptor element. This means that when a UWI constraint already exists on the table descriptor (e.g., during transaction retry or rollback of DROP CONSTRAINT), the intended `Validity = Validating` update is silently lost, and the constraint is persisted with its stale Validity (e.g., `Dropping`).
The Check and FK constraint branches handle this correctly, making this an inconsistency.
The impact is that a UWI constraint undergoing rollback of DROP CONSTRAINT could remain in `Dropping` state, potentially causing it to be dropped later or to stop being enforced, allowing duplicate data that violates the uniqueness guarantee.
**Affected code**
- **Bug site:** [`backfill.go:706`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/backfill.go#L706) — `c.Validity = descpb.ConstraintValidity_Validating` mutates a copy
- **Loop declaration:** [`backfill.go:696`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/backfill.go#L696) — `for _, c := range scTable.UniqueWithoutIndexConstraints` creates value copies
- **Correct FK pattern for comparison:** [`backfill.go:643-644`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/backfill.go#L643) — `for j := range scTable.OutboundFKs { def := &scTable.OutboundFKs[j] }`
**Suggested fix**
Change the loop to iterate by index (matching the FK pattern):
```go
for j := range scTable.UniqueWithoutIndexConstraints {
// ...
scTable.UniqueWithoutIndexConstraints[j].Validity = descpb.ConstraintValidity_Validating
}
```
**To Reproduce**
This bug requires internal/unit-test-level access to trigger (injecting a UWI constraint in `Dropping` state on a table descriptor). No SQL repro is feasible.
**Expected behavior**
After rollback, the UWI constraint's Validity should be set to `Validating`, not remain in `Dropping`.
**Environment:**
- CockroachDB version: master (at least since the UWI constraint support was added)
Jira issue: CRDB-62041
Contributor guide
Assessment
This issue has not been assessed yet.