-
Notifications
You must be signed in to change notification settings - Fork 179
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(torii): i64 deser #2636
fix(torii): i64 deser #2636
Conversation
WalkthroughOhayo, sensei! The changes in this pull request primarily involve the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Model
participant Database
Client->>Database: Request data
Database-->>Client: Send data as String (hexadecimal)
Client->>Model: Call map_row_to_ty
Model->>Model: Convert String to i64 (old method)
Model->>Client: Return i64 value
Client->>Database: Request data
Database-->>Client: Send data as i64
Client->>Model: Call map_row_to_ty
Model->>Client: Return i64 value (new method)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
crates/torii/core/src/model.rs (1)
509-510
: LGTM! Efficient i64 deserialization.Ohayo, sensei! The direct i64 retrieval is more efficient than the previous string manipulation approach. This change aligns well with how we handle other primitive integer types like i8, i16, and i32.
Consider applying similar optimizations to other integer types like u64, u128, and i128 that are still using string manipulation and hex conversion. For example:
Primitive::U64(_) => { - let value = row.try_get::<String, &str>(&column_name)?; - let hex_str = value.trim_start_matches("0x"); - primitive.set_u64(Some( - u64::from_str_radix(hex_str, 16).map_err(ParseError::ParseIntError)?, - ))?; + let value = row.try_get::<u64, &str>(&column_name)?; + primitive.set_u64(Some(value))?; }
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2636 +/- ##
==========================================
- Coverage 56.79% 56.78% -0.02%
==========================================
Files 397 397
Lines 49560 49557 -3
==========================================
- Hits 28148 28141 -7
- Misses 21412 21416 +4 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
Primitive::I64
values by simplifying the conversion process.