Skip to content

sql/schemachanger: add support for setting schema of enum types#168255

Open
bghal wants to merge 1 commit intocockroachdb:masterfrom
bghal:alter-type-set-schema
Open

sql/schemachanger: add support for setting schema of enum types#168255
bghal wants to merge 1 commit intocockroachdb:masterfrom
bghal:alter-type-set-schema

Conversation

@bghal
Copy link
Copy Markdown
Contributor

@bghal bghal commented Apr 13, 2026

This change implements support for changing the
schema of enum UDTs and rewires the command to it.

Closes: #164216

Epic: CRDB-31325

Release note: None

@trunk-io
Copy link
Copy Markdown
Contributor

trunk-io Bot commented Apr 13, 2026

✨ Submitted to Merge by @bghal. It will be added to the merge queue once all branch protection rules pass and there are no merge conflicts with the target branch. See more details here.

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 13, 2026

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

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

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 13, 2026

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 13, 2026

Detected infrastructure failure (matched: ). Automatically rerunning failed jobs. (run link)

@bghal bghal force-pushed the alter-type-set-schema branch from 07358c4 to de4be8f Compare April 13, 2026 20:27
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 13, 2026

Detected infrastructure failure (matched: ). Automatically rerunning failed jobs. (run link)

1 similar comment
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 13, 2026

Detected infrastructure failure (matched: ). Automatically rerunning failed jobs. (run link)

@bghal bghal force-pushed the alter-type-set-schema branch from de4be8f to 0721c1c Compare April 14, 2026 14:56
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 14, 2026

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 14, 2026

Detected infrastructure failure (matched: ). Automatically rerunning failed jobs. (run link)

@bghal bghal force-pushed the alter-type-set-schema branch from 0721c1c to 6f06e3b Compare April 14, 2026 19:51
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 14, 2026

Detected infrastructure failure (matched: ). Automatically rerunning failed jobs. (run link)

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 14, 2026

Detected infrastructure failure (matched: ). Automatically rerunning failed jobs. (run link)

1 similar comment
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 14, 2026

Detected infrastructure failure (matched: ). Automatically rerunning failed jobs. (run link)

@bghal bghal force-pushed the alter-type-set-schema branch 4 times, most recently from eaa1537 to c6c78e5 Compare April 22, 2026 21:49
@bghal bghal marked this pull request as ready for review April 27, 2026 20:11
@bghal bghal requested a review from a team as a code owner April 27, 2026 20:11
@bghal bghal marked this pull request as draft April 27, 2026 20:14
@bghal bghal force-pushed the alter-type-set-schema branch 2 times, most recently from f8b1dfe to 86734ff Compare April 29, 2026 13:25
bghal added a commit to bghal/cockroach that referenced this pull request Apr 29, 2026
…escriptors

The schema ID field was not being set when walked over for an update
because of a silent fallthrough. This caused inconsistencies between the
descriptor and the namespace entry. The change handles the table desc
case and adds an error-default to prevent any future silent
fallthroughs.

Supports: cockroachdb#168255

Part of: cockroachdb#164216

Epic: CRDB-31325

Release note: None
bghal added a commit to bghal/cockroach that referenced this pull request Apr 29, 2026
…escriptors

The schema ID field was not being set when walked over for an update
because of a silent fallthrough. This caused inconsistencies between the
descriptor and the namespace entry. The change adds an error-default
that exposes the silent fallthrough and handling for the table
descriptor's update.

Supports: cockroachdb#168255

Part of: cockroachdb#164216

Epic: CRDB-31325

Release note: None
bghal added a commit to bghal/cockroach that referenced this pull request Apr 29, 2026
…escriptors

The schema ID field was not being set when walked over for an update
because of a silent fallthrough. This caused inconsistencies between the
descriptor and the namespace entry. The change adds an error-default
that exposes the silent fallthrough and handling for the table
descriptor's update.

Supports: cockroachdb#168255

Part of: cockroachdb#164216

Epic: CRDB-31325

Release note: None
bghal added a commit to bghal/cockroach that referenced this pull request Apr 30, 2026
…iptors

The schema ID field was not being set when walked over for an update
because of a silent fallthrough; another op was updating the field. The
change adds an error-default that exposes the silent fallthrough and
refactors handling of the table descriptor's update to the expected
place.

Supports: cockroachdb#168255

Part of: cockroachdb#164216

Epic: CRDB-31325

