Skip to content

Commit

Permalink
Report correct error for value lists in joins (#156)
Browse files Browse the repository at this point in the history
Some SQL syntax does not pass a table prefix (atom or string)
In the case of `create_name`, then third param may be a string or atom
prefix ... and in those cases should be treated as a prefix.

However syntax such as `VALUES (...)` will pass in the values list
information as the third tuple. this was causing spurious exceptions
to be thrown, rather than creating the correct table name

Introduce `assert_valid_join` which checks for unsupported join syntax

This improves the error messages for `JOIN VALUES (...) AS` clauses
which were being mistakenly tagged as a table prefixing issue. Now the
user gets a more accurate message.

It also puts all the join syntax checking into a single function which
makes it a bit eaiser to reason about (e.g. that hints, values, etc. are
not OK, but other join constructs are)
  • Loading branch information
aseigo authored Nov 20, 2024
1 parent c32929c commit b81cd48
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 10 deletions.
32 changes: 22 additions & 10 deletions lib/ecto/adapters/sqlite3/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,8 @@ defmodule Ecto.Adapters.SQLite3.Connection do
defp using_join(%{joins: joins} = query, _kind, prefix, sources) do
froms =
Enum.map_intersperse(joins, ", ", fn
%JoinExpr{qual: _qual, ix: ix, source: source} ->
%JoinExpr{qual: _qual, ix: ix, source: source} = join ->
assert_valid_join(join, query)
{join, name} = get_source(query, sources, ix, source)
[join, " AS " | name]
end)
Expand All @@ -1014,14 +1015,9 @@ defmodule Ecto.Adapters.SQLite3.Connection do
on: %QueryExpr{expr: expression},
qual: qual,
ix: ix,
source: source,
hints: hints
} ->
if hints != [] do
raise Ecto.QueryError,
query: query,
message: "join hints are not supported by SQLite3"
end
source: source
} = join ->
assert_valid_join(join, query)

{join, name} = get_source(query, sources, ix, source)

Expand All @@ -1035,6 +1031,20 @@ defmodule Ecto.Adapters.SQLite3.Connection do
end)
end

defp assert_valid_join(%JoinExpr{hints: hints}, query) when hints != [] do
raise Ecto.QueryError,
query: query,
message: "join hints are not supported by SQLite3"
end

defp assert_valid_join(%JoinExpr{source: {:values, _, _}}, query) do
raise Ecto.QueryError,
query: query,
message: "SQLite3 adapter does not support values lists"
end

defp assert_valid_join(_join_expr, _query), do: :ok

defp join_on(:cross, true, _sources, _query), do: []

defp join_on(_qual, expression, sources, query),
Expand Down Expand Up @@ -1909,10 +1919,12 @@ defmodule Ecto.Adapters.SQLite3.Connection do

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

defp quote_table(_prefix, _name) do
defp quote_table(prefix, _name) when is_atom(prefix) or is_binary(prefix) do
raise ArgumentError, "SQLite3 does not support table prefixes"
end

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

defp quote_entity(val) when is_atom(val) do
quote_entity(Atom.to_string(val))
end
Expand Down
18 changes: 18 additions & 0 deletions test/ecto/adapters/sqlite3/connection/join_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,24 @@ defmodule Ecto.Adapters.SQLite3.Connection.JoinTest do
end
end

test "join with values is not supported" do
assert_raise Ecto.QueryError, fn ->
rows = [%{x: 1, y: 1}, %{x: 2, y: 2}]
types = %{x: :integer, y: :integer}

Schema
|> join(
:inner,
[p],
q in values(rows, types),
on: [x: p.x(), y: p.y()]
)
|> select([p, q], {p.id, q.x})
|> plan()
|> all()
end
end

test "join with fragment" do
query =
Schema
Expand Down

0 comments on commit b81cd48

Please sign in to comment.