cockroachdb / cockroachdb/cockroach

sql: enum comparison error from in-place type hydration

Open
#174,886 3 comments 0 reactions 1 assignee Assigned to @ZhouXing19 View on GitHub
A-sql-datatypes A-sql-execution A-sql-optimizer branch-release-24.2 C-bug O-agent T-sql-queries
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

Queries against a table with a user-defined `ENUM` column (including the implicit
`crdb_internal_region` enum of a `REGIONAL BY ROW` table) can fail with the internal error

```
internal error: comparison of two different versions of enum oid : versions and
```

when the enum's descriptor version changes — e.g. `ALTER TYPE ... ADD VALUE`, or any change that
bumps the type version, such as adding a back-reference when a new `REGIONAL BY ROW` table is created
— while cached query plans that reference the type are still in use.

**Root cause**

Type hydration mutates a **shared** `*types.T` in place. `EnsureTypeIsHydrated` ends in
`ensureTypeMetadataIsHydrated`, which writes the resolved version directly into the caller's
`TypeMeta`:

- [`pkg/sql/catalog/typedesc/hydrate.go:119`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/catalog/typedesc/hydrate.go#L119) — `tm.Version = version` (plus `tm.SetEnumData(...)`).

That object is the same `*types.T` a table column exposes via `column.GetType()`; it is also
registered as a memo's user-defined-type staleness sentinel and is referenced by fetch specs and by
the partition-prefix datums cached in the optimizer's `TableMeta`. Execution-time hydration can run
with a descriptor collection that resolves a **newer** version than the plan was built with (the
gateway's own flow of a distributed plan uses a fresh `descs.Collection`), so an ordinary execution
can advance this shared object from version A to version B underneath a plan that froze datums at
version A. Because the write also advances the memo's sentinel, `Memo.IsStale` stops detecting the
change and the stale plan keeps being reused.

**Two variants, one root**

*Variant 1 — histogram filtering (single session, deterministic).* After the sentinel is advanced, a
plan reused via the prepared-statement / query cache compares a histogram bound frozen at the old
version against a freshly-resolved value at the new version, during stats estimation
([`pkg/sql/opt/props/histogram.go:443`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/props/histogram.go#L443)).
The old version prints first (e.g. "88 and 89").

*Variant 2 — partition-prefix consolidation (cross-session, needs concurrency + the query cache).* In
a `REGIONAL BY ROW` index scanned with no region predicate, planning consolidates one span per region
partition and reaches `searchPrefixes`
([`pkg/sql/opt/constraint/locality.go`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/constraint/locality.go)),
comparing a partition-prefix datum against an index-constraint span key. The prefix datums live in
the cached `PrefixSorter` on `TableMeta.indexPartitionLocalities`, and `TableMeta.copyFrom`
([`pkg/sql/opt/table_meta.go:327`](https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/opt/table_meta.go#L327))
shares that structure **by reference** across every memo that reuses the statement via the query
cache. When one session's distributed execution hydrates the shared column type to the new version in
place, the shared prefix datum reads the new version while the span key in another session's reused
memo is still at the old version, so `searchPrefixes` compares new vs old. Here the new version
prints first (e.g. "89 and 88"). This variant is not reproducible from a single session — a single
session rebuilds both sides at one version — which is why it only clears when the query cache is
disabled.

**Reproduction**

Both variants are reproducible with self-contained SQL and no product changes; both are
configuration- and (for variant 2) concurrency-sensitive. Runnable, self-contained Go tests for each
variant are posted as comments on this issue.

**Fix direction**

Because both variants stem from the single in-place write, a fix at the writer — hydrating into a
copy rather than mutating the shared object — closes both: the memo's sentinel is no longer advanced
(so `IsStale` correctly rebuilds), and no shared prefix datum is poisoned across sessions. This is the
same defect class the codebase has previously fixed by making hydration copy instead of share —
recursive `CopyForHydrate` for tuple/array types (#118691), copying composite type elements in
`AsTypesT` (#119866), copying implicit-record type elements (#120054), and re-parsing shared domain
`CHECK` expressions per caller (#170295), which typically surfaced as race-detector / stress test
failures. The `ENUM` version-metadata write in `ensureTypeMetadataIsHydrated` is the instance not yet
covered.

(The cached `PrefixSorter` also shares a stale `EvalCtx *eval.Context` by reference via the same
`copyFrom`; that is a distinct data race, filed separately as #174887.)

**Expected behavior**

Changing a user-defined type's version must not corrupt cached or in-flight plans that reference an
older version. Plans built against an old type version should be detected as stale and rebuilt, and
type hydration should not mutate a `*types.T` that other plans or sessions hold.

**Impact**

User-visible internal errors (`XX000`) on otherwise valid queries against enum-typed tables during
and after a type-version change, which can persist while affected plans remain cached.

**Environment**

- CockroachDB version: affects v24.2+ (when the enum version assertion was added; the underlying
in-place hydration write is older but was previously benign). Reproduced on master and 26.2.x.
- Isolation: affects both `READ COMMITTED` and `SERIALIZABLE`. `READ COMMITTED` is the easy
trigger (its stepped read timestamp readily produces the poisoning in-place write); once any
session has poisoned the shared object, a `SERIALIZABLE` session reusing the cached plan hits the
assertion too (confirmed).
- Client app: server-side prepared statements over the pgwire extended protocol

Contributor guide

Open the contributing guide

Research direction

Start in pkg/sql/catalog/typedesc/hydrate.go at ensureTypeMetadataIsHydrated and inspect the TypeMeta update, then follow the related memo and table metadata paths in pkg/sql/opt/table_meta.go, pkg/sql/opt/props/histogram.go, and pkg/sql/opt/constraint/locality.go. Run the self-contained Go tests posted in the issue comments and verify that type-version changes leave older plans stale and prevent enum comparison errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.