cockroachdb / cockroachdb/cockroach
crosscluster/logical: checkOutboundReferences does not detect UDF references in CHECK constraints, partial indexes, or policies
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
`checkOutboundReferences` in LDR's schema validation
([logical_replication_helpers.go:100-117](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/logical_replication_helpers.go#L100-L117))
is intended to reject destination tables that reference user-defined functions,
sequences, or triggers. However, it only checks two sources of UDF references:
1. `col.UsesFunctionIds` — catches UDFs in column defaults and computed columns
2. `dst.Triggers` — catches triggers on the table
UDFs can also be referenced from locations that are **not** checked:
- **CHECK constraint expressions** — UDF IDs are embedded in the expression
string and must be extracted via `schemaexpr.GetUDFIDsFromExprStr`
([structured.go:505-518](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/structured.go#L505-L518))
- **Partial index predicates** — same extraction mechanism
([structured.go:491-498](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/structured.go#L491-L498))
- **Row-level security policies** — `Policies[i].DependsOnFunctions`
([structured.go:487-488](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/structured.go#L487-L488))
The table descriptor already has a comprehensive
`GetAllReferencedFunctionIDs()` method
([structured.go:467-501](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/structured.go#L467-L501))
that checks all of these locations, but `checkOutboundReferences` does not use it.
**Why this bug is latent**
This bug is currently difficult to exercise in practice because other validation
checks mask it:
- **Without `SKIP SCHEMA CHECK`:** If only the destination has a UDF in a CHECK
constraint, `checkCheckConstraintsMatch` rejects the stream because the CHECK
expressions differ between source and destination. The incomplete
`checkOutboundReferences` is never the gate.
- **UDF on both sides with matching expressions:** `checkCheckConstraintsMatch`
passes, and `checkOutboundReferences` doesn't catch it — but in this case the
source already validated rows against the same CHECK, so the UDF evaluating on
the destination is unlikely to cause problems.
The bug becomes exercisable with **`SKIP SCHEMA CHECK`**: the equivalence checks
(`checkCheckConstraintsMatch`, `checkSrcDstColsMatch`, etc.) are bypassed, but
`checkOutboundReferences` is called unconditionally — it is clearly intended
as a hard safety check. With `SKIP SCHEMA CHECK`, a UDF in a CHECK constraint
on only the destination slips through undetected. During replication writes the
destination evaluates the UDF, which could reject valid replicated rows or cause
unintended side effects.
**Suggested fix**
Use `GetAllReferencedFunctionIDs()` in `checkOutboundReferences` (or replicate
its logic) to cover CHECK constraints, partial index predicates, and policies
in addition to column-level references.
**Code References:**
- [logical_replication_helpers.go:100-117](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/logical_replication_helpers.go#L100-L117) — incomplete `checkOutboundReferences`
- [structured.go:467-501](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/structured.go#L467-L501) — comprehensive `GetAllReferencedFunctionIDs()`
- [logical_replication_helpers.go:25-66](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/tabledesc/logical_replication_helpers.go#L25-L66) — `CheckLogicalReplicationCompatibility` showing `checkOutboundReferences` is unconditional
Epic: none
Jira issue: CRDB-63712
Contributor guide
Assessment
This issue has not been assessed yet.