opt/memo: silence error in factorOutVirtualCols for mutation columns #139388
Conversation
|
This change is |
michae2
added
backport-24.1.x
There was a problem hiding this comment.
Nice job figuring this out!
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @fqazi and @michae2)
-- commits line 24 at r1:
nit: consider mentioning that the bug was introduced in 24.1 version.
There was a problem hiding this comment.
Nice work!
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:
Mutation columns (such as those being dropped by an ALTER TABLE DROP COLUMN statement) exist in table metadata, but might not necessarily have an associated computed column expression. This is because `optbuilder.(*Builder).addComputedColsForTable` usually skips over mutation columns. In `memo.(*statisticsBuilder).factorOutVirtualCols` we gather all the expressions for virtual computed columns of the table. We were failing an assertion if we couldn't find the expression for a column. This is fine, however, because `factorOutVirtualCols` only powers an optimization in statistics builder and is not necessary for correctness. This commit silences the assertion if the column is a mutation column, and futhermore, only checks it in test builds otherwise, because ideally we wouldn't want to miss the optimization for non-mutation columns. Fixes: cockroachdb#129405 Fixes: cockroachdb#138147 Fixes: cockroachdb#138809 Release note (bug fix): Fix a rare bug in which a query might fail with error "could not find computed column expression for column in table" while dropping a virtual computed column from the table. This bug was introduced in v23.2.4.
There was a problem hiding this comment.
TFTRs!
bors r=yuzefovich,fqazi
Reviewable status:
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: consider mentioning that the bug was introduced in 24.1 version.
Ah, good call. Looks like it was 23.2.4.
139388: opt/memo: silence error in factorOutVirtualCols for mutation columns r=yuzefovich,fqazi a=michae2 Mutation columns (such as those being dropped by an ALTER TABLE DROP COLUMN statement) exist in table metadata, but might not necessarily have an associated computed column expression. This is because `optbuilder.(*Builder).addComputedColsForTable` usually skips over mutation columns. In `memo.(*statisticsBuilder).factorOutVirtualCols` we gather all the expressions for virtual computed columns of the table. We were failing an assertion if we couldn't find the expression for a column. This is fine, however, because `factorOutVirtualCols` only powers an optimization in statistics builder and is not necessary for correctness. This commit silences the assertion if the column is a mutation column, and futhermore, only checks it in test builds otherwise, because ideally we wouldn't want to miss the optimization for non-mutation columns. Fixes: #129405 Fixes: #138147 Fixes: #138809 Release note (bug fix): Fix a rare bug in which a query might fail with error "could not find computed column expression for column in table" while dropping a virtual computed column from the table. This bug was introduced in v23.2.4. Co-authored-by: Michael Erickson <[email protected]>
|
Build failed: |
|
looks like an infra flake, trying again... bors r=yuzefovich,fqazi |
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
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 #129405: branch-release-23.2, branch-release-24.2, branch-release-24.3, branch-release-25.1. Issue #138147: branch-release-23.2, branch-release-24.1, branch-release-24.2, branch-release-25.1. Issue #138809: branch-release-23.2, branch-release-24.1, branch-release-24.2, branch-release-25.1. 🦉 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 d9eebcf to blathers/backport-release-23.2-139388: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.2.x failed. See errors above. error creating merge commit from d9eebcf to blathers/backport-release-24.1-139388: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 24.1.x failed. See errors above. error creating merge commit from d9eebcf to blathers/backport-release-24.2-139388: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 24.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Mutation columns (such as those being dropped by an ALTER TABLE DROP COLUMN statement) exist in table metadata, but might not necessarily have an associated computed column expression. This is because
optbuilder.(*Builder).addComputedColsForTableusually skips over mutation columns.In
memo.(*statisticsBuilder).factorOutVirtualColswe gather all the expressions for virtual computed columns of the table. We were failing an assertion if we couldn't find the expression for a column. This is fine, however, becausefactorOutVirtualColsonly powers an optimization in statistics builder and is not necessary for correctness.This commit silences the assertion if the column is a mutation column, and futhermore, only checks it in test builds otherwise, because ideally we wouldn't want to miss the optimization for non-mutation columns.
Fixes: #129405
Fixes: #138147
Fixes: #138809
Release note (bug fix): Fix a rare bug in which a query might fail with error "could not find computed column expression for column in table" while dropping a virtual computed column from the table. This bug was introduced in v23.2.4.