Remove order_by for count queries to avoid invalid SQL errors.

Author: seanabrahams

lib/graphql/relay/connection/ecto.ex

@@ -30,7 +30,7 @@ if Code.ensure_loaded?(Ecto) do
         nil -> false
         _ ->
           first_limit = first + 1
-          has_more_records_query = remove_select(from things in query, limit: ^first_limit)
+          has_more_records_query = make_query_countable(from things in query, limit: ^first_limit)
           has_more_records_query = from things in has_more_records_query, select: count(things.id)
           repo.one(has_more_records_query) > first
       end
@@ -39,7 +39,7 @@ if Code.ensure_loaded?(Ecto) do
         nil -> false
         _ ->
           last_limit = last + 1
-          has_prev_records_query = remove_select(from things in query, limit: ^last_limit)
+          has_prev_records_query = make_query_countable(from things in query, limit: ^last_limit)
           has_prev_records_query = from things in has_prev_records_query, select: count(things.id)
           repo.one(has_prev_records_query) > last
       end
@@ -114,15 +114,27 @@ if Code.ensure_loaded?(Ecto) do
     end
 
     def connection_count(repo, query) do
-      query = remove_select(query)
+      query = make_query_countable(query)
       count_query = from things in query, select: count(things.id)
       repo.one(count_query)
     end
 
+    defp make_query_countable(query) do
+      query
+        |> remove_select
+        |> remove_order
+    end
+
     # Remove select if it exists so that we avoid `only one select
     # expression is allowed in query` Ecto exception
     defp remove_select(query) do
-      %{ query | select: nil }
+      query |> Ecto.Query.exclude(:select)
+    end
+
+    # Remove order by if it exists so that we avoid `field X in "order_by"
+    # does not exist in the model source in query`
+    defp remove_order(query) do
+      query |> Ecto.Query.exclude(:order_by)
     end
   end
 end

priv/repo/migrations/20160307040347_create_letter.exs

@@ -4,6 +4,7 @@ defmodule EctoTest.Repo.Migrations.CreateLetter do
   def change do
     create table(:letters) do
       add :letter, :string
+      add :second_column, :string
       timestamps
     end
   end

test/graphql/relay/connection/ecto_test.exs

@@ -11,6 +11,7 @@ defmodule GraphQL.Relay.Connection.EctoTest do
 
     schema "letters" do
       field :letter, :string
+      field :second_column, :string
       timestamps
     end
   end
@@ -62,6 +63,7 @@ defmodule GraphQL.Relay.Connection.EctoTest do
   test "querying for counts does not raise exception if select already exists" do
     query = letters_query
       |> select([l], %{id: l.id, letter: l.letter})
+      |> order_by([l], asc: l.second_column)
     assert(Connection.Ecto.resolve(query, %{repo: Repo}))
   end