-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: alter primary key hangs if index references exist #124323
Conversation
1601d8b
to
29c92ec
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.
just pointed out a few typos you may want to address in a future PR on master
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @annrpom, @blathers-crl[bot], and @fqazi)
pkg/sql/alter_primary_key.go
line 452 at r1 (raw file):
indexesToRewrite = append(indexesToRewrite, idx) } // If this index is referenced by any other objects, then we wil
nit: "wil" => "will"
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go
line 690 at r1 (raw file):
scpb.ForEachSecondaryIndex(publicTableElts, func(_ scpb.Status, _ scpb.TargetStatus, idx *scpb.SecondaryIndex) { out := makeIndexSpec(b, idx.TableID, idx.IndexID) // If this index is referenced by any other objects, then we wil
nit: "wil" => "will"
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.
@mgartner TFTR. Yeah, let me get a PR open for those.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @annrpom and @blathers-crl[bot])
Previously, ALTER PRIMARY KEY could hang if an index reference existed from either a view or function using the index selection syntax. This was becasue neither the declarative schema changer or legacy schema changer support updating these references. To address this, this patch will prevent ALTER PRIMARY KEY if such references exist to any indexes that will be recreated. Fixes: #123017 Release note (bug fix): ALTER PRIMARY KEY could hang if the table had any indexes which were referred to by views or functions using the force index syntax.
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 @annrpom)
pkg/sql/alter_primary_key.go
line 452 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
nit: "wil" => "will"
Done.
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_alter_primary_key.go
line 690 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
nit: "wil" => "will"
Done.
29c92ec
to
06272b5
Compare
Backport 1/1 commits from #124132 on behalf of @fqazi.
/cc @cockroachdb/release
Previously, ALTER PRIMARY KEY could hang if an index reference existed from either a view or function using the index selection syntax. This was becasue neither the declarative schema changer or legacy schema changer support updating these references. To address this, this patch will prevent ALTER PRIMARY KEY if such references exist to any indexes that will be recreated.
Fixes: #123017
Release note (bug fix): ALTER PRIMARY KEY could hang if the table had any indexes which were referred to by views or functions using the force index syntax.
Release justification: low risk and prevents users from running into an unsupported schema change scenario that will lead to requiring a manual help from support