Skip to content
Merged
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ project adheres to [Semantic Versioning][semver].

## Unreleased

- changed: Reimplement querying prefix names.
- added: Raise on cross-database foreign keys.
- changed: Made `quote_name/1` private.
- changed: quote_name no longer accepts names with double quote.

## v0.25.0
- fixed: Precedence issues with SQL generation. See [#182](https://github.com/elixir-sqlite/ecto_sqlite3/pull/182)

Expand Down
21 changes: 21 additions & 0 deletions integration_test/prefix_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
defmodule Ecto.Integration.PrefixTest do
use Ecto.Integration.Case, async: true

import Ecto.Query, only: [from: 2]

alias Ecto.Integration.Post
alias Ecto.Integration.TestRepo

test "queries an attached database through :prefix" do
TestRepo.query!(~s|ATTACH DATABASE ":memory:" AS "foo"|)
TestRepo.query!(~s|CREATE TABLE "foo"."posts" AS SELECT * FROM "main"."posts"|)

TestRepo.insert!(%Post{id: 1}, prefix: "foo")

results =
from(Post, prefix: "foo")
|> TestRepo.all()

assert [%Post{id: 1}] = results
end
end
5 changes: 4 additions & 1 deletion integration_test/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,11 @@ excludes = [
# which is not true for SQLite
:lock_for_migrations,

# Migration we don't support
# sadly we can not run prefix tests since ecto_sql's integration test expects
# attached db to persist across connections
:prefix,

# Migration we don't support
:add_column_if_not_exists,
:remove_column_if_exists,
:alter_primary_key,
Expand Down
101 changes: 68 additions & 33 deletions lib/ecto/adapters/sqlite3/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
def insert(prefix, table, [], [[]], on_conflict, returning, [], _opts) do
[
"INSERT INTO ",
quote_table(prefix, table),
quote_name(prefix, table),
insert_as(on_conflict),
" DEFAULT VALUES",
returning(returning)
Expand All @@ -290,7 +290,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do

[
"INSERT INTO ",
quote_table(prefix, table),
quote_name(prefix, table),
insert_as(on_conflict),
values,
on_conflict(on_conflict, header),
Expand All @@ -313,7 +313,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do

[
"UPDATE ",
quote_table(prefix, table),
quote_name(prefix, table),
" SET ",
fields,
" WHERE ",
Expand All @@ -335,7 +335,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do

[
"DELETE FROM ",
quote_table(prefix, table),
quote_name(prefix, table),
" WHERE ",
filters,
returning(returning)
Expand Down Expand Up @@ -402,7 +402,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"CREATE TABLE ",
quote_table(table.prefix, table.name),
quote_name(table.prefix, table.name),
?\s,
?(,
column_definitions(table, columns),
Expand All @@ -421,7 +421,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"CREATE TABLE IF NOT EXISTS ",
quote_table(table.prefix, table.name),
quote_name(table.prefix, table.name),
?\s,
?(,
column_definitions(table, columns),
Expand All @@ -437,7 +437,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"DROP TABLE ",
quote_table(table.prefix, table.name)
quote_name(table.prefix, table.name)
]
]
end
Expand All @@ -450,7 +450,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"DROP TABLE IF EXISTS ",
quote_table(table.prefix, table.name)
quote_name(table.prefix, table.name)
]
]
end
Expand All @@ -463,7 +463,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
Enum.map(changes, fn change ->
[
"ALTER TABLE ",
quote_table(table.prefix, table.name),
quote_name(table.prefix, table.name),
?\s,
column_change(table, change)
]
Expand Down Expand Up @@ -498,9 +498,9 @@ defmodule Ecto.Adapters.SQLite3.Connection do
"CREATE ",
if_do(index.unique, "UNIQUE "),
"INDEX ",
quote_name(index.name),
quote_name(index.prefix, index.name),
" ON ",
quote_table(index.prefix, index.table),
quote_name(index.table),
" (",
fields,
?),
Expand All @@ -517,9 +517,9 @@ defmodule Ecto.Adapters.SQLite3.Connection do
"CREATE ",
if_do(index.unique, "UNIQUE "),
"INDEX IF NOT EXISTS ",
quote_name(index.name),
quote_name(index.prefix, index.name),
" ON ",
quote_table(index.prefix, index.table),
quote_name(index.table),
" (",
fields,
?),
Expand All @@ -532,7 +532,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"DROP INDEX ",
quote_table(index.prefix, index.name)
quote_name(index.prefix, index.name)
]
]
end
Expand All @@ -545,7 +545,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"DROP INDEX IF EXISTS ",
quote_table(index.prefix, index.name)
quote_name(index.prefix, index.name)
]
]
end
Expand All @@ -558,9 +558,9 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"ALTER TABLE ",
quote_table(current_table.prefix, current_table.name),
quote_name(current_table.prefix, current_table.name),
" RENAME TO ",
quote_table(nil, new_table.name)
quote_name(nil, new_table.name)
]
]
end
Expand All @@ -569,7 +569,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
[
[
"ALTER TABLE ",
quote_table(table.prefix, table.name),
quote_name(table.prefix, table.name),
" RENAME COLUMN ",
quote_name(current_column),
" TO ",
Expand Down Expand Up @@ -1539,7 +1539,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do

{table, schema, prefix} ->
name = as_prefix ++ [create_alias(table) | Integer.to_string(pos)]
{quote_table(prefix, table), name, schema}
{quote_name(prefix, table), name, schema}

%Ecto.SubQuery{} ->
{nil, as_prefix ++ [?s | Integer.to_string(pos)], nil}
Expand Down Expand Up @@ -1658,7 +1658,7 @@ defmodule Ecto.Adapters.SQLite3.Connection do
defp check_expr(nil), do: []

defp check_expr(%{name: name, expr: expr}),
do: [" CONSTRAINT ", name, " CHECK (", expr, ")"]
do: [" CONSTRAINT ", quote_name(name), " CHECK (", expr, ")"]

defp collate_expr(nil), do: []

Expand Down Expand Up @@ -1717,11 +1717,13 @@ defmodule Ecto.Adapters.SQLite3.Connection do
defp reference_expr(%Reference{with: [_]}, _table, _name), do: []

defp reference_expr(%Reference{} = ref, table, name) do
assert_same_database(table.prefix, ref.prefix)

[
" CONSTRAINT ",
reference_name(ref, table, name),
" REFERENCES ",
quote_table(ref.prefix || table.prefix, ref.table),
quote_name(ref.table),
?(,
quote_name(ref.column),
?),
Expand Down Expand Up @@ -1826,13 +1828,15 @@ defmodule Ecto.Adapters.SQLite3.Connection do
end

defp composite_fk_definition(table, {_op, name, ref, _opts}) do
assert_same_database(table.prefix, ref.prefix)

{current_columns, reference_columns} = Enum.unzip([{name, ref.column} | ref.with])

[
", FOREIGN KEY (",
quote_names(current_columns),
") REFERENCES ",
quote_table(ref.prefix || table.prefix, ref.table),
quote_name(ref.table),
?(,
quote_names(reference_columns),
?),
Expand All @@ -1855,24 +1859,32 @@ defmodule Ecto.Adapters.SQLite3.Connection do

defp quote_names(names), do: Enum.map_intersperse(names, ?,, &quote_name/1)

def quote_name(name), do: quote_entity(name)
defp quote_name(nil, name), do: quote_name(name)

def quote_table(table), do: quote_entity(table)
defp quote_name(prefix, name), do: [quote_name(prefix), ?., quote_name(name)]

defp quote_table(nil, name), do: quote_entity(name)

defp quote_table(prefix, _name) when is_atom(prefix) or is_binary(prefix) do
raise ArgumentError, "SQLite3 does not support table prefixes"
defp quote_name(val) when is_atom(val) do
quote_name(Atom.to_string(val))
end

defp quote_table(_, name), do: quote_entity(name)
defp quote_name(val) when is_binary(val) do
# Don't introduce unnecessary complexity and align with Ecto.Adapters.Postgres.Connection.
#
# Although SQLite and Postgres both allow syntax like:
# ```sql
# CREATE TABLE "lookma""quotes"(id INTEGER);
# SELECT name FROM sqlite_schema WHERE name = 'lookma"quotes';
# SELECT relname FROM pg_class WHERE relname = 'lookma"quotes';
# ```
# there isn't much practical use case for it.
if String.contains?(val, "\"") do
raise ArgumentError,
"bad literal/field/index/table name #{inspect(val)} (\" is not permitted)"
end

defp quote_entity(val) when is_atom(val) do
quote_entity(Atom.to_string(val))
[[?", val, ?"]]
end

defp quote_entity(val), do: [[?", val, ?"]]

defp intersperse_reduce(list, separator, user_acc, reducer, acc \\ [])

defp intersperse_reduce([], _separator, user_acc, _reducer, acc),
Expand Down Expand Up @@ -1903,4 +1915,27 @@ defmodule Ecto.Adapters.SQLite3.Connection do
|> escape_string()
|> :binary.replace("\"", "\\\"", [:global])
end

# We know this holds since exqlite does not export sqlite3_db_config from Sqlite3NIF,
# thus nobody can call sqlite3_db_config(db, SQLITE_DBCONFIG_MAINDBNAME, ...)
defp normalize_database_name(nil) do
# TODO: handle modifier-selected temp database somehow?
# src/parse.y: `temp(A) ::= TEMP. {A = pParse->db->init.busy==0;}`
"main"
end

defp normalize_database_name(name) when is_atom(name) do
normalize_database_name(Atom.to_string(name))
end

defp normalize_database_name(name) when is_binary(name) do
String.downcase(name, :ascii)
end

defp assert_same_database(table_prefix, ref_prefix) do
if normalize_database_name(ref_prefix || table_prefix) !=
normalize_database_name(table_prefix) do
raise ArgumentError, "SQLite3 does not support cross-database foreign keys"
end
end
end
12 changes: 6 additions & 6 deletions test/ecto/adapters/sqlite3/connection/delete_all_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,20 @@ defmodule Ecto.Adapters.SQLite3.Connection.DeleteAllTest do
end

test "delete all with prefix" do
assert_raise ArgumentError, "SQLite3 does not support table prefixes", fn ->
query =
Schema
|> Ecto.Queryable.to_query()
|> Map.put(:prefix, "prefix")
|> plan()
|> delete_all()
end

assert_raise ArgumentError, "SQLite3 does not support table prefixes", fn ->
assert delete_all(query) == ~s{DELETE FROM "prefix"."schema" AS s0}

query =
Schema
|> from(prefix: "first")
|> Map.put(:prefix, "prefix")
|> plan()
|> delete_all()
end

assert delete_all(query) == ~s{DELETE FROM "first"."schema" AS s0}
end
end
6 changes: 6 additions & 0 deletions test/ecto/adapters/sqlite3/connection/delete_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,11 @@ defmodule Ecto.Adapters.SQLite3.DeleteTest do
test "delete" do
query = delete(nil, "schema", [x: 1, y: 2], [])
assert query == ~s{DELETE FROM "schema" WHERE "x" = ? AND "y" = ?}

query = delete("prefix", "schema", [x: 1, y: 2], [])
assert query == ~s{DELETE FROM "prefix"."schema" WHERE "x" = ? AND "y" = ?}

query = delete(nil, "schema", [x: nil, y: 2], [])
assert query == ~s{DELETE FROM "schema" WHERE "x" IS NULL AND "y" = ?}
end
end
5 changes: 2 additions & 3 deletions test/ecto/adapters/sqlite3/connection/insert_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ defmodule Ecto.Adapters.SQLite3.Connection.InsertTest do
query = insert(nil, "schema", [], [[]], {:raise, [], []}, [])
assert query == ~s{INSERT INTO "schema" DEFAULT VALUES}

assert_raise ArgumentError, "SQLite3 does not support table prefixes", fn ->
insert("prefix", "schema", [], [[]], {:raise, [], []}, [])
end
assert insert("prefix", "schema", [], [[]], {:raise, [], []}, []) ==
~s{INSERT INTO "prefix"."schema" DEFAULT VALUES}

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