-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
opt: mark SimplifyRange as essential #88399
Conversation
This commit marks the `SimplifyRange` normalization rule as essential during random rule-disabling tests. This is necessary because `RangeExpr` is expected to maintain the invariant that its input is always an `AndExpr`. Other rules can replace the `AndExpr` with a different expression over the course of normalization, at which point the `RangeExpr` needs to be removed. Otherwise, various code-paths that depend on the invariant may panic (for example, predicate implication). Fixes cockroachdb#88352 Release note: None
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.
nit: add a backport label.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball)
Should I add a label for 21.2 as well? |
I think backport to only 22.2 is needed since that where we introduced the rules-disabling knobs. |
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.
Great find!
TFTRs! |
bors r+ |
Build succeeded: |
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 d6bd25c to blathers/backport-release-22.2-88399: 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 22.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
This commit marks the
SimplifyRange
normalization rule as essential during random rule-disabling tests. This is necessary becauseRangeExpr
is expected to maintain the invariant that its input is always anAndExpr
. Other rules can replace theAndExpr
with a different expression over the course of normalization, at which point theRangeExpr
needs to be removed. Otherwise, various code-paths that depend on the invariant may panic (for example, predicate implication).Fixes #88352
Release note: None