-
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
release-24.1: sql: fix including all related FK tables into the stmt bundle #137789
base: release-24.1
Are you sure you want to change the base?
Conversation
Earlier this year in 084a741 we made a change to include all tables referencing the table from the stmt bundle (i.e. those that we have an inbound FK relationship with). However, we forgot to include tables that the referencing table is dependent on, so this commit fixes that oversight. Namely, we now include all tables that we reference or that reference us via the FK relationship, and this rule is applied recursively to every table in consideration. (Previously, we didn't include inbound FKs when handling an outbound FK nor outbound FKs when handling an inbound FK.) Note that I believe we could have avoided including referencing tables (i.e. with an inbound FK relationship) that only have NO ACTION or RESTRICT actions in the ON DELETE and ON UPDATE, but I think those cases aren't very common, and it's unlikely to hurt including all "related" tables. The case of FK cycles is still not handled correctly on the stmt bundle recreation, but I think we have yet to run into one in a support ticket. Release note: None
5ec197e
to
3587ce0
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
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 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner and @yuzefovich)
Backport 1/1 commits from #137738 on behalf of @yuzefovich.
/cc @cockroachdb/release
Earlier this year in 084a741 we made a change to include all tables referencing the table from the stmt bundle (i.e. those that we have an inbound FK relationship with). However, we forgot to include tables that the referencing table is dependent on, so this commit fixes that oversight. Namely, we now include all tables that we reference or that reference us via the FK relationship, and this rule is applied recursively to every table in consideration. (Previously, we didn't include inbound FKs when handling an outbound FK nor outbound FKs when handling an inbound FK.)
Note that I believe we could have avoided including referencing tables (i.e. with an inbound FK relationship) that only have NO ACTION or RESTRICT actions in the ON DELETE and ON UPDATE, but I think those cases aren't very common, and it's unlikely to hurt including all "related" tables. The case of FK cycles is still not handled correctly on the stmt bundle recreation, but I think we have yet to run into one in a support ticket.
Informs: https://github.com/cockroachlabs/support/issues/3146.
Epic: None
Release note: None
Release justification: low-risk supportability improvement.