sql: add support for automatically repairing dangling comments#151737
sql: add support for automatically repairing dangling comments#151737craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
dd2e04c to
41253f5
Compare
rafiss
left a comment
There was a problem hiding this comment.
nice work! nothing too major in my comments.
Reviewable status:
complete! 0 of 0 LGTMs obtained
pkg/sql/sem/builtins/builtins.go line 5822 at r1 (raw file):
THEN ( SELECT crdb_internal.unsafe_delete_comment(
do we actually need to introduce a new builtin here? would DELETE ... FROM system.comments RETURNING ... work here?
pkg/sql/crdb_internal.go line 6668 at r1 (raw file):
system.namespace AS ns FULL JOIN system.descriptor AS d ON ns.id = d.id ), missing_comments
nit: `
pkg/sql/crdb_internal.go line 6692 at r1 (raw file):
AS corruption FROM data
nit: this would be a bit more readable if you did the UNION here, so the missing comments were already part of the diag table.
pkg/sql/crdb_internal.go line 6698 at r1 (raw file):
FROM (SELECT parent_id, parent_schema_id, name, id, corruption FROM diag UNION SELECT * FROM missing_comments)
i believe if we address the above comment, then we won't need to have a nested subquery here.
rafiss
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 6668 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
nit: `
oops didn't finish typing.
nit: missing_comments is not an entirely accurate name here; would dangling_comments or orphaned_comments be better?
fqazi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/sem/builtins/builtins.go line 5822 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
do we actually need to introduce a new builtin here? would
DELETE ... FROM system.comments RETURNING ...work here?
Sadly DELETE is only allowed as a top level statement even with returning, so I had to switch to adding a new builtin :(
41253f5 to
7ac0211
Compare
fqazi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 6698 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
i believe if we address the above comment, then we won't need to have a nested subquery here.
Done.
7ac0211 to
509ba99
Compare
rafiss
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 6698 at r1 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
Done.
we don't need a subquery; this can just be the same as it was originally
SELECT
parent_id, parent_schema_id, name, id, corruption
FROM
diag
fqazi
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 6698 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
we don't need a subquery; this can just be the same as it was originally
SELECT parent_id, parent_schema_id, name, id, corruption FROM diag
Forgot to push this bit
Previously, we added logic to detect dangling comments on descriptors but did not include a mechanism to clean them up. This could block initial upgrades due to dangling entries in the system.comments table. This patch adds support for automatically cleaning up these dangling comments. Fixes: cockroachdb#151497 Release note (bug fix): Added an automatic repair for dangling or invalid entries in the system.comment table.
509ba99 to
bda91c9
Compare
|
@rafiss TFTR! bors r+ |
151737: sql: add support for automatically repairing dangling comments r=fqazi a=fqazi Previously, we added logic to detect dangling comments on descriptors but did not include a mechanism to clean them up. This could block initial upgrades due to dangling entries in the system.comments table. This patch adds support for automatically cleaning up these dangling comments. Fixes: #151497 Release note (bug fix): Added an automatic repair for dangling or invalid entries in the system.comment table. 152079: roachprod: insecure cluster warn log r=herkolategan a=golgeek As of #151230, roachprod's default behavior is to provision secure clusters unless provisioning is in the cockroach-ephemeral GCP project. To warn the user about the fact that the cluster is insecure in this configuration, a log was added to stdout. The addition of this log message broke some K/V performance scripts, so this patch moves the warning to stderr instead. Epic: none Release note: None Co-authored-by: Faizan Qazi <faizan@cockroachlabs.com> Co-authored-by: Ludovic Leroux <ludo.leroux@cockroachlabs.com>
|
This PR was included in a batch that timed out, it will be automatically retried |
|
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 #151497: branch-release-24.3, branch-release-25.2. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from bda91c9 to blathers/backport-release-24.3-151737: POST https://api.github.com/repos/fqazi/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 24.3.x failed. See errors above. error creating merge commit from bda91c9 to blathers/backport-release-25.2-151737: POST https://api.github.com/repos/fqazi/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 25.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Previously, we added logic to detect dangling comments on descriptors but did not include a mechanism to clean them up. This could block initial upgrades due to dangling entries in the system.comments table. This patch adds support for automatically cleaning up these dangling comments.
Fixes: #151497
Release note (bug fix): Added an automatic repair for dangling or invalid entries in the system.comment table.