sql: track and update UDT versions in prepared statements by ZhouXing19 · Pull Request #161827 · cockroachdb/cockroach

ZhouXing19 · 2026-01-27T00:23:00Z

Previously, prepared statements referencing UDTs could fail with version
mismatch errors after the UDT was altered (e.g., via ALTER TYPE
ADD|RENAME|DROP VALUE). This occurred because the prepared statement
retained stale type information obtained from the type check during the
preparing stage, while new executions used updated type versions.

This commit fixes it by tracking all UDTs referenced in prep stmts
during preparation, checking UDT versions for staleness before each execution
and updating UDT references and re-parsing when versions change.

Note: Our behavior intentionally differs from PG, which resolves
enum values to version-agnostic OIDs.

Example:
PREPARE p AS SELECT $1::my_enum = 'enum_a';
ALTER TYPE my_enum RENAME VALUE 'enum_a' TO 'enum_aa';
EXECUTE p('enum_aa'); -- PG returns true, CRDB returns error.

Fixes: #132105

Release note (bug fix): Fixed prepared statements failing with "version
mismatch" errors when user-defined types are modified between preparation
and execution. Prepared statements now automatically detect UDT changes
and re-parse to use current type definitions.

blathers-crl · 2026-01-27T00:23:04Z

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

cockroach-teamcity · 2026-01-27T00:23:11Z

This change is

yuzefovich

@yuzefovich reviewed 2 files and all commit messages, and made 3 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @michae2, and @ZhouXing19).

-- commits line 12 at r1:
So you're saying that - when it gets to the DistSQL execution engine - we are using the DEnum with the newer version? I.e. the DEnum with old version is only stored within the prepared statement and is only used during the logical planning (i.e. when we fill in placeholders, or something like that)?

-- commits line 15 at r1:
I don't think it was added only because of this reason - true, the check was inspired by a bug we saw around stale stats, but then the check was added to catch this type of bugs elsewhere too.

pkg/sql/logictest/testdata/logic_test/prepare line 1781 at r1 (raw file):


statement ok
ALTER TYPE enum132105 RENAME VALUE 'hi' to 'pi';

nit: let's also add a test case where we drop hi (or pi) from the enum.

ZhouXing19 · 2026-01-27T17:20:24Z

-- commits line 12 at r1:
Hmm, i'm not sure if i fully understand the question here, but let me try to explain with the example test.

PREPARE p AS SELECT $1::te = 'hi';
ALTER TYPE te ADD VALUE 'hello';
EXECUTE p('hi');

At PREPARE, for the rhs (hi), we convert it into a tree.DEnum with version 1 (old), and stored it in the prepared statement.
At EXECUTE, the lhs is now resolved into a tree.DEnum with version 3 (new), and replace the placeholder ($1) when re-optimizing the new memo.
And we hit the panic during the re-optimization as well - at the stage of normalization when we try to evaluate the eq operation.

So re

when it gets to the DistSQL execution engine - we are using the DEnum with the newer version?

We hit this panic prior to we even reach the DistSQL execution engine, but when we make the plan for the EXECUTE. And it's the mixed use of DEnum with both new and old versions caused the panic. Does this answer your question?

yuzefovich

@yuzefovich made 1 comment.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @michae2, and @ZhouXing19).

-- commits line 12 at r1:

Previously, ZhouXing19 (Jane Xing) wrote…

Hmm, i'm not sure if i fully understand the question here, but let me try to explain with the example test.
PREPARE p AS SELECT $1::te = 'hi';
ALTER TYPE te ADD VALUE 'hello';
EXECUTE p('hi');
At PREPARE, for the rhs (hi), we convert it into a tree.DEnum with version 1 (old), and stored it in the prepared statement.

At EXECUTE, the lhs is now resolved into a tree.DEnum with version 3 (new), and replace the placeholder ($1) when re-optimizing the new memo.

And we hit the panic during the re-optimization as well - at the stage of normalization when we try to evaluate the eq operation.

So re

when it gets to the DistSQL execution engine - we are using the DEnum with the newer version?

We hit this panic prior to we even reach the DistSQL execution engine, but when we make the plan for the EXECUTE. And it's the mixed use of DEnum with both new and old versions caused the panic. Does this answer your question?

I meant that - after applying this fix, which removes the assertion - since we replaced DEnum v1 with DEnum v3 during the re-optimization, the logical plan that gets to the DistSQL physical planner and the execution engine will only reference DEnum v3, right? Based on your comment it seems to be the case.

