From b81cd484e5a70bb860f0d47dc248a3edfa939936 Mon Sep 17 00:00:00 2001
From: Aaron Seigo <aseigo@mykolab.com>
Date: Wed, 20 Nov 2024 22:11:45 +0100
Subject: [PATCH] Report correct error for value lists in joins (#156)

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)
---
 lib/ecto/adapters/sqlite3/connection.ex       | 32 +++++++++++++------
 .../adapters/sqlite3/connection/join_test.exs | 18 +++++++++++
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/lib/ecto/adapters/sqlite3/connection.ex b/lib/ecto/adapters/sqlite3/connection.ex
index 4bbcaea..7386347 100644
--- a/lib/ecto/adapters/sqlite3/connection.ex
+++ b/lib/ecto/adapters/sqlite3/connection.ex
@@ -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)
@@ -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)
 
@@ -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),
@@ -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
diff --git a/test/ecto/adapters/sqlite3/connection/join_test.exs b/test/ecto/adapters/sqlite3/connection/join_test.exs
index b5333ad..fbc0409 100644
--- a/test/ecto/adapters/sqlite3/connection/join_test.exs
+++ b/test/ecto/adapters/sqlite3/connection/join_test.exs
@@ -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