Release note: None
bghal added a commit to bghal/cockroach that referenced this pull request Apr 30, 2026
In cases of a name collisions of a companion array type when changing a
type's schema, the schemachanger was not handling it well. This change
addresses that and adds a test.

Supports: cockroachdb#168255

Part of: cockroachdb#164216

Epic: CRDB-31325

Release note: None
@bghal bghal force-pushed the alter-type-set-schema branch from 86734ff to 6d62376 Compare April 30, 2026 18:52
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 30, 2026

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

bghal added a commit to bghal/cockroach that referenced this pull request May 1, 2026
In cases of a name collisions of a companion array type when changing a
type's schema, the schemachanger was not handling it well. This change
addresses that and adds a test.

Supports: cockroachdb#168255

Part of: cockroachdb#164216

Epic: CRDB-31325

Release note: None
bghal added a commit to bghal/cockroach that referenced this pull request May 4, 2026
In cases of a name collisions of a companion array type when changing a
type's schema, the schemachanger was not handling it well. This change
addresses that and adds a test.

Supports: cockroachdb#168255

Part of: cockroachdb#164216

Epic: CRDB-31325

Release note: None
@bghal bghal force-pushed the alter-type-set-schema branch from 6d62376 to 1cb0aaa Compare May 4, 2026 17:33
@bghal bghal marked this pull request as ready for review May 4, 2026 19:52
Copy link
Copy Markdown
Contributor

@spilchen spilchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: just a few minor comments/suggestions.

@spilchen reviewed all commit messages and made 3 comments.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on bghal).


pkg/sql/alter_type.go line 378 at r1 (raw file):

	// The CheckObjectNameCollision checks that the companion array can be moved
	// with its name.

nit: suggest rewording

Suggestion:

// without a name collision.

pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_type.go line 318 at r1 (raw file):

		DescriptorType:    "type",
	})
	b.LogEventForExistingPayload(newSchemaChild, &eventpb.AlterType{

is the event for AlterType needed? We already have the SetSchema event. Looking at the legacy codepath, we only log a single event (SetSchema).

@bghal bghal force-pushed the alter-type-set-schema branch from 1cb0aaa to e3b9f20 Compare May 5, 2026 15:13
Copy link
Copy Markdown
Contributor Author

@bghal bghal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bghal made 1 comment and resolved 1 discussion.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on spilchen).


pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_type.go line 318 at r1 (raw file):

Previously, spilchen wrote…

is the event for AlterType needed? We already have the SetSchema event. Looking at the legacy codepath, we only log a single event (SetSchema).

Done.

@bghal bghal force-pushed the alter-type-set-schema branch 2 times, most recently from bf7878b to c37dc0a Compare May 5, 2026 21:02
bghal added a commit to bghal/cockroach that referenced this pull request May 5, 2026
The schemachanger was logging a general `alter_type` event and a
particular `set_schema` event. This change removes the duplicative
logging.

Part of: cockroachdb#164216
See: cockroachdb#57741
Supports: cockroachdb#168255
Epic: CRDB-31325

Release note: None
Copy link
Copy Markdown
Contributor Author

@bghal bghal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bghal made 1 comment.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on spilchen).


pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_type.go line 318 at r1 (raw file):

Previously, bghal (Brendan) wrote…

Done.

Actually there's a bug in the legacy that dual logs. Fixed legacy in #169780.

bghal added a commit to bghal/cockroach that referenced this pull request May 6, 2026
The schemachanger was logging a general `alter_type` event and a
particular `set_schema` event. This change removes the duplicative
logging.

Part of: cockroachdb#164216
See: cockroachdb#57741
Supports: cockroachdb#168255
Epic: CRDB-31325

Release note: None
@bghal bghal force-pushed the alter-type-set-schema branch from c37dc0a to 3f9b3af Compare May 6, 2026 15:01
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 6, 2026

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

@bghal bghal force-pushed the alter-type-set-schema branch from 3f9b3af to 24c24ce Compare May 6, 2026 17:36
@bghal
Copy link
Copy Markdown
Contributor Author

bghal commented May 6, 2026

/trunk merge

This change implements support for changing the
schema of enum UDTs and rewires the command to it.

Closes: cockroachdb#164216

Epic: CRDB-31325

Release note: None
@bghal bghal force-pushed the alter-type-set-schema branch from 24c24ce to f1a7a14 Compare May 7, 2026 17:58
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented May 7, 2026

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sql/schemachanger: implement ALTER TYPE ... SET SCHEMA in declarative schema changer

3 participants