Add float related database functions with rusqlite#2324
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe PR enables the rusqlite "functions" feature and introduces a module system for registering SQLite user-defined functions (FLOAT_IS_ZERO, FLOAT_NEGATE, FLOAT_SUM, FLOAT_ZERO_HEX) that operate on hex-encoded floats. These functions are automatically registered when a database connection is opened. Changes
Sequence Diagram(s)sequenceDiagram
participant Executor as open_connection()
participant FnReg as functions::register_all()
participant FloatFns as Float Functions<br/>(is_zero, negate, etc.)
participant SQLite as SQLite DB
Executor->>FnReg: register_all(&conn)
activate FnReg
FnReg->>FloatFns: float_is_zero::register(&conn)
FloatFns->>SQLite: Register FLOAT_IS_ZERO
FnReg->>FloatFns: float_negate::register(&conn)
FloatFns->>SQLite: Register FLOAT_NEGATE
FnReg->>FloatFns: float_sum::register(&conn)
FloatFns->>SQLite: Register FLOAT_SUM
FnReg->>FloatFns: float_zero_hex::register(&conn)
FloatFns->>SQLite: Register FLOAT_ZERO_HEX
deactivate FnReg
FnReg-->>Executor: Ok(())
Executor-->>Caller: Connection with functions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
crates/cli/Cargo.toml(1 hunks)crates/cli/src/commands/local_db/executor.rs(2 hunks)crates/cli/src/commands/local_db/functions/float_is_zero.rs(1 hunks)crates/cli/src/commands/local_db/functions/float_negate.rs(1 hunks)crates/cli/src/commands/local_db/functions/float_sum.rs(1 hunks)crates/cli/src/commands/local_db/functions/float_zero_hex.rs(1 hunks)crates/cli/src/commands/local_db/functions/mod.rs(1 hunks)crates/cli/src/commands/local_db/mod.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rs:84-84
Timestamp: 2025-10-06T14:12:34.864Z
Learning: The SQL function FLOAT_ZERO_HEX() is available in the db lib SDK and does not need to be defined in the codebase. It is used in SQL queries as an external function provided by the database library.
📚 Learning: 2025-10-06T14:41:41.909Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2159
File: crates/cli/src/commands/local_db/sync/runner/mod.rs:52-113
Timestamp: 2025-10-06T14:41:41.909Z
Learning: The local DB sync CLI command (crates/cli/src/commands/local_db/sync/) is designed for CI-only usage, and simple println! statements are preferred over structured logging for status messages.
Applied to files:
crates/cli/src/commands/local_db/mod.rscrates/cli/src/commands/local_db/executor.rs
📚 Learning: 2025-10-21T05:15:50.518Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2256
File: crates/common/src/raindex_client/local_db/query/fetch_tables.rs:7-7
Timestamp: 2025-10-21T05:15:50.518Z
Learning: In Rust 2021+ editions, passing a reference to a temporary into an async function within the same expression (e.g., `exec.query_json(&fetch_tables_stmt()).await`) is safe and idiomatic. The temporary lifetime is extended across the await point, so this pattern should not be flagged as an issue.
Applied to files:
crates/cli/src/commands/local_db/executor.rs
📚 Learning: 2025-10-18T10:38:41.273Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2237
File: crates/common/src/raindex_client/local_db/sync.rs:79-89
Timestamp: 2025-10-18T10:38:41.273Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the sync_database method currently only supports indexing a single orderbook per chain ID, which is why `.first()` is used to select the orderbook configuration. Multi-orderbook support per chain ID is planned for future PRs.
Applied to files:
crates/cli/src/commands/local_db/executor.rs
📚 Learning: 2025-10-06T14:12:05.824Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/local_db/query/fetch_order_trades/query.sql:86-108
Timestamp: 2025-10-06T14:12:05.824Z
Learning: CTEs (Common Table Expressions) are not supported in the local DB SQL implementation used in the rain.orderbook codebase.
Applied to files:
crates/cli/src/commands/local_db/executor.rs
📚 Learning: 2025-10-06T11:13:29.956Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2123
File: crates/common/src/raindex_client/local_db/mod.rs:23-29
Timestamp: 2025-10-06T11:13:29.956Z
Learning: In `crates/common/src/raindex_client/local_db/mod.rs`, the `Default` implementation for `LocalDb` that creates an RPC client pointing to `http://localhost:4444` is acceptable because the RPC client must be explicitly configured before actual usage in production scenarios.
Applied to files:
crates/cli/src/commands/local_db/executor.rs
📚 Learning: 2025-10-06T14:12:34.864Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/local_db/query/fetch_vaults/mod.rs:84-84
Timestamp: 2025-10-06T14:12:34.864Z
Learning: The SQL function FLOAT_ZERO_HEX() is available in the db lib SDK and does not need to be defined in the codebase. It is used in SQL queries as an external function provided by the database library.
Applied to files:
crates/cli/src/commands/local_db/functions/float_is_zero.rscrates/cli/src/commands/local_db/functions/float_sum.rscrates/cli/src/commands/local_db/functions/float_negate.rscrates/cli/src/commands/local_db/functions/mod.rscrates/cli/src/commands/local_db/functions/float_zero_hex.rs
📚 Learning: 2025-05-16T17:24:34.724Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1844
File: tauri-app/src-tauri/src/commands/wallet.rs:27-28
Timestamp: 2025-05-16T17:24:34.724Z
Learning: In Rust projects, dependencies listed in the [dependencies] section of Cargo.toml are available for both application code and tests. The tokio crate with the "full" feature set includes the "test" feature needed for `#[tokio::test]` to work properly.
Applied to files:
crates/cli/Cargo.toml
📚 Learning: 2025-05-14T05:13:59.713Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Applied to files:
crates/cli/Cargo.toml
📚 Learning: 2025-05-15T22:27:49.771Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1777
File: tauri-app/src-tauri/Cargo.toml:64-64
Timestamp: 2025-05-15T22:27:49.771Z
Learning: User 0xgleb prefers to maintain the dot notation format for some Cargo.toml dependencies (e.g., `rain-interpreter-eval.path = "path"`) rather than converting all to inline table format, as both formats are valid in Rust.
Applied to files:
crates/cli/Cargo.toml
🔇 Additional comments (11)
crates/cli/Cargo.toml (1)
36-36: LGTM! Feature flag correctly enables SQLite function registration.The addition of the
functionsfeature to rusqlite is necessary for registering the custom SQLite functions introduced in this PR.crates/cli/src/commands/local_db/functions/float_zero_hex.rs (2)
4-15: LGTM! Function correctly returns hex representation of zero.The implementation properly handles the Float::zero() error case and returns the hex string. The use of SQLITE_DETERMINISTIC and SQLITE_UTF8 flags is appropriate.
21-32: LGTM! Test validates the function output.The test properly registers the function, executes it, and verifies the result matches the expected hex representation of zero.
crates/cli/src/commands/local_db/functions/float_is_zero.rs (2)
4-24: LGTM! Function correctly parses hex input and checks for zero.The implementation properly:
- Extracts the hex string from the argument
- Parses it using Float::from_hex()
- Checks if the value is zero
- Converts all errors to UserFunctionError
30-47: LGTM! Test covers both zero and non-zero cases.The test validates that the function correctly identifies zero values (returns true) and non-zero values (returns false).
crates/cli/src/commands/local_db/functions/float_sum.rs (2)
16-43: LGTM! Aggregate implementation is correct.The FloatSum aggregate properly:
- Initializes the accumulator to zero
- Validates argument count in each step
- Accumulates parsed float values
- Returns None for empty sets (SQL standard behavior for SUM)
The use of
transpose()in finalize is idiomatic Rust.
49-93: LGTM! Tests comprehensively validate aggregate behavior.The tests cover:
- Single value aggregation
- Multiple value aggregation
- Empty set returns NULL (line 81-93)
All critical paths are tested.
crates/cli/src/commands/local_db/mod.rs (1)
3-3: LGTM! Module declaration exposes the new functions module.This allows the executor to access and register all SQLite functions via
functions::register_all().crates/cli/src/commands/local_db/executor.rs (1)
1-1: LGTM! Functions are registered on every connection.The integration is correct:
- Functions are registered immediately after opening the connection and setting the busy timeout
- Errors are properly converted to LocalDbQueryError with a descriptive message
- All connections will have the custom SQLite functions available
Also applies to: 40-42
crates/cli/src/commands/local_db/functions/float_negate.rs (1)
30-44: LGTM! Test validates negation behavior.The test correctly verifies that +1 is negated to -1 by comparing hex representations.
crates/cli/src/commands/local_db/functions/mod.rs (1)
8-15: LGTM! Clean aggregator pattern for function registration.The
register_allfunction properly:
- Registers each custom SQLite function in sequence
- Propagates any registration errors via
?- Provides a single entry point for the executor to register all functions
4 tasks
0xgleb
approved these changes
Nov 25, 2025
hardyjosh
approved these changes
Nov 30, 2025
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M 🧠 Learnings used |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chained PR
Motivation
We had float related functions in our
sqlite-webpackage, but we didn't have a way to use these with our producer pipeline. This PR aims to add the same functions that we use insqlite-weband this repository.Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit