Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 25 additions & 14 deletions lib/ecto_libsql/native.ex
Original file line number Diff line number Diff line change
Expand Up @@ -704,13 +704,17 @@ defmodule EctoLibSql.Native do
num_rows
end

# For INSERT/UPDATE/DELETE without RETURNING, columns and rows will be empty
# Set them to nil to match Ecto's expectations
{columns, rows} =
if command in [:insert, :update, :delete] and columns == [] and rows == [] do
{nil, nil}
else
{columns, rows}
# Normalize: columns MUST be a list, rows nil only for successful writes without RETURNING
columns = columns || []

rows =
cond do
command in [:insert, :update, :delete] and columns == [] and rows == [] and actual_num_rows > 0 ->
nil
rows == nil ->
[]
true ->
rows
end

result = %EctoLibSql.Result{
Expand Down Expand Up @@ -767,13 +771,17 @@ defmodule EctoLibSql.Native do
"rows" => rows,
"num_rows" => num_rows
} ->
# For INSERT/UPDATE/DELETE without actual returned rows, normalise empty lists to nil
# This ensures consistency with non-transactional path
{columns, rows} =
if command in [:insert, :update, :delete] and columns == [] and rows == [] do
{nil, nil}
else
{columns, rows}
# Normalize: columns MUST be a list, rows nil only for successful writes without RETURNING
columns = columns || []

rows =
cond do
command in [:insert, :update, :delete] and columns == [] and rows == [] and num_rows > 0 ->
nil
rows == nil ->
[]
true ->
rows
end

result = %EctoLibSql.Result{
Expand All @@ -790,10 +798,13 @@ defmodule EctoLibSql.Native do
end
else
# Use execute_with_transaction for INSERT/UPDATE/DELETE without RETURNING
# columns MUST be [] (not nil), rows should be nil
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,
num_rows: num_rows
}

Expand Down
44 changes: 43 additions & 1 deletion lib/ecto_libsql/query.ex
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,49 @@ defmodule EctoLibSql.Query do

def describe(query, _opts), do: query

def encode(_query, params, _opts), do: params
# Convert Elixir types to SQLite-compatible values before sending to NIF
def encode(_query, params, _opts) do
Enum.map(params, &encode_param/1)
end
Comment on lines +41 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the file structure and locate the query.ex file
find . -name "query.ex" -type f

Repository: ocean/ecto_libsql

Length of output: 87


🏁 Script executed:

# Let's read the file to see the encode function and surrounding context
cat -n lib/ecto_libsql/query.ex | head -100

Repository: ocean/ecto_libsql

Length of output: 3588


🏁 Script executed:

# Let's also check if there are any tests or documentation about the encode function
rg "def encode" -A 5 lib/

Repository: ocean/ecto_libsql

Length of output: 448


🏁 Script executed:

# Search for DBConnection documentation or any comments about the encode contract
rg "DBConnection\|encode.*params" lib/ecto_libsql/ -B 2 -A 2

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Check if there are any other database adapters or examples that show how params are handled
rg "Enum\.map.*params" lib/ -B 3 -A 3

Repository: ocean/ecto_libsql

Length of output: 1327


🏁 Script executed:

# Search for DBConnection.Query protocol definition or documentation
rg "DBConnection\.Query" lib/ --type elixir -B 2 -A 5

Repository: ocean/ecto_libsql

Length of output: 897


🏁 Script executed:

# Look for test files that might show how encode is called
find . -name "*test*.exs" -type f | head -5

Repository: ocean/ecto_libsql

Length of output: 210


🏁 Script executed:

# Check if there's any documentation about DBConnection contract
rg "encode.*_query.*params" . --type elixir -B 3 -A 3 | head -50

Repository: ocean/ecto_libsql

Length of output: 459


🏁 Script executed:

# Search for how params are passed to encode in this adapter
rg "DBConnection\." lib/ecto_libsql/ --type elixir -B 2 -A 2

Repository: ocean/ecto_libsql

Length of output: 990


🏁 Script executed:

# Look for the defimpl DBConnection.Query usage and see if there are type specs
ast-grep --pattern 'defimpl DBConnection.Query do
  $$$
end'

Repository: ocean/ecto_libsql

Length of output: 3323


🌐 Web query:

DBConnection.Query encode/3 callback signature params contract

💡 Result:

Signature/contract:

  • Callback: encode(query, params, opts) :: any(). This is called before execute to encode the params for the query. If it returns encoded params, those are used; if it raises DBConnection.EncodeError the query will be prepared again. [1][2][3]