In other words, we have mixed-version comparison only during re-optimization of the memo, and if we remove the assertion, no other mixed-version enum comparisons are ever performed, correct?

If so, what do you think about about extending CompareContext with a method indicating whether mixed-version DEnum comparison is ok, and then eval.Context would be modified during the re-optimization of the memo to allow the mixed-version DEnum comparison, and only in that case? This would allow us to keep the assertion for all other use cases while exempting this particular case with prepared statements that we think should be allowed.

michae2

I'll read this PR more deeply later today, but at first glance I don't agree that we should remove this version check.

ZhouXing19 · 2026-01-27T18:46:10Z

-- commits line 12 at r1:

if we remove the assertion, no other mixed-version enum comparisons are ever performed, correct?

Yes, and at least this is what debugger tells me. Even with the version check removed, we only hit DEnum.Compare() once. I think this makes sense as well, since this function is to fold the comparison to a bool at the normalization, so I don't think we will need to re-compare the datums in later phases.

If so, what do you think about about extending CompareContext with a method indicating whether mixed-version DEnum comparison is ok, and then eval.Context would be modified during the re-optimization of the memo to allow the mixed-version DEnum comparison, and only in that case? This would allow us to keep the assertion for all other use cases while exempting this particular case with prepared statements that we think should be allowed.

Yup, I like this idea! Feels a safer change.

michae2

@michae2 made 1 comment.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @yuzefovich, and @ZhouXing19).

-- commits line 12 at r1:

whether mixed-version DEnum comparison is ok

Could someone spell out the scenario in which we are certain that mixed-version DEnum comparison is ok?

michae2

@michae2 made 1 comment.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @yuzefovich, and @ZhouXing19).

-- commits line 12 at r1:
The version check is protecting us from a real problem. For example, consider this scenario:

CREATE TYPE e AS ENUM ('e', 'f', 'g');
PREPARE p AS SELECT $1::e = 'g';
ALTER TYPE e RENAME VALUE 'g' TO 'h';
ALTER TYPE e RENAME VALUE 'e' TO 'g';
EXECUTE p ('h');

The last statement should return false, because after the two renames, 'h' != 'g'. Currently it hits the version check and fails with an error. But if we remove the version check, it returns true.

In the issue, you wrote:

A possible fix is to have a staleness check when retrieving the prepared stmt from the cache, and re-parse the stmt if the enum types used in the statement is found stale.

I think something like this is the way to go. We should invalidate the cached prepared statement after the type changes.

ZhouXing19 · 2026-01-28T00:53:07Z

-- commits line 12 at r1:

Previously, michae2 (Michael Erickson) wrote…

The version check is protecting us from a real problem. For example, consider this scenario:
CREATE TYPE e AS ENUM ('e', 'f', 'g');
PREPARE p AS SELECT $1::e = 'g';
ALTER TYPE e RENAME VALUE 'g' TO 'h';
ALTER TYPE e RENAME VALUE 'e' TO 'g';
EXECUTE p ('h');
The last statement should return false, because after the two renames, 'h' != 'g'. Currently it hits the version check and fails with an error. But if we remove the version check, it returns true.

In the issue, you wrote:

A possible fix is to have a staleness check when retrieving the prepared stmt from the cache, and re-parse the stmt if the enum types used in the statement is found stale.

I think something like this is the way to go. We should invalidate the cached prepared statement after the type changes.

Actually, pg returns true with the test you provided:

defaultdb=# CREATE TYPE e AS ENUM ('e', 'f', 'g');
CREATE TYPE
defaultdb=# PREPARE p AS SELECT $1::e = 'g';
PREPARE
defaultdb=# ALTER TYPE e RENAME VALUE 'g' TO 'h';
ALTER TYPE
defaultdb=# ALTER TYPE e RENAME VALUE 'e' TO 'g';
ALTER TYPE
defaultdb=# EXECUTE p ('h');
 ?column? 
----------
 t
(1 row)

I need to confirm with the pg source code, but my guess is that we are comparing the logical format of the enum rather than the physical format (i.e. the string representation).

My concern for the re-parsing the stmt is that 1. it might add overhead to traverse the whole stmt, and 2. we do need to track the logical representation of the stmt, not just the string representation of the enum entry, for which re-parsing the original string sql could lead to incorrect result.

michae2

@michae2 made 1 comment.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @yuzefovich, and @ZhouXing19).

-- commits line 12 at r1:

Previously, ZhouXing19 (Jane Xing) wrote…

