Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: avoid invalid SQL in raw_config_sql in crdb_internal.zones #137584

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

yuzefovich
Copy link
Member

Previously, we could include an invalid ALTER ... CONFIGURE ZONE USING; stmt in the raw_config_sql column of crdb_internal.zones. This was the case when some of the config was inherited and we didn't want to include it for the corresponding object. The contract of this virtual tables allows NULLs in this column, so this commit returns NULL in such cases. Note that I spent non-trivial amount of time trying to figure out how we could end up in this situation but was unsuccessful.

This invalid stmt was included into the output of SHOW CREATE TABLE which, among other things, we use when recreating the bundle, so we should now have easier time recreating multi-region bundles.

Fixes: #123904.

Release note: None

Copy link

blathers-crl bot commented Dec 17, 2024

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
Copy link
Member

This change is Reviewable

Previously, we could include an invalid `ALTER ... CONFIGURE ZONE
USING;` stmt in the `raw_config_sql` column of `crdb_internal.zones`.
This was the case when some of the config was inherited and we didn't
want to include it for the corresponding object. The contract of this
virtual tables allows NULLs in this column, so this commit returns NULL
in such cases. Note that I spent non-trivial amount of time trying to
figure out how we could end up in this situation but was unsuccessful.

This invalid stmt was included into the output of SHOW CREATE TABLE
which, among other things, we use when recreating the bundle, so we
should now have easier time recreating multi-region bundles.

Release note: None
@yuzefovich yuzefovich marked this pull request as ready for review December 17, 2024 21:01
@yuzefovich yuzefovich requested review from rafiss, annrpom and a team December 17, 2024 21:01
@yuzefovich yuzefovich added backport-23.2.x Flags PRs that need to be backported to 23.2. backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2 backport-24.3.x Flags PRs that need to be backported to 24.3 labels Dec 17, 2024
Copy link
Contributor

@annrpom annrpom left a comment

Choose a reason for hiding this comment

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

:lgtm:

@yuzefovich
Copy link
Member Author

TFTR!

bors r+

@craig craig bot merged commit 8e24b34 into cockroachdb:master Dec 18, 2024
22 checks passed
Copy link

blathers-crl bot commented Dec 18, 2024

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.


Issue #123904: branch-release-23.2, branch-release-24.1, branch-release-24.2, branch-release-24.3.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.2.x Flags PRs that need to be backported to 23.2. backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x Flags PRs that need to be backported to 24.2 backport-24.3.x Flags PRs that need to be backported to 24.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bundle: presence of custom zone configs breaks recreation
3 participants