Feature explain query support #52
Conversation
…ld_use_query and fix explain_query implementation
- Add should_use_query_path NIF to detect SELECT/EXPLAIN/WITH/RETURNING queries - Modify handle_execute to route query-returning statements through query path - Implement explain_query callback to convert results to list of maps - Update tests for EXPLAIN QUERY PLAN correctness (4 cols, not 8) - Tests passing: explain_simple_test.exs (2/2), explain_query_test.exs (partial) Still needed: - Fix Ecto.Multi wrapper behavior for explain results with wrap_in_transaction - Verify all 10 explain_query_test.exs tests pass - Run full test suite
- Modified explain_query/4 callback to return {:ok, maps} for Ecto.Multi compatibility
(Ecto.Multi requires callbacks to return {:ok, value} or {:error, value})
- Fixed explain tests to handle ecto_sql quirk where wrap_in_transaction: false
returns {:ok, list} instead of just list (this is how other adapters work too)
- Added explanatory comments about the ecto_sql behaviour
- Fixed test cleanup to handle already-closed resources gracefully
- Both explain_simple_test.exs and explain_query_test.exs now pass (12/12 tests)
Document the new EXPLAIN QUERY PLAN feature including:
- Query detection in Rust NIF
- Ecto.Multi compatibility with {:ok, maps} return format
- Usage examples for Repo.explain and direct SQL execution
- Test coverage details
WalkthroughThe PR adds EXPLAIN QUERY PLAN support to the LibSql Ecto adapter by introducing query-type detection in the Rust NIF layer, implementing conditional routing between query and execute paths, normalising parameters, and formatting EXPLAIN results as lists of maps. It includes comprehensive test coverage and security documentation addressing UTF-8 validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Ecto
participant Adapter as Adapter<br/>(connection.ex)
participant LibSql as LibSql<br/>(ecto_libsql.ex)
participant Native as Native<br/>(native.ex)
participant NIF as NIF<br/>(Rust)
participant DB as SQLite
rect rgb(200, 240, 255)
Note over Ecto,DB: EXPLAIN Query Execution Flow
Ecto->>Adapter: explain/2 or explain/3
Adapter->>Adapter: Prefix SQL with<br/>EXPLAIN QUERY PLAN
Adapter->>LibSql: query(conn, sql, params, opts)
LibSql->>Native: should_use_query_path(sql)
Native->>NIF: should_use_query_path(sql)
NIF-->>Native: true (EXPLAIN detected)
Native-->>LibSql: true
alt Query Path (EXPLAIN detected)
LibSql->>LibSql: normalise_args_for_query(sql, args)
LibSql->>NIF: query_args(conn_id, mode, sync,<br/>sql, normalised_args)
NIF->>DB: Execute EXPLAIN QUERY PLAN
DB-->>NIF: Raw result (rows, columns)
NIF-->>LibSql: Raw result
LibSql->>LibSql: format_query_result(result)
LibSql->>LibSql: Convert rows + columns<br/>to list of maps
LibSql-->>Adapter: {:ok, maps}
end
Adapter->>Adapter: Return mapped results
Adapter-->>Ecto: {:ok, [%{id, parent,<br/>notused, detail}, ...]}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI Agents
In @lib/ecto_libsql.ex:
- Around line 190-192: The error clause in format_query_result/2 returns the
wrong tuple shape; change defp format_query_result({:error, reason}, state) to
return a 3-tuple matching the handle_execute/4 spec: build an
%EctoLibSql.Error{} from reason (e.g. %EctoLibSql.Error{message: reason} or
mapping fields appropriately) and return {:error, error_struct, state} instead
of the current 4-tuple.
In @lib/ecto_libsql/native.ex:
- Around line 925-982: The file fails CI because it isn't formatted with mix
format; run mix format on lib/ecto_libsql/native.ex (which contains the
execute_stmt/4 function and surrounding docs) to correct formatting issues,
re-run tests, and commit the formatted file; ensure you run mix format
--check-formatted in CI and locally (or just mix format
lib/ecto_libsql/native.ex) so execute_stmt, query_stmt,
normalise_arguments_for_stmt and execute_prepared code conforms to project
style.
- Around line 905-923: The Elixir helper statement_returns_rows?/1 duplicates
NIF logic in should_use_query_path/1; update execute_stmt/4 to call
should_use_query_path(sql) (the NIF) instead of using statement_returns_rows?/1,
remove or deprecate the duplicate helper, and ensure execute_stmt/4 handles the
boolean result from should_use_query_path/1 the same way the current helper was
used (including any error/fallback behavior if the NIF may be unavailable).
🧹 Nitpick comments (2)
test/explain_simple_test.exs (2)
76-76: Remove debug code before merging.The
IO.inspectcall appears to be debug code left in the test. Consider removing it unless there's a specific reason to keep it for documentation purposes.🔎 Proposed fix
result = Ecto.Adapters.SQL.explain(TestRepo, :all, query) - IO.inspect(result, label: "EXPLAIN result") # Check it's a list of results assert is_list(result)
1-81: Consider adding error case tests.The current tests only cover successful EXPLAIN operations. Consider adding tests for error scenarios such as:
- EXPLAIN on non-existent tables
- EXPLAIN with invalid SQL syntax
- EXPLAIN on tables without permissions (if applicable)
As per coding guidelines, tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.beads/last-touchedCHANGELOG.mdlib/ecto/adapters/libsql/connection.exlib/ecto_libsql.exlib/ecto_libsql/native.exnative/ecto_libsql/src/hooks.rsnative/ecto_libsql/src/utils.rstest/explain_query_test.exstest/explain_simple_test.exs
🧰 Additional context used
📓 Path-based instructions (11)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/hooks.rsnative/ecto_libsql/src/utils.rs
native/ecto_libsql/src/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/*.rs: Lock Arc<Mutex> usingsafe_lock_arc(&client, "function_name client")?instead of.unwrap()to prevent panics
Handle Options using.ok_or_else()pattern with proper error messages instead of.unwrap()
Files:
native/ecto_libsql/src/hooks.rsnative/ecto_libsql/src/utils.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/hooks.rsnative/ecto_libsql/src/utils.rs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
native/ecto_libsql/src/hooks.rslib/ecto/adapters/libsql/connection.exnative/ecto_libsql/src/utils.rstest/explain_simple_test.exslib/ecto_libsql.exlib/ecto_libsql/native.exCHANGELOG.mdtest/explain_query_test.exs
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formatted
**/*.{ex,exs}: Use named parameters (:name,@name, or$namesyntax) for clarity and maintainability in SQL queries with multiple parameters
Always handle transaction errors with proper error recovery using with clauses or case statements to ensure rollback on failure
Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Use batch operations (batch/batch_transactional) instead of individual inserts to reduce database roundtrips and improve performance
Use cursors and streams for large result sets instead of loading all rows into memory to prevent memory exhaustion
Always pattern match on error tuples from database operations rather than using destructuring assignment to avoid unhandled failures
Use savepoints for partial rollback within transactions to enable error recovery and batch import patterns
Always preload associations before accessing them to avoid N+1 query problems in Ecto queries
Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Use vector operations for semantic search instead of LIKE pattern matching to provide more relevant search results based on embeddings
Use DBConnection.stream() with max_rows parameter for memory-efficient processing of large result sets
Files:
lib/ecto/adapters/libsql/connection.extest/explain_simple_test.exslib/ecto_libsql.exlib/ecto_libsql/native.extest/explain_query_test.exs
lib/ecto/adapters/libsql/connection.ex
📄 CodeRabbit inference engine (CLAUDE.md)
Add SQLite function support by defining expression handlers for native SQLite functions
Files:
lib/ecto/adapters/libsql/connection.ex
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/explain_simple_test.exstest/explain_query_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/explain_simple_test.exstest/explain_query_test.exs
lib/ecto_libsql/native.ex
📄 CodeRabbit inference engine (CLAUDE.md)
Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Files:
lib/ecto_libsql/native.ex
**/*.{md,markdown}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British/Australian English for all documentation files (except SQL keywords and compatibility requirements)
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update CHANGELOG.md when adding new features or fixing bugs to document changes
Files:
CHANGELOG.md
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type dumpers for converting Ecto types to SQLite values using `dumpers/2` function
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
native/ecto_libsql/src/hooks.rsnative/ecto_libsql/src/utils.rslib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
native/ecto_libsql/src/hooks.rsnative/ecto_libsql/src/utils.rslib/ecto_libsql/native.exCHANGELOG.md
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
native/ecto_libsql/src/hooks.rsnative/ecto_libsql/src/utils.rslib/ecto_libsql.exlib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/hooks.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
native/ecto_libsql/src/hooks.rslib/ecto/adapters/libsql/connection.exnative/ecto_libsql/src/utils.rslib/ecto_libsql/native.exCHANGELOG.mdtest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to AGENTS.md : Update AGENTS.md API reference when adding new public NIF functions or Ecto features that users should know about
Applied to files:
native/ecto_libsql/src/hooks.rslib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Applied to files:
lib/ecto/adapters/libsql/connection.exnative/ecto_libsql/src/utils.rslib/ecto_libsql/native.exCHANGELOG.md
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
Applied to files:
lib/ecto/adapters/libsql/connection.extest/explain_simple_test.exsCHANGELOG.mdtest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use named parameters (`:name`, `name`, or `$name` syntax) for clarity and maintainability in SQL queries with multiple parameters
Applied to files:
lib/ecto/adapters/libsql/connection.exnative/ecto_libsql/src/utils.rslib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to lib/**/*schema*.ex : Use Ecto changesets with changeset/2 functions to validate and cast data before database operations
Applied to files:
lib/ecto/adapters/libsql/connection.extest/explain_simple_test.exslib/ecto_libsql.exlib/ecto_libsql/native.extest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Applied to files:
lib/ecto/adapters/libsql/connection.exlib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
native/ecto_libsql/src/utils.rstest/explain_simple_test.exsCHANGELOG.mdtest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
native/ecto_libsql/src/utils.rslib/ecto_libsql.exlib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/explain_simple_test.exstest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
CHANGELOG.md
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type dumpers for converting Ecto types to SQLite values using `dumpers/2` function
Applied to files:
test/explain_query_test.exs
🧬 Code graph analysis (6)
native/ecto_libsql/src/hooks.rs (2)
native/ecto_libsql/src/utils.rs (1)
should_use_query(327-419)native/ecto_libsql/src/tests/utils_tests.rs (1)
should_use_query_tests(113-660)
lib/ecto/adapters/libsql/connection.ex (1)
lib/ecto_libsql/native.ex (1)
query(610-627)
test/explain_simple_test.exs (2)
lib/ecto/adapters/libsql/connection.ex (2)
query(717-719)from(765-768)lib/ecto_libsql/native.ex (1)
query(610-627)
lib/ecto_libsql.ex (3)
native/ecto_libsql/src/hooks.rs (1)
should_use_query_path(177-179)native/ecto_libsql/src/transaction.rs (1)
query_with_trx_args(322-401)native/ecto_libsql/src/query.rs (1)
query_args(33-116)
lib/ecto_libsql/native.ex (2)
native/ecto_libsql/src/hooks.rs (1)
should_use_query_path(177-179)native/ecto_libsql/src/statement.rs (1)
execute_prepared(164-221)
test/explain_query_test.exs (1)
lib/ecto/adapters/libsql/connection.ex (3)
delete_all(552-560)insert(563-584)from(765-768)
🪛 GitHub Actions: CI
lib/ecto/adapters/libsql/connection.ex
[error] 698-708: Mix format check failed. The following sections are not formatted: connection.ex. Run 'mix format' or 'mix format --check-formatted' to fix formatting.
lib/ecto_libsql.ex
[error] 156-171: Mix format check failed. Formatting changes suggested in ecto_libsql.ex. Run 'mix format' to fix formatting.
lib/ecto_libsql/native.ex
[error] 907-936: Mix format check failed. Formatting changes suggested in native.ex. Run 'mix format' to fix formatting.
test/explain_query_test.exs
[error] 223-230: Mix format check failed. Formatting changes suggested in test file. Run 'mix format' to fix formatting.
⏰ 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). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (10)
.beads/last-touched (1)
1-1: Metadata file update—no review needed.This is a build system tracking file and does not contain functional code requiring review.
native/ecto_libsql/src/utils.rs (1)
345-359: LGTM! EXPLAIN detection correctly implemented.The EXPLAIN keyword detection follows the established pattern for SELECT and WITH, using efficient byte-wise comparison with proper word boundary validation. This ensures EXPLAIN statements are correctly routed through the query path (returns rows).
CHANGELOG.md (1)
100-123: LGTM! Comprehensive documentation of EXPLAIN support.The changelog entry thoroughly documents the EXPLAIN QUERY PLAN feature with clear usage examples, implementation details, and test coverage notes. The documentation will help users understand the feature and maintainers locate the relevant code.
native/ecto_libsql/src/hooks.rs (1)
164-179: LGTM! Clean NIF bridge for query routing.The
should_use_query_pathfunction provides a clean bridge between Elixir and the Rust routing logic. The documentation clearly explains the routing rules, and the implementation correctly delegates to the underlyingshould_use_queryfunction.test/explain_query_test.exs (1)
1-252: Fix formatting and consider simplifying comment.The test coverage is comprehensive and well-structured:
- Covers all query types (SELECT, WHERE, ORDER BY, LIMIT, JOIN, UPDATE, DELETE)
- Tests options and output format
- Proper setup/teardown with database cleanup
- Follows testing best practices
However, the CI pipeline reports formatting violations at lines 223-230. Run
mix formatto fix formatting issues.Additionally, consider simplifying the comment at lines 217-220. The explanation of Ecto's wrap_in_transaction behaviour is useful but could be more concise.
#!/bin/bash # Verify formatting violations in test file mix format --check-formatted test/explain_query_test.exs⛔ Skipped due to learnings
Learnt from: CR Repo: ocean/ecto_libsql PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-06T06:36:01.571Z Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operationsLearnt from: CR Repo: ocean/ecto_libsql PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-06T06:36:01.571Z Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`Learnt from: CR Repo: ocean/ecto_libsql PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-06T06:36:01.571Z Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setupLearnt from: CR Repo: ocean/ecto_libsql PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-06T06:36:18.238Z Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performancelib/ecto/adapters/libsql/connection.ex (1)
696-714: Fix formatting before merge.The
explain_queryimplementation logic is correct:
- Correctly prepends
EXPLAIN QUERY PLANto the SQL- Uses
query()path (appropriate since EXPLAIN returns rows)- Properly transforms results to list of maps for Ecto consumption
- Returns
{:ok, maps}format required by Ecto.MultiHowever, the CI pipeline reports formatting violations. Run
mix formatto fix formatting issues.#!/bin/bash # Verify formatting violations cd lib/ecto/adapters/libsql && mix format --check-formatted connection.ex⛔ Skipped due to learnings
Learnt from: CR Repo: ocean/ecto_libsql PR: 0 File: AGENTS.md:0-0 Timestamp: 2026-01-06T06:36:18.238Z Learning: Applies to **/*.{ex,exs} : Use named parameters (`:name`, `name`, or `$name` syntax) for clarity and maintainability in SQL queries with multiple parameterstest/explain_simple_test.exs (2)
14-42: LGTM!The setup is well-structured with proper table creation and defensive cleanup using
try/catchto handle any errors during teardown.
44-55: LGTM!The test properly validates EXPLAIN QUERY PLAN output structure, including the expected column names for SQLite's explain format.
lib/ecto_libsql/native.ex (1)
148-149: LGTM!The NIF stub follows the established pattern for NIF functions in this module. It's appropriately marked with
@doc falseas an internal API.lib/ecto_libsql.ex (1)
155-177: Fix formatting to pass CI checks.The pipeline failure indicates this code needs to be formatted with
mix format. The routing logic itself is sound, with clear branching based on whether the query returns rows.As per coding guidelines, always format Elixir code before committing using
mix format --check-formatted.#!/bin/bash # Run mix format to fix formatting issues mix format lib/ecto_libsql.ex⛔ Skipped due to learnings
Learnt from: CR Repo: ocean/ecto_libsql PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-01-06T06:36:01.571Z Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
| @doc """ | ||
| Execute a prepared statement with arguments (for non-SELECT queries). | ||
| Returns the number of affected rows. | ||
| Execute a prepared statement with arguments. | ||
| Automatically routes to query_stmt if the statement returns rows (e.g., SELECT, EXPLAIN, RETURNING), | ||
| or to execute_prepared if it doesn't (e.g., INSERT/UPDATE/DELETE without RETURNING). | ||
| ## Parameters | ||
| - state: The connection state | ||
| - stmt_id: The statement ID from prepare/2 | ||
| - sql: The original SQL (for sync detection) | ||
| - sql: The original SQL (for sync detection and statement type detection) | ||
| - args: List of positional parameters OR map with atom keys for named parameters | ||
| ## Examples | ||
| # Positional parameters | ||
| # INSERT without RETURNING | ||
| {:ok, stmt_id} = EctoLibSql.Native.prepare(state, "INSERT INTO users (name) VALUES (?)") | ||
| {:ok, rows_affected} = EctoLibSql.Native.execute_stmt(state, stmt_id, sql, ["Alice"]) | ||
| {:ok, num_rows} = EctoLibSql.Native.execute_stmt(state, stmt_id, sql, ["Alice"]) | ||
| # Named parameters with atom keys | ||
| {:ok, stmt_id} = EctoLibSql.Native.prepare(state, "INSERT INTO users (name) VALUES (:name)") | ||
| {:ok, rows_affected} = EctoLibSql.Native.execute_stmt(state, stmt_id, sql, %{name: "Alice"}) | ||
| """ | ||
| # SELECT query | ||
| {:ok, stmt_id} = EctoLibSql.Native.prepare(state, "SELECT * FROM users WHERE id = ?") | ||
| {:ok, result} = EctoLibSql.Native.execute_stmt(state, stmt_id, sql, [1]) | ||
| # EXPLAIN query | ||
| {:ok, stmt_id} = EctoLibSql.Native.prepare(state, "EXPLAIN QUERY PLAN SELECT * FROM users") | ||
| {:ok, result} = EctoLibSql.Native.execute_stmt(state, stmt_id, sql, []) | ||
| # INSERT with RETURNING | ||
| {:ok, stmt_id} = EctoLibSql.Native.prepare(state, "INSERT INTO users (name) VALUES (?) RETURNING *") | ||
| {:ok, result} = EctoLibSql.Native.execute_stmt(state, stmt_id, sql, ["Alice"]) | ||
| """ | ||
| def execute_stmt( | ||
| %EctoLibSql.State{conn_id: conn_id, mode: mode, sync: syncx} = _state, | ||
| %EctoLibSql.State{conn_id: conn_id, mode: mode, sync: syncx} = state, | ||
| stmt_id, | ||
| sql, | ||
| args | ||
| ) do | ||
| # Normalise arguments (convert map to positional list if needed). | ||
| case normalise_arguments_for_stmt(conn_id, stmt_id, args) do | ||
| {:error, reason} -> | ||
| {:error, "Failed to normalise parameters: #{reason}"} | ||
|
|
||
| normalised_args -> | ||
| case execute_prepared(conn_id, stmt_id, mode, syncx, sql, normalised_args) do | ||
| num_rows when is_integer(num_rows) -> | ||
| {:ok, num_rows} | ||
|
|
||
| {:error, reason} -> | ||
| {:error, reason} | ||
| end | ||
| # Check if this statement returns rows | ||
| if statement_returns_rows?(sql) do | ||
| # Use query_stmt path for statements that return rows | ||
| query_stmt(state, stmt_id, args) | ||
| else | ||
| # Use execute path for statements that don't return rows | ||
| # Normalise arguments (convert map to positional list if needed). | ||
| case normalise_arguments_for_stmt(conn_id, stmt_id, args) do | ||
| {:error, reason} -> | ||
| {:error, "Failed to normalise parameters: #{reason}"} | ||
|
|
||
| normalised_args -> | ||
| case execute_prepared(conn_id, stmt_id, mode, syncx, sql, normalised_args) do | ||
| num_rows when is_integer(num_rows) -> | ||
| {:ok, num_rows} | ||
|
|
||
| {:error, reason} -> | ||
| {:error, reason} | ||
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Fix formatting to pass CI checks.
The pipeline failure indicates this code needs to be formatted with mix format. The implementation logic itself is sound, though note the separate comment about using the NIF directly for routing consistency.
As per coding guidelines, always format Elixir code before committing using mix format --check-formatted.
#!/bin/bash
# Run mix format to fix formatting issues
mix format lib/ecto_libsql/native.ex🤖 Prompt for AI Agents
In @lib/ecto_libsql/native.ex around lines 925 - 982, The file fails CI because
it isn't formatted with mix format; run mix format on lib/ecto_libsql/native.ex
(which contains the execute_stmt/4 function and surrounding docs) to correct
formatting issues, re-run tests, and commit the formatted file; ensure you run
mix format --check-formatted in CI and locally (or just mix format
lib/ecto_libsql/native.ex) so execute_stmt, query_stmt,
normalise_arguments_for_stmt and execute_prepared code conforms to project
style.
…cute
- Fix format_query_result/2 to return correct 3-tuple {:error, error, state}
instead of incorrect 4-tuple (matches handle_execute/4 spec)
- Add build_error/1 helpers to construct %EctoLibSql.Error{} from various
reason formats (binary, map, struct, other)
- Add normalise_args_for_query/2 to convert map arguments to lists for NIFs
- Add extract_named_params/1 to parse :name, $name, @name from SQL
The named parameter normalisation fixes 11 failing tests that broke when
EXPLAIN QUERY PLAN support routed SELECT queries through query_args/
query_with_trx_args NIFs (which expect lists, not maps).
- Remove statement_returns_rows?/1 Elixir helper that duplicated NIF logic - Update execute_stmt/4 to use should_use_query_path/1 NIF instead - Fix heredoc indentation warning (closing """ alignment) This ensures consistency with handle_execute/4 which also uses the NIF, and eliminates code duplication between Elixir and Rust.
- Fix outdated tests that expected EXPLAIN NOT to be detected - EXPLAIN queries return rows (query plan), so should_use_query returns true - Rename tests to reflect correct behaviour: - test_explain_select_detected (was test_explain_select_not_detected) - test_explain_insert_detected (was test_explain_insert_not_detected) - test_explain_update_delete_detected (was test_explain_update_delete_not_detected) - test_explain_queries_detected_as_returning_rows - Add new tests for case insensitivity and whitespace handling
- Remove debug IO.inspect call - Add test for EXPLAIN on non-existent table (error handling) - Add test for EXPLAIN with invalid SQL syntax (error handling) - Add test for EXPLAIN on empty table (edge case - still returns plan)
….exs Remove trailing whitespace and fix line formatting per mix format.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/explain_query_test.exs (1)
141-252: Consider adding error case tests for completeness.The tests cover happy path scenarios well. Per coding guidelines, tests should also cover error cases (invalid IDs, missing resources, constraint violations). Consider adding tests for:
- EXPLAIN on non-existent tables
- EXPLAIN with invalid SQL syntax
Note: These cases are covered in
explain_simple_test.exs, so this may be intentional separation of concerns.lib/ecto_libsql.ex (1)
241-248: Potential atom exhaustion with dynamic SQL parameter names.
String.to_atom/1creates atoms at runtime, which are not garbage collected. If this code processes SQL with user-controlled or highly dynamic parameter names, it could lead to atom table exhaustion.Consider using
String.to_existing_atom/1with a rescue clause, similar to the pattern inget_map_value_flexible/2innative.ex(lines 385-392), or keep parameter names as strings throughout.🔎 Proposed fix using existing atoms only
defp extract_named_params(sql) do # Match :name, $name, or @name patterns. ~r/[:$@]([a-zA-Z_][a-zA-Z0-9_]*)/ |> Regex.scan(sql) - |> Enum.map(fn [_full, name] -> String.to_atom(name) end) + |> Enum.map(fn [_full, name] -> + try do + String.to_existing_atom(name) + rescue + ArgumentError -> name + end + end) endThen update
normalise_args_for_query/2to handle both atom and string keys when fetching from the map.test/explain_simple_test.exs (1)
57-79: Consider moving inline schema module to module level.Defining
Usermodule inside the test function (lines 61-68) works but may cause recompilation warnings on repeated test runs. Consider moving it to the module level alongsideTestRepofor consistency withexplain_query_test.exs.🔎 Proposed refactor
defmodule TestRepo do use Ecto.Repo, otp_app: :ecto_libsql, adapter: Ecto.Adapters.LibSql end + defmodule User do + use Ecto.Schema + + schema "explain_test_users" do + field(:name, :string) + field(:email, :string) + end + end + @test_db "z_ecto_libsql_test-explain-simple.db"Then update the test to use the module-level
User:test "EXPLAIN via explain API returns rows" do - # Import Ecto.Query import Ecto.Query - defmodule User do - use Ecto.Schema - - schema "explain_test_users" do - field(:name, :string) - field(:email, :string) - end - end - # Build a simple query query = from(u in User, select: u.name)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/ecto/adapters/libsql/connection.exlib/ecto_libsql.exlib/ecto_libsql/native.exnative/ecto_libsql/src/tests/utils_tests.rstest/explain_query_test.exstest/explain_simple_test.exs
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formatted
**/*.{ex,exs}: Use named parameters (:name,@name, or$namesyntax) for clarity and maintainability in SQL queries with multiple parameters
Always handle transaction errors with proper error recovery using with clauses or case statements to ensure rollback on failure
Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Use batch operations (batch/batch_transactional) instead of individual inserts to reduce database roundtrips and improve performance
Use cursors and streams for large result sets instead of loading all rows into memory to prevent memory exhaustion
Always pattern match on error tuples from database operations rather than using destructuring assignment to avoid unhandled failures
Use savepoints for partial rollback within transactions to enable error recovery and batch import patterns
Always preload associations before accessing them to avoid N+1 query problems in Ecto queries
Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Use vector operations for semantic search instead of LIKE pattern matching to provide more relevant search results based on embeddings
Use DBConnection.stream() with max_rows parameter for memory-efficient processing of large result sets
Files:
test/explain_simple_test.exslib/ecto/adapters/libsql/connection.exlib/ecto_libsql/native.exlib/ecto_libsql.extest/explain_query_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/explain_simple_test.exstest/explain_query_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/explain_simple_test.exstest/explain_query_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/explain_simple_test.exslib/ecto/adapters/libsql/connection.exlib/ecto_libsql/native.exlib/ecto_libsql.exnative/ecto_libsql/src/tests/utils_tests.rstest/explain_query_test.exs
lib/ecto/adapters/libsql/connection.ex
📄 CodeRabbit inference engine (CLAUDE.md)
Add SQLite function support by defining expression handlers for native SQLite functions
Files:
lib/ecto/adapters/libsql/connection.ex
lib/ecto_libsql/native.ex
📄 CodeRabbit inference engine (CLAUDE.md)
Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Files:
lib/ecto_libsql/native.ex
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/tests/utils_tests.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/tests/utils_tests.rs
native/ecto_libsql/src/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
native/ecto_libsql/src/tests/utils_tests.rs
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/explain_simple_test.exstest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/explain_simple_test.exslib/ecto_libsql/native.exnative/ecto_libsql/src/tests/utils_tests.rstest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to lib/**/*schema*.ex : Use Ecto changesets with changeset/2 functions to validate and cast data before database operations
Applied to files:
test/explain_simple_test.exslib/ecto/adapters/libsql/connection.exlib/ecto_libsql/native.exlib/ecto_libsql.extest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
Applied to files:
test/explain_simple_test.exslib/ecto/adapters/libsql/connection.extest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
lib/ecto/adapters/libsql/connection.exlib/ecto_libsql/native.exlib/ecto_libsql.extest/explain_query_test.exs
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Applied to files:
lib/ecto/adapters/libsql/connection.exlib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Applied to files:
lib/ecto/adapters/libsql/connection.exlib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use named parameters (`:name`, `name`, or `$name` syntax) for clarity and maintainability in SQL queries with multiple parameters
Applied to files:
lib/ecto/adapters/libsql/connection.exlib/ecto_libsql/native.exlib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
lib/ecto_libsql/native.exlib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
lib/ecto_libsql/native.exnative/ecto_libsql/src/tests/utils_tests.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to AGENTS.md : Update AGENTS.md API reference when adding new public NIF functions or Ecto features that users should know about
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
lib/ecto_libsql/native.exlib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
lib/ecto_libsql/native.exlib/ecto_libsql.exnative/ecto_libsql/src/tests/utils_tests.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Always pattern match on error tuples from database operations rather than using destructuring assignment to avoid unhandled failures
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
native/ecto_libsql/src/tests/utils_tests.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
native/ecto_libsql/src/tests/utils_tests.rs
🧬 Code graph analysis (6)
test/explain_simple_test.exs (2)
lib/ecto/adapters/libsql/connection.ex (2)
query(719-721)from(767-770)lib/ecto_libsql/native.ex (1)
query(610-627)
lib/ecto/adapters/libsql/connection.ex (1)
lib/ecto_libsql/native.ex (1)
query(610-627)
lib/ecto_libsql/native.ex (2)
native/ecto_libsql/src/hooks.rs (1)
should_use_query_path(177-179)native/ecto_libsql/src/statement.rs (1)
execute_prepared(164-221)
lib/ecto_libsql.ex (3)
native/ecto_libsql/src/hooks.rs (1)
should_use_query_path(177-179)native/ecto_libsql/src/query.rs (1)
query_args(33-116)lib/ecto_libsql/query.ex (1)
to_string(49-51)
native/ecto_libsql/src/tests/utils_tests.rs (1)
native/ecto_libsql/src/utils.rs (1)
should_use_query(327-419)
test/explain_query_test.exs (1)
lib/ecto/adapters/libsql/connection.ex (3)
delete_all(552-560)insert(563-584)from(767-770)
⏰ 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). (4)
- GitHub Check: Rust Fuzz Testing
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (12)
native/ecto_libsql/src/tests/utils_tests.rs (2)
421-474: LGTM! Comprehensive EXPLAIN detection tests.The updated tests correctly validate the new EXPLAIN routing behaviour:
- Renamed tests accurately reflect that EXPLAIN queries are now detected as returning rows
- Case insensitivity tests cover mixed case variants
- Whitespace handling tests verify leading whitespace tolerance
- Coverage includes EXPLAIN SELECT, EXPLAIN INSERT, EXPLAIN UPDATE, and EXPLAIN DELETE
The assertions align with the implementation in
utils.rswhere EXPLAIN statements are pre-checked before other keyword detection.
642-649: LGTM! Test correctly renamed to reflect new behaviour.The test
test_explain_queries_detected_as_returning_rowsproperly validates that bothEXPLAIN SELECTandEXPLAIN QUERY PLAN SELECTare detected as queries returning rows, aligning with the query path routing logic.lib/ecto/adapters/libsql/connection.ex (1)
696-716: LGTM! Clean EXPLAIN QUERY PLAN implementation.The implementation correctly:
- Routes through the
query()path since EXPLAIN returns rows- Converts results to a list of maps as expected by Ecto's explain API
- Propagates errors without transformation
The use of
IO.iodata_to_binary/1is appropriate sincequeryis iodata.lib/ecto_libsql/native.ex (2)
148-150: LGTM! NIF stub follows established pattern.The
should_use_query_path/1NIF stub correctly follows the pattern used by other NIF stubs in this module, returning:nif_not_loadederror until the NIF is loaded. Based on learnings, this aligns with the guideline to provide NIF stubs in the Native module.
935-964: LGTM! Routing now uses NIF for consistency.The updated
execute_stmt/4now usesshould_use_query_path/1from the NIF layer (line 942), addressing the prior review feedback about avoiding duplicate routing logic between Elixir and Rust. This ensures consistent routing decisions across the codebase.The implementation correctly:
- Routes to
query_stmt/3for row-returning statements (EXPLAIN, SELECT, RETURNING)- Routes to
execute_prepared/6for non-row statements- Normalises arguments before execution
- Handles errors appropriately
test/explain_query_test.exs (2)
1-43: LGTM! Well-structured test module with proper schema definitions.The test module follows good practices:
- Inline TestRepo and schemas keep tests self-contained
changeset/2functions defined per coding guidelinesasync: falseis appropriate for shared database state
65-139: LGTM! Proper test lifecycle management.The setup correctly:
- Creates tables with IF NOT EXISTS for idempotency
- Creates indexes for realistic explain output
- Cleans up on exit with proper error handling
- Seeds fresh test data before each test
lib/ecto_libsql.ex (2)
155-187: LGTM! Correct routing implementation using NIF.The routing logic correctly:
- Uses
should_use_query_path/1from the NIF for consistent routing decisions- Routes to query path for row-returning statements (SELECT, EXPLAIN, RETURNING)
- Routes to execute path for non-row statements (INSERT/UPDATE/DELETE without RETURNING)
- Handles both transactional and non-transactional contexts
189-220: Error handling now returns correct tuple structure.The
format_query_result/2function correctly returns a 3-tuple{:error, error, state}matching thehandle_execute/4callback specification. Thebuild_error/1helpers properly normalise various error formats into%EctoLibSql.Error{}structs.test/explain_simple_test.exs (3)
1-42: LGTM! Good test setup with proper cleanup.The test module follows good practices:
- Uses
async: falsefor shared database state- Creates table with IF NOT EXISTS for idempotency
- Proper cleanup in
on_exitwith error handling
44-55: LGTM! Validates direct EXPLAIN QUERY PLAN execution.Good test that validates the expected EXPLAIN QUERY PLAN output format with columns
["id", "parent", "notused", "detail"].
81-105: LGTM! Good error case coverage.These tests properly cover error scenarios per coding guidelines:
- Non-existent table returns error with meaningful message
- Invalid SQL syntax returns error
- Empty table still returns query plan (edge case)
- Change extract_named_params/1 to return strings instead of atoms
- Add get_param_value/2 using String.to_existing_atom/1 with rescue
- Supports both atom keys (%{name: val}) and string keys (%{"name" => val})
This prevents potential atom table exhaustion when processing SQL with
user-controlled or highly dynamic parameter names, following the same
pattern used in get_map_value_flexible/2 in native.ex.
Move inline User module definition from inside test function to module level for consistency with explain_query_test.exs and to avoid potential recompilation warnings on repeated test runs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/ecto_libsql.ex (2)
155-187: Consider moving argument normalisation inside the query path branch.The arguments are normalised at line 160 before determining which path to use. However, the execute path (lines 179-185) already performs its own normalisation via
execute_with_trx/execute_non_trx(seelib/ecto_libsql/native.ex:674-691). This means normalisation at line 160 is redundant for queries that don't return rows.For better efficiency, consider moving
normalise_args_for_queryinside thetruebranch (lines 163-177) where it's actually needed.🔎 Potential refactor
# Check if query returns rows (SELECT, EXPLAIN, WITH, RETURNING clauses) # If so, route through query path instead of execute path sql = query_struct.statement - - # Convert map arguments to list if needed (NIFs expect lists). - normalised_args = normalise_args_for_query(sql, args) case EctoLibSql.Native.should_use_query_path(sql) do true -> + # Convert map arguments to list if needed (NIFs expect lists). + normalised_args = normalise_args_for_query(sql, args) + # Query returns rows, use the query path. if trx_id do EctoLibSql.Native.query_with_trx_args(trx_id, state.conn_id, sql, normalised_args)
249-256: Add a code comment documenting the regex parameter extraction limitation.The regex-based approach cannot distinguish between parameter-like patterns in SQL strings/comments and actual parameters. Whilst this is an edge case (would require unusual SQL patterns like
SELECT ':param_name'alongside actual parameters), adding an explicit comment in the function would clarify this known limitation for future maintainers.Consider adding to the function documentation something like:
Note: This regex approach matches parameter patterns anywhere in the SQL string, including within string literals or comments. Users should avoid parameter-like patterns in SQL strings (e.g., ':param_name' within quotes).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/ecto_libsql.extest/explain_simple_test.exs
🚧 Files skipped from review as they are similar to previous changes (1)
- test/explain_simple_test.exs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formatted
**/*.{ex,exs}: Use named parameters (:name,@name, or$namesyntax) for clarity and maintainability in SQL queries with multiple parameters
Always handle transaction errors with proper error recovery using with clauses or case statements to ensure rollback on failure
Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Use batch operations (batch/batch_transactional) instead of individual inserts to reduce database roundtrips and improve performance
Use cursors and streams for large result sets instead of loading all rows into memory to prevent memory exhaustion
Always pattern match on error tuples from database operations rather than using destructuring assignment to avoid unhandled failures
Use savepoints for partial rollback within transactions to enable error recovery and batch import patterns
Always preload associations before accessing them to avoid N+1 query problems in Ecto queries
Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Use vector operations for semantic search instead of LIKE pattern matching to provide more relevant search results based on embeddings
Use DBConnection.stream() with max_rows parameter for memory-efficient processing of large result sets
Files:
lib/ecto_libsql.ex
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
lib/ecto_libsql.ex
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to lib/**/*schema*.ex : Use Ecto changesets with changeset/2 functions to validate and cast data before database operations
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use named parameters (`:name`, `name`, or `$name` syntax) for clarity and maintainability in SQL queries with multiple parameters
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Always pattern match on error tuples from database operations rather than using destructuring assignment to avoid unhandled failures
Applied to files:
lib/ecto_libsql.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
lib/ecto_libsql.ex
🧬 Code graph analysis (1)
lib/ecto_libsql.ex (4)
native/ecto_libsql/src/hooks.rs (1)
should_use_query_path(177-179)native/ecto_libsql/src/transaction.rs (1)
query_with_trx_args(322-401)native/ecto_libsql/src/query.rs (1)
query_args(33-116)lib/ecto_libsql/native.ex (2)
execute_with_trx(675-692)execute_non_trx(605-607)
⏰ 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). (4)
- GitHub Check: Rust Fuzz Testing
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
lib/ecto_libsql.ex (1)
189-220: LGTM! Proper error handling and result formatting.The result formatting correctly handles both success and error cases, returning proper tuple structures that match the
handle_execute/4callback specification. Thebuild_error/1helper provides robust error normalisation with defensive pattern matching for various error formats.Note: The previous review concern about incorrect error tuple structure has been properly addressed.
Validates that all required named parameters exist in the map before executing queries, preventing silent NULL insertions or confusing SQLite errors. Applies to both query path (SELECT) and execute path (INSERT/UPDATE/DELETE). Changes: - Added has_param?/2 and has_map_key_flexible?/2 to check parameter existence - Added validate_params_exist/2 to validate and raise clear errors for missing params - Updated normalise_args_for_query/2 and map_to_positional_args/2 to validate parameters - Moved argument normalisation into query path branch to avoid redundant work - Enhanced tests to verify parameter validation for all query types - Updated case-sensitivity test to expect errors instead of silent failures Fixes issues where missing parameters would: 1. Pass NULL to nullable columns silently (incorrect behavior) 2. Generate confusing SQLite constraint errors (poor UX) 3. Allow case mismatches to silently fail (dangerous) All parameter validation now raises ArgumentError with clear messages listing which parameters are missing and which are required.
Added comprehensive comment to extract_named_params/1 explaining that the regex approach cannot distinguish between parameter-like patterns in SQL string literals/comments and actual parameters. The comment includes: - Clear example of the edge case (parameter in string literal) - Why this is rarely problematic in practice - Suggestion to use prepared statement introspection if needed This helps future maintainers understand the trade-offs of the simple regex approach versus a full SQL parser.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/ecto_libsql/native.ex (1)
410-428: Consider filtering out nil parameter names.When SQL uses positional parameters (
?),extract_param_name/3returnsnil. If a user passes a map to such a statement, this validation would flag all positional params as "missing" with a confusing error message showing:for each nil name.Consider filtering nil values from
param_namesbefore validation to produce clearer error messages, or explicitly handle the case where a map is provided but the statement uses positional parameters:🔎 Suggested improvement
defp validate_params_exist(param_map, param_names) do + # Filter out nil names (positional parameters) - maps can only supply named params + named_params = Enum.filter(param_names, &(&1 != nil)) + + if named_params == [] and map_size(param_map) > 0 do + raise ArgumentError, + "Statement uses positional parameters (?), but a named parameter map was provided. " <> + "Use a list for positional parameters." + end + missing_params = - Enum.filter(param_names, fn name -> + Enum.filter(named_params, fn name -> not has_map_key_flexible?(param_map, name) end)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/ecto_libsql.exlib/ecto_libsql/native.extest/named_parameters_execution_test.exs
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ecto_libsql.ex
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ex,exs}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ex,exs}: Use British/Australian English for all Elixir code comments and documentation (except SQL keywords and compatibility requirements)
Always format Elixir code before committing usingmix format --check-formatted
**/*.{ex,exs}: Use named parameters (:name,@name, or$namesyntax) for clarity and maintainability in SQL queries with multiple parameters
Always handle transaction errors with proper error recovery using with clauses or case statements to ensure rollback on failure
Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Use batch operations (batch/batch_transactional) instead of individual inserts to reduce database roundtrips and improve performance
Use cursors and streams for large result sets instead of loading all rows into memory to prevent memory exhaustion
Always pattern match on error tuples from database operations rather than using destructuring assignment to avoid unhandled failures
Use savepoints for partial rollback within transactions to enable error recovery and batch import patterns
Always preload associations before accessing them to avoid N+1 query problems in Ecto queries
Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Use vector operations for semantic search instead of LIKE pattern matching to provide more relevant search results based on embeddings
Use DBConnection.stream() with max_rows parameter for memory-efficient processing of large result sets
Files:
test/named_parameters_execution_test.exslib/ecto_libsql/native.ex
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests may use
.unwrap()for simplicity when handling test data and setup
Files:
test/named_parameters_execution_test.exs
test/**/*test.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Files:
test/named_parameters_execution_test.exs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
test/named_parameters_execution_test.exslib/ecto_libsql/native.ex
lib/ecto_libsql/native.ex
📄 CodeRabbit inference engine (CLAUDE.md)
Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Files:
lib/ecto_libsql/native.ex
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to test/**/*test.exs : Tests must cover happy path (successful operations), error cases (invalid IDs, missing resources, constraint violations), edge cases (NULL values, empty strings, large datasets), transaction rollback scenarios, type conversions, and concurrent operations
Applied to files:
test/named_parameters_execution_test.exs
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use named parameters (`:name`, `name`, or `$name` syntax) for clarity and maintainability in SQL queries with multiple parameters
Applied to files:
test/named_parameters_execution_test.exslib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
test/named_parameters_execution_test.exslib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to lib/**/*schema*.ex : Use Ecto changesets with changeset/2 functions to validate and cast data before database operations
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to AGENTS.md : Update AGENTS.md API reference when adding new public NIF functions or Ecto features that users should know about
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Applied to files:
lib/ecto_libsql/native.ex
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to **/*.{ex,exs} : Always format Elixir code before committing using `mix format --check-formatted`
Applied to files:
lib/ecto_libsql/native.ex
🧬 Code graph analysis (2)
test/named_parameters_execution_test.exs (1)
lib/ecto_libsql.ex (1)
handle_execute(148-188)
lib/ecto_libsql/native.ex (2)
native/ecto_libsql/src/hooks.rs (1)
should_use_query_path(177-179)native/ecto_libsql/src/statement.rs (1)
execute_prepared(164-221)
⏰ 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). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (9)
test/named_parameters_execution_test.exs (3)
480-493: Excellent improvement to error handling validation!The refactor from expecting successful execution to asserting ArgumentError is raised strengthens parameter validation testing. The error message clearly identifies missing parameters (:email, :age), making it easier for developers to diagnose issues.
495-508: Well-reasoned test for nullable column parameter validation!This test ensures that even nullable columns require explicit parameter provision, preventing silent NULL insertions from accidental omissions. This "explicit over implicit" approach reduces bugs and makes parameter requirements clear to users.
560-593: Robust case sensitivity validation!The two-phase test approach effectively validates both the error path (mismatched case raises ArgumentError) and the success path (matching case works correctly). The error message helpfully shows the exact case expected (:Name), making parameter case requirements explicit.
lib/ecto_libsql/native.ex (6)
148-150: LGTM!The NIF stub follows the established pattern and matches the Rust implementation in
hooks.rs. Based on learnings, this correctly provides the NIF stub in the Native module.
380-393: LGTM!Good defensive implementation that avoids atom table exhaustion by using
String.to_existing_atom/1with a proper rescue fallback. This correctly supports both%{name: value}and%{"name" => value}map styles without runtime atom creation.
568-571: LGTM!Good placement of validation after cache hit to catch missing parameters early, before conversion would silently produce
nilvalues. The comment accurately documents the raising behaviour.
604-606: LGTM!Consistent validation between cache hit and cache miss paths ensures uniform error behaviour regardless of cache state.
976-1005: LGTM! Addresses previous review feedback.This refactoring correctly uses the
should_use_query_path/1NIF for routing decisions, eliminating the duplicate Elixir-side detection logic that was flagged in the previous review. The implementation ensures consistency with the Rust layer's statement type detection.The documentation updates appropriately reflect the new EXPLAIN query support and routing behaviour.
946-975: Documentation updates look good.The expanded examples clearly demonstrate all routing scenarios: INSERT without RETURNING, SELECT queries, EXPLAIN queries, and INSERT with RETURNING. The parameter documentation correctly notes the dual purpose of the
sqlparameter.
Adds explicit UTF-8 validation at SQL entry points to provide defence-in-depth against CVE-2025-47736, a crash vulnerability in libsql-sqlite3-parser ≤ 0.13.0. Note: We are already protected by Rust's type system and Rustler's validation, but this adds an explicit safety layer and documentation. Changes: - Added validate_utf8_sql() function in utils.rs with comprehensive docs - Validates UTF-8 at all SQL entry points: - prepare_statement() in statement.rs - declare_cursor() in cursor.rs - execute_batch_native() in batch.rs - Created SECURITY.md documenting: - Our multi-layer mitigation strategy - Why we're not vulnerable (type system + Rustler + explicit validation) - Upstream fix status and update plan - Security best practices for users Protection layers: 1. Rust type system: &str guarantees valid UTF-8 2. Rustler validation: Elixir→Rust conversion validates UTF-8 3. Explicit validation: validate_utf8_sql() at every SQL entry point The vulnerability cannot be triggered through our API because invalid UTF-8 would fail at layers 1 or 2 before reaching libsql-sqlite3-parser. References: - CVE-2025-47736: https://advisories.gitlab.com/pkg/cargo/libsql-sqlite3-parser/CVE-2025-47736/ - Fix commit: 14f422a (not yet released to crates.io)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
native/ecto_libsql/src/batch.rs (1)
272-276: Add UTF-8 validation to execute_transactional_batch_native.The
execute_transactional_batch_nativefunction accepts ansqlparameter and executes it without UTF-8 validation. For consistency withexecute_batch_nativeand complete defence-in-depth coverage, add the same validation.🔎 Proposed fix
#[rustler::nif(schedule = "DirtyIo")] pub fn execute_transactional_batch_native<'a>( env: Env<'a>, conn_id: &str, sql: &str, ) -> NifResult<Term<'a>> { + // Validate UTF-8 as defence against CVE-2025-47736. + validate_utf8_sql(sql)?; + let conn_map = safe_lock( &CONNECTION_REGISTRY, "execute_transactional_batch_native conn_map", )?;
🤖 Fix all issues with AI agents
In @native/ecto_libsql/src/cursor.rs:
- Around line 34-35: Add the same UTF-8 validation call used elsewhere to the
start of declare_cursor_with_context: call utils::validate_utf8_sql(sql)? at the
top of the declare_cursor_with_context function (before decoding args or
executing queries) so the sql parameter is validated for UTF-8 and returns an
error early if invalid.
🧹 Nitpick comments (1)
native/ecto_libsql/src/utils.rs (1)
38-60: Tautological validation in char_indices iteration.The check at line 51 (
is_char_boundary(idx)) inside thechar_indices()iteration is tautological. By definition,char_indices()only yields indices that are valid UTF-8 character boundaries. This check will always pass and adds unnecessary runtime overhead without providing additional security value.Since the documentation acknowledges that
&stralready guarantees valid UTF-8, and Rustler validates UTF-8 during conversion, consider either:
- Simplifying to just attempt iteration without the boundary check:
for _ in sql.chars() {}- Removing the iteration entirely and relying solely on the initial boundary checks
🔎 Proposed simplification
pub fn validate_utf8_sql(sql: &str) -> Result<(), rustler::Error> { // This check is technically redundant since &str is guaranteed to be valid UTF-8, // but it serves as explicit documentation and defence-in-depth. if !sql.is_char_boundary(0) || !sql.is_char_boundary(sql.len()) { return Err(rustler::Error::Term(Box::new( "SQL contains invalid UTF-8 sequences", ))); } - // Additional validation: ensure the string can be iterated as UTF-8 chars. - // This will detect any invalid UTF-8 sequences that might have slipped through. - for (idx, _) in sql.char_indices() { - if !sql.is_char_boundary(idx) { - return Err(rustler::Error::Term(Box::new(format!( - "SQL contains invalid UTF-8 at byte position {}", - idx - )))); - } - } - Ok(()) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
SECURITY.mdnative/ecto_libsql/src/batch.rsnative/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/utils.rs
🧰 Additional context used
📓 Path-based instructions (5)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/batch.rsnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/utils.rs
native/ecto_libsql/src/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/*.rs: Lock Arc<Mutex> usingsafe_lock_arc(&client, "function_name client")?instead of.unwrap()to prevent panics
Handle Options using.ok_or_else()pattern with proper error messages instead of.unwrap()
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/batch.rsnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/utils.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/batch.rsnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/utils.rs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/batch.rsSECURITY.mdnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/utils.rs
**/*.{md,markdown}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British/Australian English for all documentation files (except SQL keywords and compatibility requirements)
Files:
SECURITY.md
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/batch.rsSECURITY.mdnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/batch.rsnative/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/batch.rsnative/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
native/ecto_libsql/src/batch.rsnative/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
native/ecto_libsql/src/batch.rsnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
native/ecto_libsql/src/batch.rsnative/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/batch.rsnative/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:18.238Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Use RANDOM ROWID in migration table definitions to prevent ID enumeration attacks and improve security
Applied to files:
SECURITY.md
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to **/*.rs : NEVER use `.unwrap()` in production Rust code—use `safe_lock` helpers and proper error handling instead (tests may use `.unwrap()` for simplicity)
Applied to files:
native/ecto_libsql/src/utils.rs
🧬 Code graph analysis (2)
native/ecto_libsql/src/cursor.rs (3)
native/ecto_libsql/src/utils.rs (1)
validate_utf8_sql(39-60)lib/ecto_libsql/native.ex (2)
def declare_cursor(_conn, _sql, _args), do: :erlang.nif_error(:nif_not_loaded)(119-119)def declare_cursor_with_context(_conn_id, _id, _id_type, _sql, _args),(103-104)native/ecto_libsql/src/lib.rs (1)
cursor(8-8)
native/ecto_libsql/src/batch.rs (1)
native/ecto_libsql/src/utils.rs (3)
safe_lock(65-72)safe_lock_arc(77-84)validate_utf8_sql(39-60)
🪛 LanguageTool
SECURITY.md
[uncategorized] ~22-~22: The preposition ‘to’ seems more likely in this position.
Context: ...create invalid UTF-8 in Rust would fail at compile time ##### 2. **Rustler Valida...
(AI_HYDRA_LEO_REPLACE_AT_TO)
⏰ 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). (3)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
native/ecto_libsql/src/utils.rs (1)
394-408: LGTM! Correct EXPLAIN detection logic.The implementation properly detects EXPLAIN statements with case-insensitive matching, boundary verification, and appropriate placement before the SELECT check. The logic correctly identifies that EXPLAIN queries return rows and should use the query path.
SECURITY.md (1)
1-112: Excellent security documentation with comprehensive mitigation strategy.The documentation clearly explains the CVE-2025-47736 vulnerability, the three-layer defence strategy, and provides comprehensive guidance for users. The structure is well-organised and includes all necessary information about the mitigation, upstream status, testing, and reporting procedures.
Minor note: Line 85 uses "parameterised queries" which should be "parameterised" in British/Australian English (though the current spelling is acceptable).
The static analysis hint about line 22 ("at compile time" vs "to compile time") is a false positive—"at compile time" is correct.
native/ecto_libsql/src/statement.rs (1)
31-32: LGTM! Optimal placement of UTF-8 validation.The UTF-8 validation is correctly placed as the first operation in the function, before any resource acquisition or locks. This provides early input validation and aligns with the defence-in-depth strategy documented in SECURITY.md.
native/ecto_libsql/src/batch.rs (1)
7-9: LGTM! Correct import and validation placement.The import of
validate_utf8_sqland its usage inexecute_batch_nativeare correctly implemented. The validation occurs before any resource acquisition, providing early input validation consistent with the defence-in-depth strategy.Also applies to: 206-207
Enhances parameter validation to handle edge cases and extends UTF-8 validation to all SQL entry points for complete CVE-2025-47736 protection. Parameter Validation Improvements (lib/ecto_libsql/native.ex): - Detect positional parameters (?) when map is provided - Raise clear error: "Cannot use named parameter map with SQL that has positional parameters (?). Use a list of values instead" - Filter out nil parameter names before validation - Prevents confusing error messages like ":, :" for positional params - Added test case for map-with-positional-SQL error UTF-8 Validation Extensions (native/ecto_libsql/src/): - Added validation to declare_cursor_with_context() in cursor.rs - Added validation to query_args() in query.rs - Added validation to pragma_query() in query.rs - Added validation to query_with_trx_args() in transaction.rs - Now covers ALL SQL entry points in the codebase Complete SQL Entry Point Coverage: ✅ prepare_statement() - statement.rs ✅ declare_cursor() - cursor.rs ✅ declare_cursor_with_context() - cursor.rs (NEW) ✅ execute_batch_native() - batch.rs ✅ query_args() - query.rs (NEW) ✅ pragma_query() - query.rs (NEW) ✅ query_with_trx_args() - transaction.rs (NEW) All functions now validate UTF-8 before passing SQL to libsql-sqlite3-parser, providing complete defence-in-depth against CVE-2025-47736. Test Results: ✅ All 23 named parameter tests pass ✅ New test for positional-param-with-map error ✅ No regressions
The check 'is_char_boundary(idx)' inside char_indices() iteration is redundant since char_indices() only yields valid UTF-8 character boundaries by definition. This check always passes and adds unnecessary runtime overhead. Simplify to use chars() iteration which still validates UTF-8 is iterable without the redundant boundary checks. Maintains defense-in-depth validation while eliminating wasted cycles.
Add missing import and proper Result type annotation to the doctest example. Mark as no_run since the example requires a full Rustler NIF environment to execute.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
native/ecto_libsql/src/utils.rs (1)
13-57: The UTF-8 validation is guaranteed to be redundant.Whilst the documentation acknowledges the redundancy, these checks literally cannot fail for
&str:
is_char_boundary(0)andis_char_boundary(sql.len())are always true for valid&strchars()iteration never fails for valid&str(guaranteed by Rust's type system)- Rustler validates UTF-8 during Elixir→Rust conversion, so invalid UTF-8 never reaches this code
This function will always return
Ok(()), making it runtime overhead without actual protection.Consider removing this function entirely if the goal is defence-in-depth documentation—a comment explaining the UTF-8 guarantees would suffice without the overhead. Alternatively, if you want to keep it for explicitness, consider marking it with
#[inline(always)]to ensure it's optimised away.Alternative: Remove the function or simplify to a no-op
If you want to keep explicit documentation without runtime overhead:
-#[inline] -pub fn validate_utf8_sql(sql: &str) -> Result<(), rustler::Error> { - // This check is technically redundant since &str is guaranteed to be valid UTF-8, - // but it serves as explicit documentation and defence-in-depth. - if !sql.is_char_boundary(0) || !sql.is_char_boundary(sql.len()) { - return Err(rustler::Error::Term(Box::new( - "SQL contains invalid UTF-8 sequences", - ))); - } - - // Additional validation: ensure the string can be iterated as UTF-8 chars. - // char_indices() only yields valid UTF-8 boundaries, so iteration itself - // serves as the validation without needing explicit is_char_boundary checks. - for _ in sql.chars() {} - - Ok(()) -} +/// Defence against CVE-2025-47736: Rust's `&str` guarantees valid UTF-8. +/// Rustler validates UTF-8 during Elixir→Rust conversion, providing protection. +#[inline(always)] +pub fn validate_utf8_sql(_sql: &str) -> Result<(), rustler::Error> { + // No-op: &str is guaranteed valid UTF-8 by Rust's type system + Ok(()) +}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
native/ecto_libsql/src/utils.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/utils.rs
native/ecto_libsql/src/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/*.rs: Lock Arc<Mutex> usingsafe_lock_arc(&client, "function_name client")?instead of.unwrap()to prevent panics
Handle Options using.ok_or_else()pattern with proper error messages instead of.unwrap()
Files:
native/ecto_libsql/src/utils.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/utils.rs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
native/ecto_libsql/src/utils.rs
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/connection.rs : Lock a registry using `safe_lock(®ISTRY, "function_name context")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/utils.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to **/*.rs : NEVER use `.unwrap()` in production Rust code—use `safe_lock` helpers and proper error handling instead (tests may use `.unwrap()` for simplicity)
Applied to files:
native/ecto_libsql/src/utils.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). (3)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
native/ecto_libsql/src/utils.rs (1)
391-405: LGTM! Correct EXPLAIN detection.The EXPLAIN keyword detection is correctly implemented:
- Uses the same zero-allocation byte scanning pattern as SELECT/WITH checks
- Properly validates word boundary (whitespace or end-of-string)
- Correctly routes EXPLAIN queries through the query() path since they return rows
The placement before SELECT/WITH checks is appropriate since EXPLAIN is independent.
The validate_utf8_sql function was guaranteed to always return Ok(()) because: - Rust's &str type guarantees valid UTF-8 at the type system level - Rustler validates UTF-8 during Elixir→Rust conversion - Any invalid UTF-8 would fail NIF conversion before reaching our code This function provided no actual protection against CVE-2025-47736, only runtime overhead. The type system itself is the defence, so explicit runtime validation is unnecessary. Removed the function and updated all call sites with a comment explaining the UTF-8 guarantees from the type system.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
native/ecto_libsql/src/query.rs (2)
41-42: Comment is accurate but potentially redundant.The comment correctly states that UTF-8 validation is guaranteed by Rust's
&strtype and Rustler's conversion from Elixir strings. However, this is fundamental Rust type system behaviour that experienced developers would already know. The comment might be helpful for onboarding developers less familiar with Rust, but could be considered optional documentation.
186-187: Comment is accurate but potentially redundant (consistent with query_args).This comment is identical to the one in
query_argsand the same assessment applies: whilst accurate, it documents fundamental Rust type system guarantees that may not require explicit documentation. The consistency between both functions is appropriate if these comments are retained.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
native/ecto_libsql/src/batch.rsnative/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rsnative/ecto_libsql/src/statement.rsnative/ecto_libsql/src/transaction.rsnative/ecto_libsql/src/utils.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- native/ecto_libsql/src/batch.rs
- native/ecto_libsql/src/statement.rs
- native/ecto_libsql/src/utils.rs
- native/ecto_libsql/src/transaction.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Always format Rust code before committing usingcargo fmt
NEVER use.unwrap()in production Rust code—usesafe_lockhelpers and proper error handling instead (tests may use.unwrap()for simplicity)
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
native/ecto_libsql/src/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/*.rs: Lock Arc<Mutex> usingsafe_lock_arc(&client, "function_name client")?instead of.unwrap()to prevent panics
Handle Options using.ok_or_else()pattern with proper error messages instead of.unwrap()
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
native/ecto_libsql/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
native/ecto_libsql/src/**/*.rs: Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Validate transaction ownership by checking thatentry.conn_id == conn_idbefore performing transaction operations
Add#[rustler::nif]attribute to NIF functions—modern Rustler auto-detects them without manual registration
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
When ending a work session, complete the mandatory workflow: file issues for remaining work, run quality gates (tests, linters, builds), update issue status, commit changes with
git commit, clean up stashes, and provide context for next session
Files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto/adapters/libsql/connection.ex : Add SQLite function support by defining expression handlers for native SQLite functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use prepared statements with automatic caching for repeated queries to achieve 10-15x performance improvement
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to priv/repo/migrations/*.exs : Create indexes on frequently queried columns and composite indexes for common multi-column queries to improve query performance
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use JSON helper functions from EctoLibSql.JSON module for safe JSON manipulation instead of string concatenation
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T06:36:18.238Z
Learning: Applies to **/*.{ex,exs} : Use named parameters (`:name`, `name`, or `$name` syntax) for clarity and maintainability in SQL queries with multiple parameters
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Add `#[rustler::nif]` attribute to NIF functions—modern Rustler auto-detects them without manual registration
Applied to files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to **/*.rs : Use British/Australian English for all Rust code comments and documentation (except SQL keywords and compatibility requirements)
Applied to files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to lib/ecto_libsql/native.ex : Provide NIF stubs and safe wrapper functions in the Native module for all Rust NIF functions
Applied to files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations
Applied to files:
native/ecto_libsql/src/cursor.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Lock Arc<Mutex<T>> using `safe_lock_arc(&client, "function_name client")?` instead of `.unwrap()` to prevent panics
Applied to files:
native/ecto_libsql/src/cursor.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/*.rs : Handle Options using `.ok_or_else()` pattern with proper error messages instead of `.unwrap()`
Applied to files:
native/ecto_libsql/src/cursor.rsnative/ecto_libsql/src/query.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Validate and document unsupported Ecto features by returning `:unsupported` atom errors in Rust implementations with clear documentation and comprehensive tests
Applied to files:
native/ecto_libsql/src/query.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/tests/**/*.rs : Tests may use `.unwrap()` for simplicity when handling test data and setup
Applied to files:
native/ecto_libsql/src/query.rs
📚 Learning: 2026-01-06T06:36:01.571Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.571Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Drop locks before async operations in Tokio runtime blocks to avoid holding locks across await points
Applied to files:
native/ecto_libsql/src/query.rs
🧬 Code graph analysis (1)
native/ecto_libsql/src/cursor.rs (2)
native/ecto_libsql/src/lib.rs (1)
cursor(8-8)native/ecto_libsql/src/decode.rs (1)
verify_cursor_ownership(59-66)
⏰ 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). (3)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
native/ecto_libsql/src/cursor.rs (2)
34-36: LGTM! Accurate documentation of UTF-8 safety guarantees.The comment correctly documents that UTF-8 validation is handled by Rust's type system and Rustler's conversion layer. This defence-in-depth approach is sound—invalid UTF-8 would be rejected during the Elixir-to-Rust boundary crossing before this function executes.
131-133: LGTM! Consistent documentation across cursor functions.The comment maintains consistency with
declare_cursorand accurately documents the UTF-8 validation guarantees. Both cursor declaration functions now have uniform documentation about relying on Rust's type system for UTF-8 safety.
…formatting - Remove redundant comments in query_args and pragma_query that document basic Rust type system guarantees - Fix rustfmt formatting (extra blank lines in utils.rs)
Summary by CodeRabbit
New Features
explain/2andexplain/3functions returning query execution plans as structured data.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.