Actually, pg returns true with the test you provided:
defaultdb=# CREATE TYPE e AS ENUM ('e', 'f', 'g');
CREATE TYPE
defaultdb=# PREPARE p AS SELECT $1::e = 'g';
PREPARE
defaultdb=# ALTER TYPE e RENAME VALUE 'g' TO 'h';
ALTER TYPE
defaultdb=# ALTER TYPE e RENAME VALUE 'e' TO 'g';
ALTER TYPE
defaultdb=# EXECUTE p ('h');
 ?column? 
----------
 t
(1 row)
I need to confirm with the pg source code, but my guess is that we are comparing the logical format of the enum rather than the physical format (i.e. the string representation).

My concern for the re-parsing the stmt is that 1. it might add overhead to traverse the whole stmt, and 2. we do need to track the logical representation of the stmt, not just the string representation of the enum entry, for which re-parsing the original string sql could lead to incorrect result.

Interesting. Hmm. It kinda seems like a bug in PG, too.

michae2=# CREATE TYPE e AS ENUM ('e', 'f', 'g');
CREATE TYPE
michae2=# PREPARE p AS SELECT $1::e = 'g';
PREPARE
michae2=# ALTER TYPE e RENAME VALUE 'g' TO 'h';
ALTER TYPE
michae2=# ALTER TYPE e RENAME VALUE 'e' TO 'g';
ALTER TYPE
michae2=# EXECUTE p ('h');
 ?column?
----------
 t
(1 row)

michae2=# SELECT 'h'::e = 'g';
 ?column?
----------
 f
(1 row)

michae2=# PREPARE p2 AS SELECT $1::e = 'g';
PREPARE
michae2=# EXECUTE p2 ('h');
 ?column?
----------
 f
(1 row)

michae2=# EXECUTE p ('h');
 ?column?
----------
 t
(1 row)

ZhouXing19 · 2026-01-28T01:29:55Z

-- commits line 12 at r1:

Previously, michae2 (Michael Erickson) wrote…

Interesting. Hmm. It kinda seems like a bug in PG, too.

michae2=# CREATE TYPE e AS ENUM ('e', 'f', 'g');
CREATE TYPE
michae2=# PREPARE p AS SELECT $1::e = 'g';
PREPARE
michae2=# ALTER TYPE e RENAME VALUE 'g' TO 'h';
ALTER TYPE
michae2=# ALTER TYPE e RENAME VALUE 'e' TO 'g';
ALTER TYPE
michae2=# EXECUTE p ('h');
 ?column?
----------
 t
(1 row)

michae2=# SELECT 'h'::e = 'g';
 ?column?
----------
 f
(1 row)

michae2=# PREPARE p2 AS SELECT $1::e = 'g';
PREPARE
michae2=# EXECUTE p2 ('h');
 ?column?
----------
 f
(1 row)

michae2=# EXECUTE p ('h');
 ?column?
----------
 t
(1 row)

This is the pg code that uses the oids to compare enums.

I think the oid comparison can explain the test above:

CREATE TYPE e AS ENUM ('e', 'f', 'g');
-- e:1, f:2, g:3

PREPARE p AS SELECT $1::e = 'g';
-- $1 == 3

ALTER TYPE e RENAME VALUE 'g' TO 'h';
-- e:1, f:2, h:3

ALTER TYPE e RENAME VALUE 'e' TO 'g';
-- g:1, f:2, h:3

EXECUTE p ('h');
-- 3 == 3 -> true

SELECT 'h'::e = 'g';
-- 3 == 1 -> false

PREPARE p2 AS SELECT $1::e = 'g';
-- $1 == 1

EXECUTE p2 ('h');
-- 3 == 1 -> false

EXECUTE p ('h');
-- 3 = 3 -> true

michae2

@michae2 made 1 comment.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @yuzefovich, and @ZhouXing19).

-- commits line 12 at r1:

Previously, ZhouXing19 (Jane Xing) wrote…

This is the pg code that uses the oids to compare enums.

I think the oid comparison can explain the test above:

CREATE TYPE e AS ENUM ('e', 'f', 'g');
-- e:1, f:2, g:3

PREPARE p AS SELECT $1::e = 'g';
-- $1 == 3

ALTER TYPE e RENAME VALUE 'g' TO 'h';
-- e:1, f:2, h:3

ALTER TYPE e RENAME VALUE 'e' TO 'g';
-- g:1, f:2, h:3

EXECUTE p ('h');
-- 3 == 3 -> true

SELECT 'h'::e = 'g';
-- 3 == 1 -> false