Notes / common expectations:

  • Types are left as "any()" by DBConnection — the concrete encoded shape is adapter-specific (e.g. Postgres/Postgrex or MyXQL encode routines). Implementations typically return the params transformed into the adapter's expected format (or raise DBConnection.EncodeError on invalid input). [2][3]

Sources:

  1. DBConnection.Query (encode/3) docs. [hexdocs.pm/db_connection/DBConnection.Query.html]. [1]
  2. DBConnection EncodeError and callback discussion (behaviour examples). [hexdocs.pm/db_connection/DBConnection.EncodeError.html; blog/example]. [2]
  3. Adapter examples (Postgrex/MyXQL) showing encode usage/expectations. [postgrex docs; MyXQL example]. [3]

🌐 Web query:

Elixir DBConnection.Query protocol documentation encode

💡 Result:

The DBConnection.Query protocol defines encode/3 as:

  • Spec: encode(query, params, opts) :: any().
  • Purpose: encode parameters for a query before execution. If encode/3 raises DBConnection.EncodeError the query will be prepared again.
  • Related functions in the protocol: parse/2, describe/2, decode/3.

Reference documentation: DBConnection.Query (hexdocs). [1]
(DBConnection shows encode is invoked when preparing/executing and handles EncodeError by re-preparing.) [2]

Sources:
[1] DBConnection.Query — db_connection (protocol docs).
[2] DBConnection implementation showing encode usage and EncodeError handling (lib/db_connection.ex).


🏁 Script executed:

# Check if there are any guards used elsewhere in encode implementations
rg "def encode.*when" lib/

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Look for how params are used in the codebase - is it always passed as a list?
rg "encode\(" lib/ --type elixir -B 2 -A 2 | head -40

Repository: ocean/ecto_libsql

Length of output: 1395


🏁 Script executed:

# Check test files to see how encode is called
head -100 test/ecto_adapter_test.exs

Repository: ocean/ecto_libsql

Length of output: 3127


🏁 Script executed:

# Check how params flow through the system - search for places where encode is called or params are prepared
rg "execute\|prepare" lib/ecto_libsql/ --type elixir -B 2 -A 2 | head -60

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Look for DBConnection usage patterns to understand param handling
rg "DBConnection\." lib/ecto_libsql/connection.ex --type elixir -B 3 -A 3 2>/dev/null | head -80

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Check what Ecto adapters typically do - look at existing implementations
rg "def execute" lib/ --type elixir -B 2 -A 5 | head -60

Repository: ocean/ecto_libsql

Length of output: 3014


🏁 Script executed:

# Check the main connection execution flow to see how params reach encode
cat -n lib/ecto_libsql.ex | head -150

Repository: ocean/ecto_libsql

Length of output: 6261


🏁 Script executed:

# Search for where encode is invoked via DBConnection
rg "cast_params\|prepared\|DBConnection\." lib/ecto_libsql.ex --type elixir -B 2 -A 3 | head -80

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Look at similar Ecto adapters to understand if params guard is common practice
web_search: "Ecto adapter SQLite encode params guard DBConnection"

Repository: ocean/ecto_libsql

Length of output: 112


🏁 Script executed:

# Check if there are any existing nil or non-list handling patterns in the codebase
rg "is_list" lib/ --type elixir -B 1 -A 1

Repository: ocean/ecto_libsql

Length of output: 4534


🏁 Script executed:

# Examine how query execution happens - trace the flow from Ecto to encode
cat -n lib/ecto_libsql.ex | grep -A 20 "def execute"

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Let me trace through handle_execute to see how params flow to encode
cat -n lib/ecto_libsql.ex | sed -n '148,200p'

Repository: ocean/ecto_libsql

Length of output: 2210


🏁 Script executed:

# Check if DBConnection.Query protocol is documented in the repo or if there are type specs
rg "@spec encode" lib/ --type elixir -B 2 -A 2

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Check the complete flow - does DBConnection guarantee args is a list before calling encode?
rg "handle_execute\|def encode" lib/ecto_libsql.ex --type elixir -B 1 -A 10 | head -80

Repository: ocean/ecto_libsql

Length of output: 43


🏁 Script executed:

# Verify: are there any error cases or edge paths where encode could receive non-list?
rg "encode\(" lib/ecto_libsql.ex --type elixir -B 3 -A 3

Repository: ocean/ecto_libsql

Length of output: 43


🌐 Web query:

DBConnection Elixir encode callback receives list params or generic?

💡 Result:

Short answer: generic — the encode callback takes params() :: any(), not specifically a list. DBConnection passes whatever your adapter/query uses (lists, maps, tuples, etc.), and the DBConnection.Query.encode implementation for your adapter defines how they’re turned into iodata. [1][2]


