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

opt: minor optbuilder improvements related to assignment casts #73650

Merged
merged 3 commits into from
Dec 10, 2021

Conversation

mgartner
Copy link
Collaborator

@mgartner mgartner commented Dec 9, 2021

opt: do not rely on column names for GENERATED ALWAYS AS IDENTITY checks

Previously, optbuilder relied on matching column names to validate that
mutations do not explicitly write to GENERATED ALWAYS AS IDENTITY
columns. There are no known bugs caused by this, but relying on column
names can be brittle. This commit updates the logic to use ordinals of
columns within their table instead.

Release note: None

opt: simplify optbuilder update logic

Release note: None

opt: simplify assignment cast logic for inserts

Creation of invalid assignment cast errors has been abstracted to a
helper function for reuse for other mutations in a future commit.

Release note: None

Previously, optbuilder relied on matching column names to validate that
mutations do not explicitly write to `GENERATED ALWAYS AS IDENTITY`
columns. There are no known bugs caused by this, but relying on column
names can be brittle. This commit updates the logic to use ordinals of
columns within their table instead.

Release note: None
@mgartner mgartner requested a review from a team as a code owner December 9, 2021 20:01
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Needs a make bazel-generate

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ZhouXing19)

Creation of invalid assignment cast errors has been abstracted to a
helper function for reuse for other mutations in a future commit.

Release note: None
@mgartner mgartner force-pushed the simplify-optbuilder-update-logic branch from 660f4f7 to f86da8b Compare December 9, 2021 22:23
@mgartner
Copy link
Collaborator Author

mgartner commented Dec 9, 2021

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 10, 2021

Build succeeded:

@craig craig bot merged commit 5509f95 into cockroachdb:master Dec 10, 2021
@mgartner mgartner deleted the simplify-optbuilder-update-logic branch December 10, 2021 02:19
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.

3 participants