PREPARE p2 AS SELECT $1::e = 'g';
-- $1 == 1

EXECUTE p2 ('h');
-- 3 == 1 -> false

EXECUTE p ('h');
-- 3 = 3 -> true

Good argument, but I still think we need some kind of prepared statement invalidation / re-type-checking. I don't think PG necessarily has the right semantics here.

Here's another scenario to consider:

CREATE TYPE i AS ENUM ('i', 'j', 'k');
CREATE TABLE t (i i);
CREATE TABLE u (i STRING);
PREPARE p2 AS INSERT INTO t VALUES ('k');
PREPARE p3 AS INSERT INTO u VALUES ('k'::i);
ALTER TYPE i DROP VALUE 'k';

-- this fails because somewhere we try converting to the new version of the type, and get an error
EXECUTE p2;

-- however, this succeeds. I don't think this is valid; we should invalidate the cached prepared statement and also fail this one
EXECUTE p3;

michae2

@michae2 made 1 comment.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @yuzefovich, and @ZhouXing19).

-- commits line 12 at r1:

Previously, michae2 (Michael Erickson) wrote…

Good argument, but I still think we need some kind of prepared statement invalidation / re-type-checking. I don't think PG necessarily has the right semantics here.

Here's another scenario to consider:

CREATE TYPE i AS ENUM ('i', 'j', 'k');
CREATE TABLE t (i i);
CREATE TABLE u (i STRING);
PREPARE p2 AS INSERT INTO t VALUES ('k');
PREPARE p3 AS INSERT INTO u VALUES ('k'::i);
ALTER TYPE i DROP VALUE 'k';

-- this fails because somewhere we try converting to the new version of the type, and get an error
EXECUTE p2;

-- however, this succeeds. I don't think this is valid; we should invalidate the cached prepared statement and also fail this one
EXECUTE p3;

One more interesting scenario that I was curious about:

CREATE TYPE m AS ENUM ('m', 'n', 'o');
PREPARE p4 AS SELECT enum_range(NULL::m), enum_range('m'::m), enum_range('n'::m);
ALTER TYPE m RENAME VALUE 'n' TO 'p';
EXECUTE p4;

We actually differ from PG for this one. Here's PG 18:

 enum_range | enum_range | enum_range
------------+------------+------------
 {m,p,o}    | {m,p,o}    | {m,p,o}

and here's CRDB:

  enum_range | enum_range | enum_range
-------------+------------+-------------
  {m,p,o}    | {m,n,o}    | {m,n,o}

yuzefovich

I can't see where the enum's version is updated

crdb_internal_region is of the enum type, and I suspect it has something to do with schema changes. This test has existed for a while, yet we saw two failures (#161528, #161975) very recently, and I imagine it was due to a schema change / lease related PR. (Also I remember in the past there was an issue #144293 where we bumped the type descriptor version too frequently which wasn't necessary.)

It's probably orthogonal to this fix, so let's ignore that issue.

I have tried to find a case where a statement does depends on an enum but failed to be registered it in AllUserDefinedTypes, but i could not find one.

What I'm concerned about is an issue like #146126. In short, we build the stmt bundles also using AllUserDefinedTyped from the metadata, and the issue has an example where arrays of enums aren't represented there. We should probably add some tests with arrays of enums too in this patch.

@yuzefovich reviewed 2 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @michae2, and @ZhouXing19).

ZhouXing19 · 2026-02-03T22:31:36Z

Thanks for the pointer. The statement in #146126 -- SELECT \* FROM t; -- is not very relevant to this issue. In fact that query won't error with the udt version change, as the ast of this stmt doesn't contains a enum datum, thus doesn't actually reach the datum comparison function. But the fix for that issue itself seem not complicated, so i included it in the second commit, with tests with enum array added as well.

michae2

@michae2 made 2 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @yuzefovich, and @ZhouXing19).

pkg/sql/plan_opt.go line 196 at r3 (raw file):

					if md := cachedData.Memo.Metadata(); md != nil {
						udts = md.AllUserDefinedTypes()

IsStale() is supposed to detect when any of the types in md.userDefinedTypesSlice have changed. Why isn't that check working?

pkg/sql/conn_executor_exec.go line 374 at r3 (raw file):

		return nil
	}
	newStmt, err := parser.ParseOne(prep.SQL)

I'm a little confused by this. Which part of the AST changes when re-parsing after the ALTER TYPE statement? I would expect the AST to be the same (with a tree.StrVal("hi") for the constant 'hi' in the statement).

