Skip to content

Fix upsert ON CONFLICT: missing :identifier expr + numbered parameters #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AlanMcCann wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix upsert ON CONFLICT: missing :identifier expr + numbered parameters #95

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 32 additions & 10 deletions lib/ecto/adapters/libsql/connection.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,13 +754,14 @@ defmodule Ecto.Adapters.LibSql.Connection do

@impl true
def insert(prefix, table, header, rows, on_conflict, returning, placeholders) do
counter_offset = length(placeholders) + 1
fields = intersperse_map(header, ", ", &quote_name/1)

values =
if rows == [] do
[" DEFAULT VALUES"]
else
[" VALUES ", encode_values(rows)]
[" VALUES ", encode_insert_values(rows, counter_offset)]
end

[
Expand All @@ -776,12 +777,26 @@ defmodule Ecto.Adapters.LibSql.Connection do
]
end

defp encode_values(rows) do
rows
|> Enum.map(fn row ->
["(", intersperse_map(row, ", ", fn _ -> "?" end), ")"]
end)
|> Enum.intersperse(", ")
# Generate VALUES with numbered positional parameters (?1, ?2, ...).
# SQLite requires numbered parameters when the same statement contains
# multiple parameter groups (e.g., INSERT values + ON CONFLICT UPDATE).
# Bare `?` causes "near ?: syntax error" in upsert queries.
defp encode_insert_values(rows, counter) do
{encoded, _} =
Enum.map_reduce(rows, counter, fn row, acc ->
{params, new_acc} =
Enum.map_reduce(row, acc, fn
{:placeholder, placeholder_index}, c ->
{[?? | placeholder_index], c}

_, c ->
{[?? | Integer.to_string(c)], c + 1}
end)

{["(", Enum.intersperse(params, ", "), ")"], new_acc}
end)

Enum.intersperse(encoded, ", ")
end

# Helper for INSERT OR ... syntax (not used for now, keeping for SQLite REPLACE compatibility)
Expand Down Expand Up @@ -1166,9 +1181,11 @@ defmodule Ecto.Adapters.LibSql.Connection do
[?(, expr(expr, sources, query), ?)]
end

# Parameter placeholder
defp expr({:^, [], [_ix]}, _sources, _query) do
~c"?"
# Parameter placeholder - use numbered parameters (?1, ?2, ...).
# SQLite requires numbered parameters when a statement has multiple
# parameter groups (e.g., INSERT values + ON CONFLICT UPDATE).
defp expr({:^, [], [ix]}, _sources, _query) do
[?? | Integer.to_string(ix + 1)]
end
Comment on lines +1184 to 1189
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Finish the numbered-placeholder pass for IN ^list.

This change only renumbers the generic {:^, ...} case. The dedicated {:in, _, [left, {:^, _, [_, length]}]} branch below still expands to bare ? placeholders, so queries that use IN ^list can still hit SQLite's numbered-parameter requirement.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/ecto/adapters/libsql/connection.ex` around lines 1184 - 1189, The IN with
a bound list branch still emits bare "?" placeholders; update the clause
handling the pattern {:in, _, [left, {:^, _, [_, length]}]} inside expr/3 to
emit numbered placeholders like the generic {:^, [], [ix]} clause does: generate
a sequence of "?" + Integer.to_string(start_index) through "?" +
Integer.to_string(start_index + length - 1) (matching the same numbering scheme
used by expr({:^,[],[ix]},...)), join them with ", " and return the full
parenthesized placeholder list so SQLite gets ?1, ?2, ... for IN ^list. Ensure
you reference and reuse the same index base/offset logic used by expr({:^,...})
so numbering remains consistent across other parameter groups.


# Qualified field reference: s0.field
Expand Down Expand Up @@ -1306,6 +1323,11 @@ defmodule Ecto.Adapters.LibSql.Connection do
["max(", expr(arg, sources, query), ?)]
end

# Identifier expression (used in fragment expressions like EXCLUDED."column_name")
defp expr({:identifier, _, [name]}, _sources, _query) do
quote_name(name)
end

# Fragment for raw SQL
defp expr({:fragment, _, parts}, sources, query) do
Enum.map(parts, fn
Expand Down