cockroachdb / cockroachdb/cockroach

crosscluster/logical: checkOutboundReferences does not detect UDF references in CHECK constraints, partial indexes, or policies

Open
#169,918 2 comments 0 reactions 0 assignees View on GitHub
A-cdc A-cross-cluster-replication C-bug O-agent T-cdc
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

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.