yuzefovich · 2026-02-03T23:21:15Z

But the fix for that issue itself seem not complicated, so i included it in the second commit, with tests with enum array added as well.

Nice! Let's extract that fix into a separate PR (since it seems independent of what we're fixing here).

yuzefovich · 2026-02-03T23:23:53Z

Oh, we now rely on the fix in the second commit for the test added with arrays of enums? I guess in that case keeping both commits together seems reasonable. Nvm me.

ZhouXing19

we now rely on the fix in the second commit for the test added with arrays of enums?

Actually we don't, sorry for the confusion. I combined the new test with the first commit now and will move the second commit mod those tests in a new PR.

@ZhouXing19 made 3 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball, @michae2, and @yuzefovich).

pkg/sql/conn_executor_exec.go line 374 at r3 (raw file):

Previously, michae2 (Michael Erickson) wrote…

I'm a little confused by this. Which part of the AST changes when re-parsing after the ALTER TYPE statement? I would expect the AST to be the same (with a tree.StrVal("hi") for the constant 'hi' in the statement).

The hi is actually converted from a tree.StrVal to an enum datum at type checking when building the memo for PREPARE. And in between PREPARE and EXECUTE, the enum type version changed. But the AST still has the enum datum of the old version. So this re-parse is to have AST back with atree.StrVal rather than the old enum datum.

pkg/sql/plan_opt.go line 196 at r3 (raw file):

Previously, michae2 (Michael Erickson) wrote…

IsStale() is supposed to detect when any of the types in md.userDefinedTypesSlice have changed. Why isn't that check working?

The checking worked -- it did invalidate the memo. But the tree.StrVal has been converted to an enum datum at type checking. So when EXECUTE, the new memo is still built based on the enum datum with the old version.

michae2

Thanks for doing that!

@michae2 reviewed 7 files and all commit messages, made 4 comments, and resolved 2 discussions.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @ZhouXing19).

pkg/sql/conn_executor_exec.go line 374 at r3 (raw file):

Previously, ZhouXing19 (Jane Xing) wrote…

The hi is actually converted from a tree.StrVal to an enum datum at type checking when building the memo for PREPARE. And in between PREPARE and EXECUTE, the enum type version changed. But the AST still has the enum datum of the old version. So this re-parse is to have AST back with atree.StrVal rather than the old enum datum.

Ahhh. Thank you for explaining. Now things are making sense. (Wow, it's pure evil that type checking mutates the AST! Why would we do that?! Oh well.)

pkg/sql/plan_opt.go line 196 at r3 (raw file):

Previously, ZhouXing19 (Jane Xing) wrote…

The checking worked -- it did invalidate the memo. But the tree.StrVal has been converted to an enum datum at type checking. So when EXECUTE, the new memo is still built based on the enum datum with the old version.

I see, thanks for explaining!

pkg/sql/opt/exec/execbuilder/testdata/prepare line 360 at r5 (raw file):

EXECUTE p132105arrcount;

subtest end

I'd also be curious to see how a prepared statement like this changes after ALTER TYPE enum132105 RENAME VALUE 'hi' to 'aloha';:

PREPARE p132105enumrange AS SELECT enum_range(NULL::enum132105), enum_range('hello'::enum132105);
EXECUTE p132105enumrange;

yuzefovich

Nice work!

Thoughts on backporting this? I'm kinda on the fence - we have seen this a couple of support tickets (https://github.com/cockroachlabs/support/issues/3229, https://github.com/cockroachlabs/support/issues/3246) around this bug plus CC clusters frequently hit them (mostly serverless users though); but at the same time this feels a bit risky. Perhaps we could backport this to 26.1 (and maybe 25.4) "default off" and enable only on serverless host clusters?

@yuzefovich reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @DrewKimball and @ZhouXing19).

ZhouXing19 · 2026-02-04T06:14:31Z

pkg/sql/opt/exec/execbuilder/testdata/prepare line 360 at r5 (raw file):

Previously, michae2 (Michael Erickson) wrote…

I'd also be curious to see how a prepared statement like this changes after ALTER TYPE enum132105 RENAME VALUE 'hi' to 'aloha';:
PREPARE p132105enumrange AS SELECT enum_range(NULL::enum132105), enum_range('hello'::enum132105);
EXECUTE p132105enumrange;

Good idea, added (and i hope i got your idea correctly here)

michae2

I think we should let this bake on master for a little while. It's a great fix, but it's always a bit risky to modify the phases of the front end.

