cockroachdb / cockroachdb/cockroach
sql: lift restriction on descriptor references in domain CHECK expressions
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
## Summary
Domain `CHECK` expressions cannot safely reference other descriptors
(sequences, functions, or user-defined types) because the type descriptor
does not yet establish back-references to those objects. Without
back-references, a referenced object can be dropped while the domain still
depends on it, leaving the domain in a broken state.
This issue tracks the multi-step plan to lift that restriction for both
`CREATE DOMAIN ... CHECK` and `ALTER DOMAIN ... ADD CONSTRAINT ... CHECK`.
It is the CHECK-expression complement to #171439, which tracks the same
work for `DEFAULT` expressions.
## Background
The `ALTER DOMAIN ... ADD CONSTRAINT ... CHECK` path in the declarative
schema changer ([PR #314](https://github.com/cockroachdb/cockroach/pull/314))
explicitly rejects CHECK expressions that reference sequences, functions,
or UDTs. See [alter_domain.go:210-214](https://github.com/cockroachdb/cockroach/blob/1671d9e83741fa4718b164e585a0965888985696/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_domain.go#L210-L214):
\`\`\`go
if err := schemaexpr.CheckNoDescriptorRefsInDomainExpr(
\"CHECK\", tn.Object(), typedExpr, baseType,
); err != nil {
panic(err)
}
\`\`\`
The `CREATE DOMAIN ... CHECK` path in [create_type.go:581-585](https://github.com/cockroachdb/cockroach/blob/1671d9e83741fa4718b164e585a0965888985696/pkg/sql/create_type.go#L581-L585)
does **not** have an equivalent restriction on master today. It only
type-checks the expression and stores the serialized string:
\`\`\`go
for i, c := range n.DomainConstraints {
if _, err := schemaexpr.TypeCheckDomainCheckExpr(params.ctx, params.p.SemaCtx(), c.Expr, baseType); err != nil {
return pgerror.Wrapf(err, pgcode.InvalidObjectDefinition,
\"invalid CHECK expression for domain %s\", typeName.Type())
}
...
}
\`\`\`
The result is inconsistent behavior: `ALTER DOMAIN ... ADD CONSTRAINT
... CHECK` explicitly rejects CHECK expressions that `CREATE DOMAIN
... CHECK` silently accepts (with no protection against the referenced
object being dropped).
## Resolution Plan
Three sequential parts, mirroring the plan in #171439:
### Part 1 — Fix `CREATE DOMAIN ... CHECK` correctness
Make `CREATE DOMAIN` reject CHECK expressions that reference descriptors,
using the same `schemaexpr.CheckNoDescriptorRefsInDomainExpr` helper that
`ALTER DOMAIN ... ADD CONSTRAINT ... CHECK` already uses. This closes the
silent-acceptance gap and gives both statements consistent behavior
before any larger work begins.
### Part 2 — Implement safe back-references for CHECK dependencies
Establish back-references from sequences, functions, and user-defined
types into the type descriptor when a domain CHECK expression
references them, so the referenced object cannot be dropped while the
domain still depends on it. This parallels the back-reference plumbing
described in Part 2 of #171439, but for CHECK constraints — each domain
CHECK constraint carries its own set of referenced IDs (multiple CHECK
constraints per domain, unlike a single DEFAULT).
### Part 3 — Lift the restriction in both paths
Once back-references are in place, remove the rejection in
`ALTER DOMAIN ... ADD CONSTRAINT ... CHECK` and the equivalent rejection
added by Part 1 in `CREATE DOMAIN ... CHECK`, so domain CHECK
constraints can reference sequences, functions, and UDTs.
## Related
- Companion issue (DEFAULT expressions): cockroachdb/cockroach#171439
- PR that added the ALTER DOMAIN CHECK rejection: cockroachdb/cockroach#314
Epic CRDB-66030
Jira issue: CRDB-64656
Contributor guide
Assessment
This issue has not been assessed yet.