🏁 Script executed:

# Final verification: confirm that the spelling issue exists at line 53
sed -n '53p' lib/ecto_libsql/query.ex

Repository: ocean/ecto_libsql

Length of output: 112


🏁 Script executed:

# Verify the proposed fix aligns with codebase patterns by checking similar encode-like functions
rg "def.*(_.*params.*_opts)" lib/ --type elixir -A 2 | head -40

Repository: ocean/ecto_libsql

Length of output: 224


Add guard to encode/3 to prevent crashes on non-list params.

The DBConnection.Query protocol specifies encode/3 receives params as any() type, not guaranteed to be a list. Enum.map/2 will raise if params is nil or a non-list, causing failures before the NIF layer. The codebase already uses is_list() guards elsewhere (e.g., normalise_args_for_query/2), confirming this edge case was anticipated.

Proposed fix
-    def encode(_query, params, _opts) do
-      Enum.map(params, &encode_param/1)
-    end
+    def encode(_query, params, _opts) when is_list(params) do
+      Enum.map(params, &encode_param/1)
+    end
+
+    def encode(_query, params, _opts), do: params
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Convert Elixir types to SQLite-compatible values before sending to NIF
def encode(_query, params, _opts) do
Enum.map(params, &encode_param/1)
end
# Convert Elixir types to SQLite-compatible values before sending to NIF
def encode(_query, params, _opts) when is_list(params) do
Enum.map(params, &encode_param/1)
end
def encode(_query, params, _opts), do: params
🤖 Prompt for AI Agents
In @lib/ecto_libsql/query.ex around lines 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.


defp encode_param(%DateTime{} = dt), do: DateTime.to_iso8601(dt)
defp encode_param(%NaiveDateTime{} = dt), do: NaiveDateTime.to_iso8601(dt)
defp encode_param(%Date{} = d), do: Date.to_iso8601(d)
defp encode_param(%Time{} = t), do: Time.to_iso8601(t)
defp encode_param(%Decimal{} = d), do: Decimal.to_string(d)
defp encode_param(value), do: value

# Normalize results for ecto_sql compatibility.
# Rules:
# 1. columns MUST ALWAYS be a list (even empty []), NEVER nil
# 2. rows should be nil only for write commands without RETURNING that affected rows
# 3. For all other cases (SELECT, RETURNING queries), rows must be a list
def decode(_query, result, _opts) when is_map(result) do
columns = case Map.get(result, :columns) do
nil -> []
cols when is_list(cols) -> cols
_ -> []
end

cmd = Map.get(result, :command)
rows = Map.get(result, :rows)
num_rows = Map.get(result, :num_rows, 0)

rows = cond do
# Write commands that affected rows but have no RETURNING -> rows should be nil
cmd in [:insert, :update, :delete] and rows == [] and num_rows > 0 and columns == [] ->
nil
# All other cases: rows must be a list
rows == nil ->
[]
true ->
rows
end

result
|> Map.put(:columns, columns)
|> Map.put(:rows, rows)
end
Comment on lines +53 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tighten decode/3 row normalisation + fix en-AU spelling in comments.
Two points:

  1. If :rows comes back as a non-list (unexpected but possible given NIF/map boundaries), it currently passes through and may explode later when enumerated.
  2. Comments use “Normalize”; guidelines ask for British/Australian English (“Normalise”).
Proposed fix
-    # Normalize results for ecto_sql compatibility.
+    # Normalise results for ecto_sql compatibility.
     # Rules:
     # 1. columns MUST ALWAYS be a list (even empty []), NEVER nil
     # 2. rows should be nil only for write commands without RETURNING that affected rows
     # 3. For all other cases (SELECT, RETURNING queries), rows must be a list
     def decode(_query, result, _opts) when is_map(result) do
       columns = case Map.get(result, :columns) do
         nil -> []
         cols when is_list(cols) -> cols
         _ -> []
       end
 
       cmd = Map.get(result, :command)
       rows = Map.get(result, :rows)
       num_rows = Map.get(result, :num_rows, 0)
 
       rows = cond do
         # Write commands that affected rows but have no RETURNING -> rows should be nil
         cmd in [:insert, :update, :delete] and rows == [] and num_rows > 0 and columns == [] ->
           nil
         # All other cases: rows must be a list
-        rows == nil ->
+        rows == nil ->
           []
+        not is_list(rows) ->
+          []
         true ->
           rows
       end
🤖 Prompt for AI Agents
In @lib/ecto_libsql/query.ex around lines 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.


def decode(_query, result, _opts), do: result
end
Expand Down
Loading