cockroachdb / cockroachdb/cockroach

sql: lift restriction on descriptor references in domain CHECK expressions

Open
#171,536 0 comments 0 reactions 1 assignee Claimed by @bghal View on GitHub
A-schema-changes A-sql-datatypes C-enhancement O-agent T-sql-foundations
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.