-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: Fix assertion panic on LazyFrame scratch.is_empty()
#20219
Conversation
scratch.is_empty()
63e073e
to
a56c992
Compare
a56c992
to
0c5cf8f
Compare
@@ -25,6 +25,7 @@ mod inner { | |||
} | |||
|
|||
pub fn nodes_scratch_mut(&mut self) -> &mut UnitVec<Node> { | |||
self.scratch.clear(); |
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.
🙈
scratch.is_empty()
scratch.is_empty()
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #20219 +/- ##
=======================================
Coverage 79.62% 79.62%
=======================================
Files 1565 1565
Lines 218188 218192 +4
Branches 2475 2475
=======================================
+ Hits 173735 173744 +9
+ Misses 43886 43881 -5
Partials 567 567 ☔ View full report in Codecov by Sentry. |
@@ -49,7 +50,8 @@ fn can_pushdown_slice_past_projections( | |||
arena: &Arena<AExpr>, | |||
scratch: &mut UnitVec<Node>, | |||
) -> (bool, bool) { | |||
assert!(scratch.is_empty()); | |||
debug_assert!(scratch.is_empty()); | |||
scratch.clear(); |
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.
make this only panic in debug builds, and in release we clear it
Ai, will patch this. |
Fixes #20216