-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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/catalog/tabledesc: fixed a bug in maybeAddConstraintIDs
#85778
sql/catalog/tabledesc: fixed a bug in maybeAddConstraintIDs
#85778
Conversation
Previously, when we RunPostDeserializationChanges, one of the step is to fill in constraint IDs if they are not already set, which is done in function `maybeAddConstraintIDs`. That function however is buggy in that, if there is constraint mutation on the table descriptor, we directly go assign a constraint ID (from `tbl.nextConstraintID`) to it, without checking whether it already has a non-zero constraint ID or not. This PR fixes that. Release note (bug fix): Fixed a bug in post deserialization changes where we might incorrectly change constraint ID of a constraint that lives in the mutation slice of a table descriptor.
00f0991
to
be2ddb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 7 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @Xiang-Gu)
-- commits
line 12 at r1:
I don't think this needs a release note because we never released this bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner)
Previously, ajwerner wrote…
I don't think this needs a release note because we never released this bug.
removed.
TFTR! bors r+ |
Build failed (retrying...): |
Build failed (retrying...): |
Build failed: |
The merge failed due to a flaky test, retrying bors r+ |
Build failed (retrying...): |
Build failed: |
It's sad to see merge failure again but it seems unrelated to this change, retrying again, |
Build succeeded: |
Previously, when we RunPostDeserializationChanges, one of the step is
to fill in constraint IDs if they are not already set, which is done
in function
maybeAddConstraintIDs
. That function however is buggyin that, if there is constraint mutation on the table descriptor, we
directly go assign a constraint ID (from
tbl.nextConstraintID
) to it,without checking whether it already has a non-zero constraint ID or not.
This PR fixes that.
Release note: None