workload/schemachanger: fix dependency detection for DROP COLUMN#160659
workload/schemachanger: fix dependency detection for DROP COLUMN#160659craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
spilchen
left a comment
There was a problem hiding this comment.
Thanks for fixing this!
@spilchen reviewed all commit messages and made 2 comments.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @DarrylWong, @fqazi, and @golgeek).
pkg/workload/schemachange/error_screening.go line 153 at r1 (raw file):
func (og *operationGenerator) columnIsDependedOn( ctx context.Context, tx pgx.Tx, tableName *tree.TableName, columnName tree.Name, includeFKs bool,
I don't think there is a case where we pass in true for the incluedFKs parameter? Just wondering what the benefit is to have this new parm. We could simplify the function a bit if we unconditionally omitted FKs. Unless setColumnType usage was meant to pass in true?
Previously, the DROP COLUMN operation in the schema changer could fail, since it detected self referential foreign key dependencies as external ones. To address this, this patch excludes them from the detection. Fixes: cockroachdb#160318 Fixes: cockroachdb#159952 Release note: None
fqazi
left a comment
There was a problem hiding this comment.
@fqazi made 1 comment.
Reviewable status:
pkg/workload/schemachange/error_screening.go line 153 at r1 (raw file):
Previously, spilchen wrote…
I don't think there is a case where we pass in true for the
incluedFKsparameter? Just wondering what the benefit is to have this new parm. We could simplify the function a bit if we unconditionally omitted FKs. UnlesssetColumnTypeusage was meant to pass in true?
Done.
Yes, I forgot to flip setColumnType to true
spilchen
left a comment
There was a problem hiding this comment.
@spilchen made 1 comment and resolved 1 discussion.
Reviewable status:
|
@spilchen TFTR! bors r+ |
|
Build succeeded: |
|
blathers backport 25.2 |
|
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 #160318: branch-release-25.2. Issue #159952: branch-release-25.2. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Previously, the DROP COLUMN operation in the schema changer could fail, since it detected self referential foreign key dependencies as external ones. To address this, this patch excludes them from the detection.
Fixes: #160318
Fixes: #159952
Release note: None