Skip to content

Commit

Permalink
fix: Implement and support cell-wise placeholder support (#154)
Browse files Browse the repository at this point in the history
I went ahead and added in support for cell-wise support for bounded
values. This has been on the todo list for a while.

fixes: #152
  • Loading branch information
warmwaffles authored Oct 28, 2024
1 parent b39bcba commit 3d69b84
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 20 deletions.
31 changes: 18 additions & 13 deletions lib/ecto/adapters/sqlite3/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -267,17 +267,21 @@ defmodule Ecto.Adapters.SQLite3.Connection do
]
end

def insert(prefix, table, header, rows, on_conflict, returning, _placeholders) do
fields = quote_names(header)
def insert(prefix, table, header, rows, on_conflict, returning, placeholders) do
counter_offset = length(placeholders) + 1

values =
if header == [] do
[" VALUES " | Enum.map_intersperse(rows, ?,, fn _ -> "(DEFAULT)" end)]
else
[" (", quote_names(header), ") " | insert_all(rows, counter_offset)]
end

[
"INSERT INTO ",
quote_table(prefix, table),
insert_as(on_conflict),
" (",
fields,
") ",
insert_all(rows, on_conflict),
values,
on_conflict(on_conflict, header),
returning(returning)
]
Expand Down Expand Up @@ -766,13 +770,13 @@ defmodule Ecto.Adapters.SQLite3.Connection do
]
end

def insert_all(rows, on_conflict), do: insert_all(rows, on_conflict, 1)
def insert_all(rows), do: insert_all(rows, 1)

def insert_all(%Ecto.Query{} = query, _on_conflict, _counter) do
def insert_all(%Ecto.Query{} = query, _counter) do
[all(query)]
end

def insert_all(rows, _on_conflict, counter) do
def insert_all(rows, counter) do
[
"VALUES ",
intersperse_reduce(
Expand All @@ -797,11 +801,12 @@ defmodule Ecto.Adapters.SQLite3.Connection do
{%Ecto.Query{} = query, params_counter}, counter ->
{[?(, all(query), ?)], counter + params_counter}

{:placeholder, placeholder_index}, counter ->
{[?? | placeholder_index], counter}

_, counter ->
# TODO: Should we have cell wise value support?
# Essentially ``?1 ?2 ?3`` instead of ``? ? ?``
# {['?' | Integer.to_string(counter)], counter + 1}
{[~c"?"], counter + 1}
# Cell wise value support ex: (?1, ?2, ?3)
{[?? | Integer.to_string(counter)], counter + 1}
end)
end

Expand Down
14 changes: 7 additions & 7 deletions test/ecto/adapters/sqlite3/connection/insert_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ defmodule Ecto.Adapters.SQLite3.Connection.InsertTest do

test "insert" do
query = insert(nil, "schema", [:x, :y], [[:x, :y]], {:raise, [], []}, [:id])
assert query == ~s{INSERT INTO "schema" ("x","y") VALUES (?,?) RETURNING "id"}
assert query == ~s{INSERT INTO "schema" ("x","y") VALUES (?1,?2) RETURNING "id"}

assert_raise ArgumentError, fn ->
insert(nil, "schema", [:x, :y], [[:x, :y], [nil, :z]], {:raise, [], []}, [:id])
Expand All @@ -30,7 +30,7 @@ defmodule Ecto.Adapters.SQLite3.Connection.InsertTest do
end

query = insert(nil, "schema", [:x, :y], [[:x, :y]], {:raise, [], []}, [:id])
assert query == ~s{INSERT INTO "schema" ("x","y") VALUES (?,?) RETURNING "id"}
assert query == ~s{INSERT INTO "schema" ("x","y") VALUES (?1,?2) RETURNING "id"}

assert_raise(
ArgumentError,
Expand All @@ -46,19 +46,19 @@ defmodule Ecto.Adapters.SQLite3.Connection.InsertTest do
query = insert(nil, "schema", [:x, :y], [[:x, :y]], {:nothing, [], []}, [])

assert query ==
~s{INSERT INTO "schema" ("x","y") VALUES (?,?) ON CONFLICT DO NOTHING}
~s{INSERT INTO "schema" ("x","y") VALUES (?1,?2) ON CONFLICT DO NOTHING}

query = insert(nil, "schema", [:x, :y], [[:x, :y]], {:nothing, [], [:x, :y]}, [])

assert query ==
~s{INSERT INTO "schema" ("x","y") VALUES (?,?) ON CONFLICT ("x","y") DO NOTHING}
~s{INSERT INTO "schema" ("x","y") VALUES (?1,?2) ON CONFLICT ("x","y") DO NOTHING}

# For :update
update = from("schema", update: [set: [z: "foo"]]) |> plan(:update_all)
query = insert(nil, "schema", [:x, :y], [[:x, :y]], {update, [], [:x, :y]}, [:z])

assert query ==
~s{INSERT INTO "schema" AS s0 ("x","y") VALUES (?,?) ON CONFLICT ("x","y") DO UPDATE SET "z" = 'foo' RETURNING "z"}
~s{INSERT INTO "schema" AS s0 ("x","y") VALUES (?1,?2) ON CONFLICT ("x","y") DO UPDATE SET "z" = 'foo' RETURNING "z"}

# For :unsafe_fragment
update = from("schema", update: [set: [z: "foo"]]) |> plan(:update_all)
Expand All @@ -74,7 +74,7 @@ defmodule Ecto.Adapters.SQLite3.Connection.InsertTest do
)

assert query ==
~s{INSERT INTO "schema" AS s0 ("x","y") VALUES (?,?) ON CONFLICT foobar DO UPDATE SET "z" = 'foo' RETURNING "z"}
~s{INSERT INTO "schema" AS s0 ("x","y") VALUES (?1,?2) ON CONFLICT foobar DO UPDATE SET "z" = 'foo' RETURNING "z"}

assert_raise ArgumentError, "Upsert in SQLite3 requires :conflict_target", fn ->
conflict_target = []
Expand Down Expand Up @@ -107,7 +107,7 @@ defmodule Ecto.Adapters.SQLite3.Connection.InsertTest do
assert query ==
"""
INSERT INTO "schema" ("x","y") \
VALUES (?,?) \
VALUES (?1,?2) \
ON CONFLICT ("id") \
DO UPDATE SET "x" = EXCLUDED."x","y" = EXCLUDED."y"\
"""
Expand Down

0 comments on commit 3d69b84

Please sign in to comment.