⚡ Bolt: Optimize Value::integer() and Value::float() conversions#47
Conversation
Replaced inefficient `to_string().parse()` stringification in `Value::integer` and `Value::float` conversions with direct conversions using `to_i64` and `to_f64` from `rust_decimal::prelude::ToPrimitive`. Kept the edge case check for `scale() == 0` for `integer()` to strictly maintain parser behavior parity. Added corresponding test cases. Co-authored-by: ashyanSpada <22587148+ashyanSpada@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly optimizes the conversion of Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes Decimal to i64 and f64 conversions in the Value type by switching from string-based parsing to direct rust_decimal::prelude::ToPrimitive trait methods (to_i64, to_f64). For integer conversions, a strict check val.scale() == 0 is introduced to maintain behavioral parity with string parsing, rejecting decimal values with non-zero scales. New unit tests validate these changes. The review suggests refactoring the integer method's match statement to use a match guard for better readability and conciseness.
| match self { | ||
| Self::Number(val) => val | ||
| .to_string() | ||
| .parse() | ||
| .map_or(Err(Error::InvalidInteger), |num| Ok(num)), | ||
| Self::Number(val) => { | ||
| if val.scale() == 0 { | ||
| val.to_i64().ok_or(Error::InvalidInteger) | ||
| } else { | ||
| Err(Error::InvalidInteger) | ||
| } | ||
| } | ||
| _ => Err(Error::InvalidInteger), | ||
| } |
There was a problem hiding this comment.
For improved readability and conciseness, you can use a match guard. This simplifies the logic by incorporating the val.scale() == 0 check directly into the match arm, removing the nested if/else block.
match self {
Self::Number(val) if val.scale() == 0 => val.to_i64().ok_or(Error::InvalidInteger),
_ => Err(Error::InvalidInteger),
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #47 +/- ##
==========================================
+ Coverage 89.17% 89.55% +0.38%
==========================================
Files 11 11
Lines 1062 1063 +1
==========================================
+ Hits 947 952 +5
+ Misses 115 111 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Optimizes numeric extraction from Value::Number(Decimal) by replacing string-based conversions with direct rust_decimal primitive conversions, reducing allocations and improving runtime performance in the expression engine.
Changes:
- Replaced
to_string().parse()withto_i64()/to_f64()inValue::integer()andValue::float(). - Preserved prior
integer()failure behavior by rejecting decimals withscale() != 0. - Added unit tests covering the updated conversion behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/value.rs |
Uses ToPrimitive conversions for integer/float accessors and adds unit tests for conversion semantics. |
.jules/bolt.md |
Documents the conversion optimization and the scale()==0 parity requirement as a bolt learning/action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
💡 What: Replaced inefficient string conversion/parsing (
to_string().parse()) insideValue::integer()andValue::float()with direct conversion traits (to_i64andto_f64). Added check forscale() == 0to integer to strictly preserve former failure characteristics.🎯 Why: To avoid inefficient heap allocations caused by intermediate stringifications, which improves performance and reduces memory usage in the expression engine.
📊 Impact: Reduces memory usage from string allocation and lowers CPU execution time significantly for integer and float value access.
🔬 Measurement: Verify by running
cargo testand observing logic tests pass, and viacargo benchto benchmark conversions.PR created automatically by Jules for task 1351915934699169371 started by @ashyanSpada