Side comment: now that I'm looking at the definition of prep.Statement, I wonder if there's a similar bug hiding in the tree.PlaceholderTypesInfo. AFAICT we don't check the staleness of those types, either. (But we can follow up on that later.)

@michae2 made 5 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @DrewKimball, @yuzefovich, and @ZhouXing19).

pkg/sql/opt/exec/execbuilder/testdata/prepare line 360 at r5 (raw file):

Previously, ZhouXing19 (Jane Xing) wrote…

Good idea, added (and i hope i got your idea correctly here)

Thanks! Actually, I was hoping to see the output of enum_range. I'm hoping it will show the renamed type after something like this:

statement ok
PREPARE p132105enumrange AS SELECT enum_range(NULL::enum132105), enum_range('hello'::enum132105);

# should show hi
query TT
EXECUTE p132105enumrange;
----

statement ok
ALTER TYPE enum132105 RENAME VALUE 'hi' to 'aloha';

# should show aloha instead of hi
query TT
EXECUTE p132105enumrange;
----

pkg/sql/conn_executor_exec.go line 358 at r6 (raw file):

		if typ.TypeMeta.Version != toCheck.TypeMeta.Version {
			stale = true
			prep.UDTs[i] = toCheck

I just noticed this. Is this to try to fix up the UDTs slice? I think we need to throw away the memo(s) anyway, since we're re-parsing, so probably better to throw away the UDTs slice as well.

pkg/sql/conn_executor_exec.go line 365 at r6 (raw file):

// maybeReparsePrepStmt is to reparse the prepared statement so that the stored
// udt datum is up-to-date.

It would be nice to add something to this comment about why we need to re-parse (i.e. that type checking modifies the AST and so we need to re-parse to build a new AST if types change).

pkg/sql/conn_executor_exec.go line 379 at r6 (raw file):

	}
	prep.Statement = newStmt
	prep.AST = newStmt.AST

Hmm. Since we're re-parsing, I think we need to do a little more here to reset prep. I think we need to throw away all of the state that was based on the old AST. At the very least something like this:

prep.BaseMemo = nil
prep.GenericMemo = nil
prep.IdealGenericPlan = false
prep.Costs.Reset()
prep.HintsGeneration = 0

Also, we should add a call to log.Eventf here to indicate the re-parse in tracing.

ZhouXing19

@ZhouXing19 made 4 comments and resolved 1 discussion.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @DrewKimball, @michae2, and @yuzefovich).

pkg/sql/conn_executor_exec.go line 358 at r6 (raw file):

Is this to try to fix up the UDTs slice?

Yeah, I'm trying to fix up the UDTs slice. The idea I had here is more to ensure the UDTs are up-to-date, so that we don't need to keep re-parsing the prepared statement for every execution, if any enum type ever changed.

If you mean throwing away the UDTs slice meaning just to nil it, then if there's another enum version change, we won't be able to detect it at this step, and would trigger the version mismatch error again. Or am i missing something here ... ?

pkg/sql/conn_executor_exec.go line 365 at r6 (raw file):

Previously, michae2 (Michael Erickson) wrote…

It would be nice to add something to this comment about why we need to re-parse (i.e. that type checking modifies the AST and so we need to re-parse to build a new AST if types change).

Done.

pkg/sql/conn_executor_exec.go line 379 at r6 (raw file):

Previously, michae2 (Michael Erickson) wrote…

Hmm. Since we're re-parsing, I think we need to do a little more here to reset prep. I think we need to throw away all of the state that was based on the old AST. At the very least something like this:
prep.BaseMemo = nil
prep.GenericMemo = nil
prep.IdealGenericPlan = false
prep.Costs.Reset()
prep.HintsGeneration = 0
Also, we should add a call to log.Eventf here to indicate the re-parse in tracing.

Yeah good point -- we will need to re-bake a memo anyways. Done.

pkg/sql/opt/exec/execbuilder/testdata/prepare line 360 at r5 (raw file):

Previously, michae2 (Michael Erickson) wrote…

Thanks! Actually, I was hoping to see the output of enum_range. I'm hoping it will show the renamed type after something like this:

statement ok
PREPARE p132105enumrange AS SELECT enum_range(NULL::enum132105), enum_range('hello'::enum132105);

# should show hi
query TT
EXECUTE p132105enumrange;
----

statement ok
ALTER TYPE enum132105 RENAME VALUE 'hi' to 'aloha';

# should show aloha instead of hi
query TT
EXECUTE p132105enumrange;
----

