fix: oban lifeline plugin errors #57

nadilas · 2026-01-09T20:55:10Z

I'm not well versed enough in elixir to make this a proper PR, so if you feel like it, I'm happy to learn how to make this proper.
In case you don't want it, feel free to throw it away.

I added Oban to a small project and ran into a few issues:

Ecto migrations:

** (CaseClauseError) no case clause matching:
{:ok, %EctoLibSql.Result{command: :insert, columns: nil, rows: [], num_rows: 1}}

Datetime handling:

(EctoLibSql.Error) Unsupported argument type: #{microsecond=>{...},second=>35,
calendar=>'Elixir.Calendar.ISO',month=>1,'__struct__'=>'Elixir.DateTime',...}

And Oban's Lifeline plugin:

protocol Enumerable not implemented for Atom. Got value: nil

As best I could understand:

columns was set to nil for write commands, but Ecto always expects a list
rows was nil for UPDATE...RETURNING with 0 matching rows, but Ecto expects a list

Wonderful AI quick fixes were, (sorry if they're trash 😅):

encode/3: Convert DateTime/NaiveDateTime/Date/Time/Decimal to ISO8601/string (NIF can't serialize these)
decode/3: columns is always [], rows is nil for successful writes without RETURNING, otherwise []
do_query/7 and do_execute_with_trx/6: columns is always [], rows is nil for successful writes without RETURNING, otherwise []

Summary by CodeRabbit

Release Notes

Bug Fixes
- Improved handling of query results for INSERT, UPDATE, and DELETE operations to ensure consistent result structures.
- Enhanced parameter encoding for date and time types to ensure proper SQLite compatibility.
- Fixed result normalisation to reliably process query outcomes across different operation types.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-09T20:55:20Z

Walkthrough

Query result normalisation is adjusted across transactional and non-transactional paths to ensure consistent column and row handling. Parameter encoding is enhanced to transform Elixir types into SQLite-compatible representations, whilst result decoding incorporates normalisation logic for improved ecto_sql compatibility.

Changes

Cohort / File(s)	Summary
Query Result Normalisation `lib/ecto_libsql/native.ex`	Normalises columns to list, adjusts rows handling for INSERT/UPDATE/DELETE operations without RETURNING clause, and updates result construction to use actual_num_rows when available.
Parameter Encoding and Result Decoding `lib/ecto_libsql/query.ex`	Transforms parameter encoding to apply per-parameter type conversion (DateTime, NaiveDateTime, Date, Time, Decimal to SQLite representations) and introduces result normalisation logic with special handling for command-specific row behaviour.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Through SQLite's garden where queries now hop,
With parameters encoded from top to proper stop,
Columns normalised, rows kept just right,
Each result shaped perfectly—oh, what a sight! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title 'fix: oban lifeline plugin errors' does not match the actual changes, which address fundamental data normalization issues (columns/rows handling) and parameter encoding across multiple modules to fix Ecto compatibility.	Revise the title to reflect the core changes, e.g. 'fix: normalize query result columns and rows, encode temporal types for sqlite compatibility' or similar to accurately describe the scope of the changes.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

lib/ecto_libsql/native.ex (1)
800-815: Fix execute_with_transaction to return rows: [] for 0-row writes, not rows: nil.

The execute path inconsistently returns rows: nil for all INSERT/UPDATE/DELETE operations without RETURNING, even when num_rows == 0. The documented design rule (in Query.decode, line 56) specifies that rows should be nil only for write commands "that affected rows" (i.e., when num_rows > 0). The query path correctly normalises rows: nil to rows: [] for 0-row writes; the execute path should do the same.
Proposed fix
     else
       # Use execute_with_transaction for INSERT/UPDATE/DELETE without RETURNING
-      # columns MUST be [] (not nil), rows should be nil
+      # columns MUST be [] (not nil); rows should be nil only when rows were affected
       case execute_with_transaction(trx_id, conn_id, statement, args_for_execution) do
         num_rows when is_integer(num_rows) ->
           result = %EctoLibSql.Result{
             command: command,
             columns: [],
-            rows: nil,
+            rows: if(num_rows > 0, do: nil, else: []),
             num_rows: num_rows
           }

🤖 Fix all issues with AI agents

In @lib/ecto_libsql/query.ex:
- Around line 53-83: The decode/3 function can let non-list :rows pass through
and later crash; update decode/3 (the clauses handling columns, cmd, rows,
num_rows) to coerce rows to a list when it's non-nil and not a list (e.g. treat
any non-list rows as [rows] or convert via List.wrap semantics) before applying
the existing cond that sets nil for write commands with no RETURNING, and also
change the comment header text from "Normalize results" to British/Australian
spelling "Normalise results" to match guidelines.
- Around line 41-44: The encode/3 clause currently assumes params is a list and
calls Enum.map, which crashes for nil or non-list params; add a guarded clause
so def encode(_query, params, _opts) when is_list(params) do Enum.map(params,
&encode_param/1) end and provide a fallback def encode(_query, params, _opts)
when not is_list(params) do params end (or otherwise return the unmodified
params) to avoid raising before the NIF; reference the encode/3 function and
encode_param/1 helper.

🧹 Nitpick comments (2)

lib/ecto_libsql/native.ex (2)

707-718: Normalisation logic looks right, but fix en-AU spelling (“Normalise”).
This block aligns with the goal (columns always list; rows nil only for successful writes without RETURNING). The only change I’d request here is comment spelling per guidelines.

774-785: Same: behaviour is good; consider extracting a shared normaliser.
The query-path normalisation matches the non-transactional path; to avoid future drift, consider a small private helper (even just within this module) used by both blocks.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8c2486 and a254e09.

📒 Files selected for processing (2)

lib/ecto_libsql/native.ex
lib/ecto_libsql/query.ex

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{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 using mix format --check-formatted

Use camelCase for function names in Elixir (e.g., defmodule, def handle_execute)

Files:

lib/ecto_libsql/native.ex
lib/ecto_libsql/query.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

**/*.{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/native.ex
lib/ecto_libsql/query.ex

**/*.ex

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ex: Use camelCase for variable names in Elixir modules
All database connection errors and query failures must be handled gracefully with pattern matching on {:ok, ...} and {:error, ...} tuples - never use bare unwrap operations
Use prepared statements with automatic statement caching for repeated queries to achieve ~10-15x performance improvement
Use batch operations (batch_transactional or execute_batch_sql) for bulk inserts/updates instead of individual queries
Named parameters (:name, @name, $name syntax) are preferred over positional parameters (?) for readability and maintainability in complex queries
Use savepoints for partial rollback within transactions to enable error recovery patterns in batch operations
Use cursor streaming for large result sets to avoid loading entire datasets into memory
JSON functions (extract, type, set, replace, insert, remove, patch) should be used for efficient JSON/JSONB manipulation in LibSQL 3.45.1+
Enable foreign key constraints with EctoLibSql.Pragma.enable_foreign_keys/1 in production environments
Use WAL (Write-Ahead Logging) mode for better concurrency in production: EctoLibSql.Pragma.set_journal_mode(state, :wal)

Files:

lib/ecto_libsql/native.ex
lib/ecto_libsql/query.ex

🧠 Learnings (7)

📓 Common learnings

Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
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.585Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Validate transaction ownership by checking that `entry.conn_id == conn_id` before performing transaction operations

Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T06:36:01.585Z
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.585Z
Learning: Applies to lib/ecto/adapters/libsql.ex : Implement type loaders for converting SQLite values to Ecto types using `loaders/2` function

Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-08T04:56:27.393Z
Learning: Applies to priv/repo/migrations/*.exs : Use STRICT tables in migrations for type safety when database schema stability is critical