Refactor sql generation logic with SqlStatement #2262
Merged
Conversation
Contributor
WalkthroughThis PR removes five local_db CLI subcommands (FetchEvents, DecodeEvents, DecodedEventsToSql, TokensFetch, TokensToSql) and refactors the SQL generation layer from string-based to structured, parameterized statement batches. It introduces SqlStatementBatch and SqlStatement types, updates executors to support transactional batch execution, and adjusts sync and query pipelines to consume batches instead of raw SQL strings. Changes
Sequence Diagram(s)sequenceDiagram
participant Sync as Sync Runner
participant DataSource as SyncDataSource
participant Exec as LocalDbQueryExecutor
participant DB as SQLite DB
rect rgb(200, 220, 255)
note over Sync,DB: Old Flow: String-based SQL
Sync->>DataSource: raw_events_to_sql()
DataSource-->>Sync: String (SQL)
Sync->>DataSource: events_to_sql(end_block, prefix_sql)
DataSource-->>Sync: String (SQL)
Sync->>Sync: Concatenate strings + update block
Sync->>Exec: query_text(SqlStatement::new(sql))
Exec->>DB: Execute single statement
DB-->>Exec: Result
Exec-->>Sync: Result
end
rect rgb(200, 255, 220)
note over Sync,DB: New Flow: Batched Statements
Sync->>DataSource: raw_events_to_statements()
DataSource-->>Sync: SqlStatementBatch
Sync->>DataSource: events_to_sql()
DataSource-->>Sync: SqlStatementBatch
Sync->>Sync: batch.extend() + batch.add(update_block)
Sync->>Sync: batch.ensure_transaction()
Sync->>Exec: execute_batch(&batch)
rect rgb(240, 248, 255)
Exec->>DB: BEGIN TRANSACTION
Exec->>DB: Execute statement 1
Exec->>DB: Execute statement 2
Exec->>DB: ... (all statements)
alt Success
Exec->>DB: COMMIT
else Failure
Exec->>DB: ROLLBACK
end
end
DB-->>Exec: Result
Exec-->>Sync: Result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related issues
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Collaborator
Author
|
@coderabbitai review |
This was
linked to
issues
Oct 22, 2025
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
crates/common/src/local_db/decode.rs (1)
118-128: Minor: Redundant trim() call.The validation logic correctly handles empty hex data by stripping the "0x"/"0X" prefix. However, line 123 calls
.trim()ondata_without_prefix, which is already derived fromtrimmed_data(line 118), making this second trim redundant.Apply this diff to remove the redundant trim:
let trimmed_data = event.data.trim(); let data_without_prefix = trimmed_data .strip_prefix("0x") .or_else(|| trimmed_data.strip_prefix("0X")) .unwrap_or(trimmed_data); - if data_without_prefix.trim().is_empty() { + if data_without_prefix.is_empty() { return Err(DecodeError::MissingRequiredField {crates/common/src/raindex_client/local_db/executor.rs (1)
17-56: Include SQL context in error messagesAdd the SQL text to callback and Promise errors to aid debugging.
async fn invoke_statement(&self, stmt: &SqlStatement) -> Result<String, LocalDbQueryError> { @@ - let result = self + let result = self .callback .call2( &JsValue::NULL, - &JsValue::from_str(&stmt.sql), + &JsValue::from_str(&stmt.sql), &js_params_val, ) .map_err(|e| { - LocalDbQueryError::database(format!( - "JavaScript callback invocation failed: {:?}", - e - )) + LocalDbQueryError::database(format!( + "JavaScript callback invocation failed: {:?}; sql={}", + e, + stmt.sql + )) })?; @@ - let js_result = future.await.map_err(|e| { - LocalDbQueryError::database(format!("Promise resolution failed: {:?}", e)) - })?; + let js_result = future.await.map_err(|e| { + LocalDbQueryError::database(format!( + "Promise resolution failed: {:?}; sql={}", + e, + stmt.sql + )) + })?;crates/cli/src/commands/local_db/sync/runner/window.rs (1)
98-104: Avoid potential u64 overflow when bumping the start blockUse saturating_add to mirror the checked_add used earlier and avoid edge overflow.
pub(super) fn default_start_block(last_synced_block: u64, deployment_block: u64) -> u64 { if last_synced_block == 0 { deployment_block } else { - last_synced_block + 1 + last_synced_block.saturating_add(1) } }crates/common/src/local_db/sync.rs (1)
71-78: Guard when already up to date (start_block > latest_block).If last_synced_block == latest_block, start_block becomes latest+1 and BlockRange::inclusive() will error. Add an early return before constructing the range.
let start_block = if last_synced_block == 0 { orderbook_cfg.deployment_block } else { last_synced_block.saturating_add(1) }; + if start_block > latest_block { + status.send("Database already synced; no new blocks.".to_string())?; + return Ok(()); + } let range = BlockRange::inclusive(start_block, latest_block)?;crates/common/src/local_db/insert.rs (1)
145-150: Use structured logging instead of eprintln!.Library code shouldn’t write to stderr directly. Prefer log::warn! so downstream can control sinks/levels.
- eprintln!( - "Warning: Unknown event type for transaction {}: {}", - event.transaction_hash, decoded.note - ); + log::warn!( + "Unknown event type for transaction {}: {}", + event.transaction_hash, + decoded.note + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (21)
crates/cli/src/commands/local_db/decode_events.rs(0 hunks)crates/cli/src/commands/local_db/decoded_events_to_sql.rs(5 hunks)crates/cli/src/commands/local_db/executor.rs(5 hunks)crates/cli/src/commands/local_db/mod.rs(0 hunks)crates/cli/src/commands/local_db/sync/data_source.rs(3 hunks)crates/cli/src/commands/local_db/sync/runner/apply.rs(17 hunks)crates/cli/src/commands/local_db/sync/runner/mod.rs(10 hunks)crates/cli/src/commands/local_db/sync/runner/window.rs(2 hunks)crates/cli/src/commands/local_db/sync/token.rs(3 hunks)crates/cli/src/commands/local_db/tokens_fetch.rs(0 hunks)crates/cli/src/commands/local_db/tokens_to_sql.rs(2 hunks)crates/cli/src/lib.rs(1 hunks)crates/common/src/local_db/decode.rs(2 hunks)crates/common/src/local_db/insert.rs(37 hunks)crates/common/src/local_db/mod.rs(4 hunks)crates/common/src/local_db/query/executor.rs(2 hunks)crates/common/src/local_db/query/mod.rs(1 hunks)crates/common/src/local_db/query/sql_statement.rs(10 hunks)crates/common/src/local_db/query/sql_statement_batch.rs(1 hunks)crates/common/src/local_db/sync.rs(9 hunks)crates/common/src/raindex_client/local_db/executor.rs(4 hunks)
💤 Files with no reviewable changes (3)
- crates/cli/src/commands/local_db/decode_events.rs
- crates/cli/src/commands/local_db/mod.rs
- crates/cli/src/commands/local_db/tokens_fetch.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-06T14:41:41.909Z
Learnt from: findolor
PR: rainlanguage/rain.orderbook#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/sync/runner/window.rscrates/cli/src/lib.rscrates/cli/src/commands/local_db/sync/data_source.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Deploy-Docs-Preview
- GitHub Check: Deploy-Preview-Push
🔇 Additional comments (19)
crates/common/src/local_db/query/executor.rs (1)
3-3: LGTM: Clean trait extension for batch execution.The addition of
execute_batchto theLocalDbQueryExecutortrait is well-designed and consistent with the existing query methods. The signature appropriately uses&SqlStatementBatchand returns the same error type as other methods.Also applies to: 12-13
crates/common/src/local_db/decode.rs (1)
904-917: LGTM: Comprehensive edge case coverage.The test properly validates that both "0x" and "0X" prefixes are treated as missing data, ensuring the validation logic handles all hex prefix variations correctly.
crates/common/src/local_db/query/mod.rs (1)
15-15: LGTM: Clean public API extension.The new module and re-exports properly expose the batch-based SQL types, maintaining consistency with the existing public API structure.
Also applies to: 20-20
crates/cli/src/commands/local_db/tokens_to_sql.rs (1)
74-105: LGTM: Correct batch materialization with proper placeholder handling.The helper functions correctly convert the batch to a SQL string:
batch_to_stringproperly concatenates statements with line breaksmaterialize_statementcorrectly iterates in reverse order (line 91) to avoid placeholder numbering conflicts (e.g., replacing ?1 before ?10)render_sql_valueprovides appropriate SQL literal escaping for the use caseNote: This materialization is for generating static SQL files, not runtime parameterized queries, so the basic escaping is sufficient.
crates/cli/src/commands/local_db/executor.rs (3)
19-24: LGTM: Appropriate U64 handling for SQLite.The conversion logic correctly handles SQLite's lack of unsigned 64-bit integers by attempting to convert to i64 first and falling back to text representation for values that don't fit. This ensures data fidelity while maintaining compatibility with SQLite's type system.
37-67: LGTM: Robust batch execution with proper transaction enforcement.The implementation correctly:
- Enforces transaction wrapping for batch safety (line 38-42)
- Sets a busy timeout to handle concurrent access (lines 46-47)
- Handles both parameterized and non-parameterized statements appropriately (lines 49-64)
- Provides clear error context at each failure point
The requirement for transactions ensures atomicity of batch operations, which is essential for maintaining database consistency.
154-202: LGTM: Comprehensive test coverage.The tests thoroughly validate:
- Batch execution with mixed statement types (CREATE, INSERT, queries)
- U64 parameter binding for both small and large values
- Transaction requirement enforcement
This provides strong confidence in the batch execution functionality.
Also applies to: 337-401
crates/common/src/local_db/query/sql_statement.rs (3)
9-9: LGTM: U64 support with appropriate type handling.The addition of the
U64variant and theFrom<u64>implementation (replacingTryFrom) simplifies the API by accepting all u64 values. The overflow handling is correctly deferred to the executor layer (which converts large values to text for SQLite compatibility).Also applies to: 28-31
48-58: LGTM: Useful convenience constructor.The
new_with_paramsmethod provides a clean way to create statements with pre-populated parameters while maintaining proper placeholder numbering.
269-312: LGTM: Thorough test coverage for U64 support.The tests comprehensively validate U64 handling across all relevant scenarios: direct push, bind_param_clause, bind_list_clause, and property-based testing. This ensures the U64 variant integrates correctly with all parameter binding mechanisms.
Also applies to: 346-353, 422-432, 569-581
crates/cli/src/lib.rs (1)
1-1: LGTM: Clean removal of obsolete CLI commands.The removal of FetchEvents, DecodeEvents, DecodedEventsToSql, TokensFetch, and TokensToSql commands aligns with the PR's migration to batch-based SQL generation handled internally by the Sync command. The simplified enum maintains only the essential Dump and Sync operations.
Also applies to: 10-24
crates/cli/src/commands/local_db/sync/token.rs (1)
6-10: LGTM: Clean migration to batch-based token SQL generation.The changes consistently migrate from string-based SQL to
SqlStatementBatch, aligning with the broader refactoring. The type changes inTokenPrepResultand usage ofgenerate_erc20_token_statementsmaintain the same functional behavior while enabling safer, parameterized SQL handling.Also applies to: 18-18, 37-37, 68-68, 76-76
crates/cli/src/commands/local_db/decoded_events_to_sql.rs (1)
109-123: Minor: newline handling is fine; consider making batch_to_string private to the modulebatch_to_string is already module‑private; no action needed. LGTM for legacy output formatting.
crates/cli/src/commands/local_db/sync/data_source.rs (1)
133-148: LGTM: trait now returns SqlStatementBatch and LocalDb impl delegates correctlyThe interface aligns with the batch-based refactor; error mapping is consistent.
crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
152-155: No changes required - prepare_sql already wraps batch via into_transaction()The verification confirms that
prepare_sqlincrates/cli/src/commands/local_db/sync/runner/apply.rscorrectly callsinto_transaction()on line 106 before returning, with proper error handling. The batch will be transaction-wrapped when passed toexecute_batch(), so no runtime error will occur.crates/common/src/local_db/insert.rs (2)
156-237: LGTM: raw_events batching and deterministic ordering.Sorted by (block_number, log_index), nullable timestamp handled via SqlValue::Null; all params bound. Looks solid.
865-907: No action required—schema correctly supports the ON CONFLICT clause.The verification confirms the
interpreter_store_setstable definesPRIMARY KEY (transaction_hash, log_index)(line 204 ofquery.sql), which provides the unique constraint the upsert needs. The ON CONFLICT clause in the code is correctly designed for this schema.crates/common/src/local_db/mod.rs (1)
202-221: API transition to batched statements looks good.Public surface now returns SqlStatementBatch with clear error mapping. No issues spotted.
crates/common/src/local_db/sync.rs (1)
51-55: No changes needed—the code already supports multi-statement SQL dumps.The
query_text()method at lines 74–77 ofcrates/cli/src/commands/local_db/executor.rsusesconn.execute_batch(stmt.sql())for statements without parameters. Sincedownload_and_decompress_dump()returns a plain SQL string with no parameters, this code path is taken, andexecute_batch()correctly handles multi-statement SQL.This is confirmed by existing tests:
execute_and_query_round_tripanddetects_required_tablesboth pass multi-statement SQL throughquery_text()successfully.
Comment on lines
+101
to
108
| tokens_prefix_sql.extend(data_source.events_to_sql(&decoded_events, &decimals_by_addr)?); | ||
| batch.extend(tokens_prefix_sql); | ||
| batch.add(build_update_last_synced_block_stmt(target_block)); | ||
|
|
||
| batch | ||
| .into_transaction() | ||
| .map_err(|e| anyhow::anyhow!("Failed to wrap SQL in transaction: {}", e)) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Wrap only if needed to avoid AlreadyTransaction.
Prevent spurious failures when a data source returns a pre-wrapped batch.
- batch
- .into_transaction()
- .map_err(|e| anyhow::anyhow!("Failed to wrap SQL in transaction: {}", e))
+ (if batch.is_transaction() {
+ Ok(batch)
+ } else {
+ batch.into_transaction()
+ })
+ .map_err(|e| anyhow::anyhow!("Failed to wrap SQL in transaction: {}", e))If you adopt SqlStatementBatch::ensure_transaction(), this becomes:
- batch.into_transaction()
+ batch.ensure_transaction()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In crates/cli/src/commands/local_db/sync/runner/apply.rs around lines 101 to
108, the code always calls into_transaction() which fails when the data source
already returned a transaction-wrapped batch; change this to use
SqlStatementBatch::ensure_transaction() (or implement it if missing) so the
batch is wrapped only when necessary, and keep the same map_err(...) error
handling to surface failures from ensuring a transaction.
Comment on lines
+369
to
+374
| async fn execute_batch(&self, batch: &SqlStatementBatch) -> Result<(), LocalDbQueryError> { | ||
| for stmt in batch { | ||
| let _ = self.query_text(stmt).await?; | ||
| } | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Test executor: ignore BEGIN/COMMIT to avoid brittle failures.
MockDb currently forwards all statements to query_text; transactional sentinels will fail unless mapped. Skip them in tests.
- async fn execute_batch(&self, batch: &SqlStatementBatch) -> Result<(), LocalDbQueryError> {
- for stmt in batch {
- let _ = self.query_text(stmt).await?;
- }
- Ok(())
- }
+ async fn execute_batch(&self, batch: &SqlStatementBatch) -> Result<(), LocalDbQueryError> {
+ for stmt in batch {
+ let sql = stmt.sql().trim();
+ if sql.eq_ignore_ascii_case("BEGIN TRANSACTION")
+ || sql.eq_ignore_ascii_case("COMMIT") {
+ continue;
+ }
+ let _ = self.query_text(stmt).await?;
+ }
+ Ok(())
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async fn execute_batch(&self, batch: &SqlStatementBatch) -> Result<(), LocalDbQueryError> { | |
| for stmt in batch { | |
| let _ = self.query_text(stmt).await?; | |
| } | |
| Ok(()) | |
| } | |
| async fn execute_batch(&self, batch: &SqlStatementBatch) -> Result<(), LocalDbQueryError> { | |
| for stmt in batch { | |
| let sql = stmt.sql().trim(); | |
| if sql.eq_ignore_ascii_case("BEGIN TRANSACTION") | |
| || sql.eq_ignore_ascii_case("COMMIT") { | |
| continue; | |
| } | |
| let _ = self.query_text(stmt).await?; | |
| } | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In crates/common/src/local_db/sync.rs around lines 369 to 374, the test executor
currently forwards all statements including transactional sentinels to
query_text which makes tests brittle; update execute_batch to skip statements
that are transactional sentinels (e.g., "BEGIN", "BEGIN;", "COMMIT", "COMMIT;")
by trimming whitespace and trailing semicolons and doing a case-insensitive
match, only calling query_text for non-sentinel statements and returning Ok(())
after the loop.
hardyjosh
approved these changes
Oct 24, 2025
0xgleb
approved these changes
Oct 27, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
crates/cli/src/commands/local_db/decoded_events_to_sql.rs(0 hunks)crates/cli/src/commands/local_db/executor.rs(5 hunks)crates/cli/src/commands/local_db/fetch_events.rs(0 hunks)crates/cli/src/commands/local_db/sync/data_source.rs(3 hunks)crates/cli/src/commands/local_db/sync/runner/apply.rs(17 hunks)crates/cli/src/commands/local_db/sync/runner/mod.rs(10 hunks)crates/cli/src/commands/local_db/tokens_to_sql.rs(0 hunks)crates/common/src/local_db/insert.rs(37 hunks)crates/common/src/local_db/mod.rs(4 hunks)crates/common/src/local_db/query/mod.rs(1 hunks)crates/common/src/local_db/query/sql_statement_batch.rs(1 hunks)crates/common/src/local_db/sync.rs(9 hunks)crates/common/src/raindex_client/local_db/executor.rs(5 hunks)
💤 Files with no reviewable changes (3)
- crates/cli/src/commands/local_db/decoded_events_to_sql.rs
- crates/cli/src/commands/local_db/fetch_events.rs
- crates/cli/src/commands/local_db/tokens_to_sql.rs
🧰 Additional context used
📓 Path-based instructions (3)
crates/*/{src,tests,benches,examples}/**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Rust code must pass clippy with all warnings denied (cargo clippy --workspace --all-targets --all-features -D warnings)
Files:
crates/common/src/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/cli/src/commands/local_db/executor.rscrates/common/src/raindex_client/local_db/executor.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/common/src/local_db/query/sql_statement_batch.rscrates/common/src/local_db/sync.rscrates/common/src/local_db/insert.rscrates/common/src/local_db/mod.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Rust code lives in the workspace under crates/* (e.g., cli, common, bindings, js_api, quote, subgraph, settings, math, integration_tests)
Files:
crates/common/src/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/cli/src/commands/local_db/executor.rscrates/common/src/raindex_client/local_db/executor.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/common/src/local_db/query/sql_statement_batch.rscrates/common/src/local_db/sync.rscrates/common/src/local_db/insert.rscrates/common/src/local_db/mod.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust with nix develop -c cargo fmt --all
Lint Rust with nix develop -c rainix-rs-static (preconfigured flags)
Use snake_case for Rust crates/modules and PascalCase for types
Files:
crates/common/src/local_db/query/mod.rscrates/cli/src/commands/local_db/sync/runner/mod.rscrates/cli/src/commands/local_db/executor.rscrates/common/src/raindex_client/local_db/executor.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/common/src/local_db/query/sql_statement_batch.rscrates/common/src/local_db/sync.rscrates/common/src/local_db/insert.rscrates/common/src/local_db/mod.rs
🧠 Learnings (33)
📓 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: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". This report should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2117
File: crates/js_api/src/gui/select_tokens.rs:421-424
Timestamp: 2025-09-02T08:04:34.729Z
Learning: User findolor in the rainlanguage/rain.orderbook codebase prefers not to change test code when tests are already passing, even if there appear to be type mismatches or potential issues. Working tests should be left alone unless there are actual errors.
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.
📚 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/sync/runner/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/common/src/local_db/sync.rscrates/common/src/local_db/mod.rs
📚 Learning: 2025-10-06T14:39:36.425Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2159
File: crates/cli/src/commands/local_db/sync/cli.rs:37-78
Timestamp: 2025-10-06T14:39:36.425Z
Learning: In the SyncLocalDb CLI command (crates/cli/src/commands/local_db/sync/cli.rs), block range validation (e.g., start_block <= end_block) is handled internally by the sync runner, not at the CLI entry point.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/data_source.rs
📚 Learning: 2025-10-06T11:44:07.888Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2145
File: crates/common/src/raindex_client/local_db/query/create_tables/query.sql:71-72
Timestamp: 2025-10-06T11:44:07.888Z
Learning: The local DB feature in the rain.orderbook codebase is not live yet (as of PR #2145), so schema migrations for existing databases are not required when modifying table structures in `crates/common/src/raindex_client/local_db/query/create_tables/query.sql`.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rscrates/cli/src/commands/local_db/executor.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/data_source.rscrates/common/src/local_db/mod.rs
📚 Learning: 2025-10-14T07:51:55.148Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2202
File: crates/common/src/raindex_client/local_db/sync.rs:33-34
Timestamp: 2025-10-14T07:51:55.148Z
Learning: In `crates/common/src/raindex_client/local_db/sync.rs`, the hard-coded `DEFAULT_SYNC_CHAIN_ID` constant (set to `SUPPORTED_LOCAL_DB_CHAINS[0]`) will be replaced with proper chain ID handling in downstream PRs as part of the multi-network/orderbook implementation.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/cli/src/commands/local_db/sync/data_source.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/sync/runner/mod.rscrates/cli/src/commands/local_db/executor.rscrates/cli/src/commands/local_db/sync/runner/apply.rscrates/common/src/local_db/sync.rs
📚 Learning: 2025-10-06T14:13:18.531Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2155
File: crates/common/src/raindex_client/trades.rs:133-152
Timestamp: 2025-10-06T14:13:18.531Z
Learning: In the rain.orderbook codebase, the `page` parameter in `RaindexOrder::get_trades_list` method (in crates/common/src/raindex_client/trades.rs) is kept for backwards compatibility with subgraph logic, but the LocalDb fast-path intentionally returns all trades without implementing pagination.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rs
📚 Learning: 2025-06-17T16:32:04.554Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1903
File: crates/js_api/src/gui/order_operations.rs:124-134
Timestamp: 2025-06-17T16:32:04.554Z
Learning: In the rain.orderbook codebase, RPC lists are typically small (2 items maximum), so performance optimizations around cloning and converting small Vec<Url> collections are generally unnecessary.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rs
📚 Learning: 2025-07-15T08:01:38.534Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1994
File: crates/common/src/raindex_client/vaults.rs:282-292
Timestamp: 2025-07-15T08:01:38.534Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor prefers to avoid concurrency optimizations like using `futures::future::try_join_all` for parallel processing of balance changes, considering such optimizations "not that critical at the moment" when the performance impact is minimal.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rs
📚 Learning: 2025-08-26T14:52:37.000Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2099
File: crates/common/src/hyper_rpc.rs:3-7
Timestamp: 2025-08-26T14:52:37.000Z
Learning: In the rain.orderbook codebase, creating new reqwest::Client instances per request in HyperRpcClient is not considered an issue by the maintainers, despite potential performance benefits of client reuse.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rs
📚 Learning: 2025-07-23T10:51:12.278Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2012
File: crates/js_api/src/registry.rs:233-252
Timestamp: 2025-07-23T10:51:12.278Z
Learning: In the rainlanguage/rain.orderbook codebase, findolor considers the pattern of creating a partially initialized struct instance within a constructor and then populating it through async operations to be acceptable, as long as the partial state is never exposed to users and the constructor returns Result<T, E> with proper error propagation using the ? operator.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rs
📚 Learning: 2025-10-13T12:57:05.900Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2198
File: crates/common/src/raindex_client/local_db/query/fetch_store_addresses/mod.rs:7-8
Timestamp: 2025-10-13T12:57:05.900Z
Learning: In the rain.orderbook repository, using unqualified serde helper names (e.g., `#[serde(with = "serde_address")]`) that rely on glob imports is acceptable and does not need to be changed to fully qualified paths.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rs
📚 Learning: 2025-10-02T19:17:20.332Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2163
File: crates/common/src/raindex_client/orders.rs:738-741
Timestamp: 2025-10-02T19:17:20.332Z
Learning: In crates/common/src/raindex_client/orders.rs, fetch_dotrain_source() is intentionally called in try_from_sg_order for every order conversion because the dotrain source information is needed immediately. A future optimization with local DB logic is planned to eliminate the network round-trip concern.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.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/sync/runner/mod.rscrates/cli/src/commands/local_db/sync/runner/apply.rs
📚 Learning: 2025-05-17T15:32:28.733Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1790
File: tauri-app/src-tauri/src/commands/vault.rs:67-67
Timestamp: 2025-05-17T15:32:28.733Z
Learning: For the PR focused on testing Tauri commands::order module, the generic type parameter R: Runtime was selectively added where needed for the PR scope, applying the changes primarily to order.rs and related files while leaving other modules like vault.rs for potential future refactoring.
Applied to files:
crates/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.rscrates/common/src/raindex_client/local_db/executor.rscrates/common/src/local_db/sync.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.rscrates/cli/src/commands/local_db/sync/runner/apply.rs
📚 Learning: 2025-05-19T13:40:56.080Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1858
File: crates/subgraph/src/orderbook_client/mod.rs:54-58
Timestamp: 2025-05-19T13:40:56.080Z
Learning: The `wasm_bindgen_utils` crate in the Rain Orderbook project handles conditional compilation for `JsValue` and `JsError` internally, allowing `impl From<Error> for JsValue` to work on non-WASM targets without explicit cfg guards.
Applied to files:
crates/common/src/raindex_client/local_db/executor.rs
📚 Learning: 2025-05-14T05:51:50.277Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:55-59
Timestamp: 2025-05-14T05:51:50.277Z
Learning: In the Rain Orderbook project, the error handling for WASM exported functions uses WasmEncodedError which includes both the original error message and a human-readable version, making direct modification of the JsValue conversion unnecessary.
Applied to files:
crates/common/src/raindex_client/local_db/executor.rs
📚 Learning: 2025-04-30T09:28:36.960Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using `#[cfg(target_family = "wasm")]` and `#[cfg(not(target_family = "wasm"))]`, and don't require additional `wasm_bindgen_test_configure!(run_in_browser)` directives.
Applied to files:
crates/common/src/raindex_client/local_db/executor.rscrates/common/src/local_db/sync.rs
📚 Learning: 2025-10-19T15:52:58.239Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2245
File: crates/common/src/raindex_client/local_db/executor.rs:17-46
Timestamp: 2025-10-19T15:52:58.239Z
Learning: When formatting JsValue errors in Rust wasm code using `{:?}`, the Debug output is sufficiently informative and does not produce unhelpful "[object Object]" strings in practice.
Applied to files:
crates/common/src/raindex_client/local_db/executor.rs
📚 Learning: 2025-05-27T06:16:12.195Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1874
File: tauri-app/src-tauri/src/transaction_status.rs:87-114
Timestamp: 2025-05-27T06:16:12.195Z
Learning: In tauri-app/src-tauri/src/transaction_status.rs, testing one WriteTransactionStatus variant (like PendingPrepare) is sufficient for the update_status_and_emit method since all variants follow the same code path through the From<WriteTransactionStatus<T>> implementation.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/apply.rscrates/common/src/local_db/query/sql_statement_batch.rscrates/common/src/local_db/sync.rscrates/common/src/local_db/insert.rs
📚 Learning: 2025-05-27T06:16:35.738Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1874
File: tauri-app/src-tauri/src/transaction_status.rs:69-127
Timestamp: 2025-05-27T06:16:35.738Z
Learning: In tauri-app/src-tauri/src/transaction_status.rs, directly accessing internal RwLock state in tests using `notice.0.read().unwrap()` is acceptable and does not require encapsulation through public getter methods.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/apply.rscrates/common/src/local_db/sync.rs
📚 Learning: 2025-10-06T13:03:35.337Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2146
File: crates/cli/src/commands/local_db/README.md:22-106
Timestamp: 2025-10-06T13:03:35.337Z
Learning: In the rain.orderbook repository, `cargo run` commands work without the `--` separator (e.g., `cargo run local-db fetch-events` works directly). Do not suggest adding `--` before CLI arguments in cargo run examples for this repository.
Applied to files:
crates/cli/src/commands/local_db/sync/runner/apply.rs
📚 Learning: 2025-05-20T10:20:08.206Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.orderbook PR: 1859
File: crates/quote/src/quote_debug.rs:472-492
Timestamp: 2025-05-20T10:20:08.206Z
Learning: In the Rain Orderbook codebase, the `#[tokio::test(flavor = "multi_thread")]` annotation is specifically needed for tests that use `LocalEvm`, not just for consistency across all async tests.
Applied to files:
crates/common/src/local_db/sync.rscrates/common/src/local_db/insert.rs
📚 Learning: 2025-07-18T10:31:05.498Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like `#[cfg(not(target_family = "wasm"))] use super::*;` in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Applied to files:
crates/common/src/local_db/sync.rs
📚 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/common/src/local_db/sync.rs
📚 Learning: 2025-08-02T02:34:32.237Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:446-533
Timestamp: 2025-08-02T02:34:32.237Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore WASM tests (crates/js_api/src/filters/raindex_filter_store.rs), brusherru decided to focus on testing only methods without side effects (that don't use web_sys) due to difficulties with mocking localStorage and window APIs in the WASM test environment. This pragmatic approach tests pure logic separately from browser integration.
Applied to files:
crates/common/src/local_db/sync.rs
📚 Learning: 2025-07-31T19:34:11.716Z
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: crates/common/src/raindex_client/vaults_list.rs:363-423
Timestamp: 2025-07-31T19:34:11.716Z
Learning: In the rainlanguage/rain.orderbook project, for WASM-exposed functionality like VaultsList, the team prefers to keep comprehensive tests in the non-WASM environment due to the complexity of recreating objects like RaindexVaults in WASM. WASM tests focus on basic functionality and error cases since the WASM code reuses the already-tested non-WASM implementation.
Applied to files:
crates/common/src/local_db/sync.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/common/src/local_db/sync.rs
📚 Learning: 2025-10-06T13:09:36.912Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2146
File: crates/common/src/raindex_client/local_db/insert.rs:166-195
Timestamp: 2025-10-06T13:09:36.912Z
Learning: In the rainlanguage/rain.orderbook codebase, manual SQL string construction with apostrophe escaping (replacing single quotes with double single quotes) is acceptable for inserting token metadata into SQLite, as it meets the security requirements for their use case.
Applied to files:
crates/common/src/local_db/insert.rs
📚 Learning: 2025-09-30T21:18:01.636Z
Learnt from: hardyjosh
Repo: rainlanguage/rain.orderbook PR: 2167
File: crates/virtual-raindex/src/engine/take.rs:131-138
Timestamp: 2025-09-30T21:18:01.636Z
Learning: In the virtual-raindex take order flow (crates/virtual-raindex/src/engine/take.rs), balance diffs are written from the order's perspective where the taker is the counterparty: the order's input column receives taker_output (what the taker provides to the order) and the order's output column receives taker_input (what the taker requests from the order).
Applied to files:
crates/common/src/local_db/insert.rs
📚 Learning: 2025-10-10T05:16:12.250Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2186
File: crates/common/src/raindex_client/local_db/query/update_last_synced_block/query.sql:4-5
Timestamp: 2025-10-10T05:16:12.250Z
Learning: In the rain.orderbook codebase, SQL templates use string replacement rather than parameterized queries. Placeholders that represent string values (like `?orderbook_address`) should remain quoted in the SQL template (e.g., `'?orderbook_address'`) because the implementation escapes and replaces these placeholders directly, resulting in properly quoted SQL values after replacement.
Applied to files:
crates/common/src/local_db/insert.rs
🧬 Code graph analysis (10)
crates/common/src/local_db/query/mod.rs (1)
crates/common/src/raindex_client/local_db/query/update_last_synced_block.rs (1)
update_last_synced_block(4-10)
crates/cli/src/commands/local_db/sync/runner/mod.rs (4)
crates/cli/src/commands/local_db/sync/runner/apply.rs (2)
prepare_sql(62-106)raw_events_to_statements(210-216)crates/common/src/local_db/query/sql_statement_batch.rs (1)
statements(61-63)crates/cli/src/commands/local_db/sync/data_source.rs (2)
raw_events_to_statements(42-45)raw_events_to_statements(144-150)crates/common/src/local_db/insert.rs (2)
raw_events_to_statements(156-237)raw_events(168-191)
crates/cli/src/commands/local_db/executor.rs (3)
crates/common/src/local_db/sync.rs (5)
serde_json(392-392)execute_batch(374-379)query_text(396-405)new(351-357)query_json(381-394)crates/common/src/local_db/query/sql_statement.rs (7)
params(64-66)new(41-46)new(181-185)from(14-16)from(19-21)from(24-26)from(29-31)crates/common/src/local_db/query/executor.rs (3)
execute_batch(12-12)query_text(18-18)query_json(14-16)
crates/common/src/raindex_client/local_db/executor.rs (5)
crates/cli/src/commands/local_db/executor.rs (4)
invoke_statement(42-57)execute_batch(62-79)new(28-32)query_text(81-85)crates/common/src/local_db/sync.rs (3)
execute_batch(374-379)new(351-357)query_text(396-405)crates/common/src/local_db/query/executor.rs (2)
execute_batch(12-12)query_text(18-18)crates/common/src/local_db/query/sql_statement_batch.rs (3)
new(12-14)len(65-67)from(79-81)crates/common/src/local_db/query/sql_statement.rs (8)
new(41-46)new(181-185)sql(60-62)params(64-66)from(14-16)from(19-21)from(24-26)from(29-31)
crates/cli/src/commands/local_db/sync/runner/apply.rs (5)
crates/common/src/local_db/query/update_last_synced_block/mod.rs (1)
build_update_last_synced_block_stmt(5-10)crates/cli/src/commands/local_db/sync/token.rs (1)
prepare_token_metadata(22-85)crates/common/src/local_db/insert.rs (2)
raw_events(168-191)raw_events_to_statements(156-237)crates/common/src/local_db/query/sql_statement_batch.rs (5)
statements(61-63)new(12-14)from(79-81)is_empty(69-71)len(65-67)crates/cli/src/commands/local_db/sync/data_source.rs (2)
raw_events_to_statements(42-45)raw_events_to_statements(144-150)
crates/cli/src/commands/local_db/sync/data_source.rs (5)
crates/common/src/local_db/mod.rs (2)
raw_events_to_statements(249-256)decoded_events_to_statements(237-247)crates/cli/src/commands/local_db/sync/runner/apply.rs (1)
raw_events_to_statements(210-216)crates/common/src/local_db/insert.rs (3)
raw_events_to_statements(156-237)raw_events(168-191)decoded_events_to_statements(95-154)crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
raw_events_to_statements(329-335)crates/cli/src/commands/local_db/sync/runner/window.rs (1)
raw_events_to_statements(191-193)
crates/common/src/local_db/query/sql_statement_batch.rs (1)
crates/common/src/local_db/query/sql_statement.rs (1)
sql(60-62)
crates/common/src/local_db/sync.rs (5)
crates/cli/src/commands/local_db/executor.rs (2)
new(28-32)execute_batch(62-79)crates/common/src/local_db/query/sql_statement_batch.rs (3)
new(12-14)statements(61-63)len(65-67)crates/common/src/raindex_client/local_db/executor.rs (2)
new(13-15)execute_batch(61-76)crates/common/src/local_db/query/update_last_synced_block/mod.rs (1)
build_update_last_synced_block_stmt(5-10)crates/common/src/local_db/insert.rs (1)
generate_erc20_token_statements(240-274)
crates/common/src/local_db/insert.rs (4)
crates/common/src/local_db/mod.rs (3)
decoded_events_to_statements(237-247)new(205-207)raw_events_to_statements(249-256)crates/common/src/local_db/query/sql_statement_batch.rs (3)
new(12-14)len(65-67)statements(61-63)crates/common/src/local_db/query/sql_statement.rs (2)
new_with_params(48-58)sql(60-62)crates/common/src/local_db/tokens.rs (1)
build_event(74-83)
crates/common/src/local_db/mod.rs (5)
crates/common/src/local_db/insert.rs (3)
decoded_events_to_statements(95-154)raw_events_to_statements(156-237)raw_events(168-191)crates/cli/src/commands/local_db/sync/data_source.rs (2)
raw_events_to_statements(42-45)raw_events_to_statements(144-150)crates/cli/src/commands/local_db/sync/runner/apply.rs (1)
raw_events_to_statements(210-216)crates/cli/src/commands/local_db/sync/runner/mod.rs (1)
raw_events_to_statements(329-335)crates/cli/src/commands/local_db/sync/runner/window.rs (1)
raw_events_to_statements(191-193)
Comment on lines
+81
to
85
| async fn query_text(&self, stmt: &SqlStatement) -> Result<String, LocalDbQueryError> { | ||
| let conn = self.open_connection()?; | ||
| Self::invoke_statement(&conn, stmt)?; | ||
| Ok(String::new()) | ||
| } |
Contributor
There was a problem hiding this comment.
Restore row-returning behaviour for query_text
query_text now just runs invoke_statement, which in turn calls Connection::execute_batch/Statement::execute and drops the payload. That breaks every caller that issues a SELECT via this path: execute_batch errors as soon as the SQL produces rows, and even if it didn’t, we’d still hand back an empty String. The trait contract promises textual data (the JS executor still delivers it), and CLI commands like the dump/query helpers rely on it. Please reinstate the previous logic—prepare the statement, read the result set, and build the text (or delegate to the query_json implementation and stringify)—instead of using the non-row API here.
🤖 Prompt for AI Agents
In crates/cli/src/commands/local_db/executor.rs around lines 81 to 85,
query_text currently calls invoke_statement which uses execute_batch/execute and
drops any row payloads, returning an empty String and breaking SELECT callers;
restore the previous row-returning behavior by preparing the statement on the
opened connection, executing it as a query, iterating the result rows to build a
textual representation (or call the existing query_json to get JSON and
stringify it) and return that text, instead of using
invoke_statement/execute_batch; ensure errors are propagated as
LocalDbQueryError and resources (statement/result) are properly closed.
Comment on lines
+37
to
+53
| /// Wrap the batch in a transaction when needed; no-op if already wrapped. | ||
| pub fn ensure_transaction(mut self) -> Self { | ||
| if self | ||
| .statements | ||
| .first() | ||
| .is_some_and(|stmt| is_begin(stmt.sql())) | ||
| || self | ||
| .statements | ||
| .last() | ||
| .is_some_and(|stmt| is_commit(stmt.sql())) | ||
| { | ||
| return self; | ||
| } | ||
|
|
||
| self.wrap_with_transaction(); | ||
| self | ||
| } |
Contributor
There was a problem hiding this comment.
ensure_transaction drops COMMIT after extend
ensure_transaction bails out as soon as either the first or last statement looks transactional. If we extend a pre-wrapped batch (e.g., prep.tokens_prefix_sql) with more statements, the leading BEGIN TRANSACTION survives but its matching COMMIT is now in the middle of the batch. Because of the early return, we never append a new closing COMMIT, so batch.is_transaction() stays false and downstream executors (see LocalDbQueryExecutor::execute_batch) reject the batch at runtime. This regresses legitimate workflows that mix already-wrapped fragments with additional statements. Please make sure we only short-circuit when both boundaries are present and strip stray sentinels before wrapping to avoid nested BEGINs. For example:
- if self
- .statements
- .first()
- .is_some_and(|stmt| is_begin(stmt.sql()))
- || self
- .statements
- .last()
- .is_some_and(|stmt| is_commit(stmt.sql()))
- {
- return self;
- }
-
- self.wrap_with_transaction();
+ let has_begin = self
+ .statements
+ .first()
+ .is_some_and(|stmt| is_begin(stmt.sql()));
+ let has_commit = self
+ .statements
+ .last()
+ .is_some_and(|stmt| is_commit(stmt.sql()));
+
+ if has_begin && has_commit {
+ return self;
+ }
+
+ if has_begin {
+ let _ = self.statements.remove(0);
+ }
+ if has_commit {
+ let _ = self.statements.pop();
+ }
+ self.wrap_with_transaction();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Wrap the batch in a transaction when needed; no-op if already wrapped. | |
| pub fn ensure_transaction(mut self) -> Self { | |
| if self | |
| .statements | |
| .first() | |
| .is_some_and(|stmt| is_begin(stmt.sql())) | |
| || self | |
| .statements | |
| .last() | |
| .is_some_and(|stmt| is_commit(stmt.sql())) | |
| { | |
| return self; | |
| } | |
| self.wrap_with_transaction(); | |
| self | |
| } | |
| /// Wrap the batch in a transaction when needed; no-op if already wrapped. | |
| pub fn ensure_transaction(mut self) -> Self { | |
| let has_begin = self | |
| .statements | |
| .first() | |
| .is_some_and(|stmt| is_begin(stmt.sql())); | |
| let has_commit = self | |
| .statements | |
| .last() | |
| .is_some_and(|stmt| is_commit(stmt.sql())); | |
| if has_begin && has_commit { | |
| return self; | |
| } | |
| if has_begin { | |
| let _ = self.statements.remove(0); | |
| } | |
| if has_commit { | |
| let _ = self.statements.pop(); | |
| } | |
| self.wrap_with_transaction(); | |
| self | |
| } |
🤖 Prompt for AI Agents
In crates/common/src/local_db/query/sql_statement_batch.rs around lines 37–53,
the current early return checks only first OR last transactional sentinel and
can drop a COMMIT when extending a pre-wrapped fragment; change the logic to
only short-circuit when BOTH a leading BEGIN and a trailing COMMIT are present,
and otherwise do not return early — instead strip any stray sentinels (remove a
leading BEGIN if present, remove a trailing COMMIT if present) to avoid nested
BEGINs, then call wrap_with_transaction() and return the batch.
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=L 🧠 Learnings used |
This was referenced Nov 5, 2025
4 tasks
This was referenced Dec 2, 2025
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.
Motivation
See issues:
decoded_events_to_sql#2225Solution
SqlStatementBatchmodule for multipleSqlStatementsexecute_batchfunction toLocalDbQueryExecutortrait and implement for executorsSqlStatementnativelySqlStatementand batching logicChecks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Release Notes
Removed Features
fetch-events,decode-events,decoded-events-to-sql,tokens-fetch, andtokens-to-sqlCLI subcommands for local database operationssyncanddumpsubcommands remain availableImprovements