Ah got it. I modified the test a bit to SELECT enum_range(NULL::enum132105) and enum_range('hello'::enum132105); separately, as the latter actually fail after the ALTER TYPE.

ZhouXing19

Side comment: now that I'm looking at the definition of prep.Statement, I wonder if there's a similar bug hiding in the tree.PlaceholderTypesInfo. AFAICT we don't check the staleness of those types, either. (But we can follow up on that later.)

Good point, made #162487 for tracking. (I only had it tagged with T-sql-queries as i'm not sure which other categories it belongs to...)

@ZhouXing19 made 1 comment.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @DrewKimball, @michae2, and @yuzefovich).

michae2

Good job! Thanks for putting up with my questions!

@michae2 reviewed 4 files and all commit messages, made 3 comments, and resolved 1 discussion.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @DrewKimball and @ZhouXing19).

pkg/sql/conn_executor_exec.go line 358 at r6 (raw file):

Previously, ZhouXing19 (Jane Xing) wrote…

Is this to try to fix up the UDTs slice?

Yeah, I'm trying to fix up the UDTs slice. The idea I had here is more to ensure the UDTs are up-to-date, so that we don't need to keep re-parsing the prepared statement for every execution, if any enum type ever changed.

If you mean throwing away the UDTs slice meaning just to nil it, then if there's another enum version change, we won't be able to detect it at this step, and would trigger the version mismatch error again. Or am i missing something here ... ?

I see, you're right. I was misunderstanding: I thought we got a new UDTs slice every time we built a new memo. But now I see it's only from prepareUsingOptimizer and not other code paths where we build a memo.

pkg/sql/opt/exec/execbuilder/testdata/prepare line 360 at r5 (raw file):

Previously, ZhouXing19 (Jane Xing) wrote…

Ah got it. I modified the test a bit to SELECT enum_range(NULL::enum132105) and enum_range('hello'::enum132105); separately, as the latter actually fail after the ALTER TYPE.

I'm curious, what does it look like if you change:

ALTER TYPE enum132105 RENAME VALUE 'konnichiwa' TO 'bonjour';

to:

ALTER TYPE enum132105 RENAME VALUE 'privet' TO '안녕하세요';

because this is what I'm interested in: not whether there is an error, but whether the new name shows up in the printed values.

ZhouXing19

Thanks for the insight!
Per the convo above, I'll just merge this PR to master and won't backport it for now, if there's no objection.

@ZhouXing19 made 2 comments.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @DrewKimball and @michae2).

pkg/sql/opt/exec/execbuilder/testdata/prepare line 360 at r5 (raw file):

Previously, michae2 (Michael Erickson) wrote…

I'm curious, what does it look like if you change:
ALTER TYPE enum132105 RENAME VALUE 'konnichiwa' TO 'bonjour';
to:
ALTER TYPE enum132105 RENAME VALUE 'privet' TO '안녕하세요';
because this is what I'm interested in: not whether there is an error, but whether the new name shows up in the printed values.

Gotcha, added to the test. And the new name does show up in this case.

Previously, prepared statements referencing UDTs could fail with version mismatch errors after the UDT was altered (e.g., via ALTER TYPE ADD|RENAME|DROP VALUE). This occurred because the prepared statement retained stale type information obtained from the type check during the preparing stage, while new executions used updated type versions. This commit fixes it by tracking all UDTs referenced in prep stmts during preparation, checking UDT versions for staleness before each execution and updating UDT references and re-parsing when versions change. Note: Our behavior intentionally differs from PG, which resolves enum values to version-agnostic OIDs. Example: PREPARE p AS SELECT $1::my_enum = 'enum_a'; ALTER TYPE my_enum RENAME VALUE 'enum_a' TO 'enum_aa'; EXECUTE p('enum_aa'); -- PG returns true, CRDB returns error. Fixes: cockroachdb#132105 Release note (bug fix): Fixed prepared statements failing with "version mismatch" errors when user-defined types are modified between preparation and execution. Prepared statements now automatically detect UDT changes and re-parse to use current type definitions.

ZhouXing19 · 2026-02-05T20:38:36Z

TFTRs!

ZhouXing19 · 2026-02-05T20:38:42Z

/trunk merge

trunk-io · 2026-02-05T20:38:47Z

😎 Merged successfully - details.

ZhouXing19 force-pushed the enum-ver-check branch 2 times, most recently from 4ba582a to 1e039fe Compare January 27, 2026 03:39

ZhouXing19 changed the title ~~[WIP] sql: remove version check in enum datum comparison~~ sql: remove version check in enum datum comparison Jan 27, 2026

ZhouXing19 force-pushed the enum-ver-check branch 2 times, most recently from 6291b82 to 15346c7 Compare January 27, 2026 03:42

ZhouXing19 marked this pull request as ready for review January 27, 2026 03:42

ZhouXing19 requested a review from a team as a code owner January 27, 2026 03:42

ZhouXing19 requested review from DrewKimball, michae2, mw5h and yuzefovich and removed request for a team and mw5h January 27, 2026 03:42

yuzefovich reviewed Jan 27, 2026

View reviewed changes

michae2 requested changes Jan 27, 2026

View reviewed changes

michae2 reviewed Jan 27, 2026

View reviewed changes

michae2 requested changes Jan 28, 2026

View reviewed changes

michae2 reviewed Jan 28, 2026

View reviewed changes

ZhouXing19 mentioned this pull request Jan 29, 2026

sql: fix identity computed columns with projection with generic plans #162036

Merged

ZhouXing19 force-pushed the enum-ver-check branch from 15346c7 to 4c0bc70 Compare February 1, 2026 19:29

ZhouXing19 changed the title ~~sql: remove version check in enum datum comparison~~ sql: track and update UDT versions in prepared statements Feb 1, 2026

yuzefovich reviewed Feb 3, 2026

View reviewed changes

ZhouXing19 force-pushed the enum-ver-check branch from 8246f2b to d686627 Compare February 3, 2026 22:28

michae2 requested changes Feb 3, 2026

View reviewed changes

ZhouXing19 force-pushed the enum-ver-check branch from d686627 to b3f8b1f Compare February 4, 2026 00:06

ZhouXing19 commented Feb 4, 2026

View reviewed changes

michae2 approved these changes Feb 4, 2026

View reviewed changes

yuzefovich approved these changes Feb 4, 2026

View reviewed changes

ZhouXing19 force-pushed the enum-ver-check branch from b3f8b1f to 6566577 Compare February 4, 2026 06:13

ZhouXing19 force-pushed the enum-ver-check branch from 6566577 to 1d35a11 Compare February 4, 2026 15:19

michae2 requested changes Feb 4, 2026

View reviewed changes

ZhouXing19 force-pushed the enum-ver-check branch from 1d35a11 to 831281e Compare February 4, 2026 21:17

ZhouXing19 commented Feb 4, 2026

View reviewed changes

ZhouXing19 mentioned this pull request Feb 4, 2026

sql: investigate type staleness check for PlaceholderTypesInfo #162487

Open

ZhouXing19 commented Feb 4, 2026

View reviewed changes

ZhouXing19 requested a review from michae2 February 4, 2026 22:08

michae2 approved these changes Feb 4, 2026

View reviewed changes

ZhouXing19 force-pushed the enum-ver-check branch from 831281e to e098bc5 Compare February 5, 2026 04:08

ZhouXing19 commented Feb 5, 2026

View reviewed changes

ZhouXing19 force-pushed the enum-ver-check branch from e098bc5 to 3e198e4 Compare February 5, 2026 18:05

trunk-io bot merged commit bfaf345 into cockroachdb:master Feb 5, 2026
25 checks passed

celeste-cockroachdb bot added the target-release-26.2.0 label Feb 5, 2026

celeste-cockroachdb bot added the v26.2.0-prerelease label Mar 11, 2026

Conversation

ZhouXing19 commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blathers-crl bot commented Jan 27, 2026

Uh oh!

cockroach-teamcity commented Jan 27, 2026

Uh oh!

yuzefovich left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 commented Jan 27, 2026

Uh oh!

yuzefovich left a comment

Choose a reason for hiding this comment

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 commented Jan 27, 2026

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 commented Jan 28, 2026

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 commented Jan 28, 2026

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

yuzefovich left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 commented Feb 3, 2026

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

yuzefovich commented Feb 3, 2026

Uh oh!

yuzefovich commented Feb 3, 2026

Uh oh!

ZhouXing19 left a comment

Choose a reason for hiding this comment

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

yuzefovich left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 commented Feb 4, 2026

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 left a comment

Choose a reason for hiding this comment

Uh oh!

michae2 left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 left a comment

Choose a reason for hiding this comment

Uh oh!

ZhouXing19 commented Feb 5, 2026

Uh oh!

ZhouXing19 commented Jan 27, 2026 •

edited

Loading

trunk-io bot commented Feb 5, 2026 •

edited

Loading