fix integer comparisons against infinity (#6)

CHANGELOG.md

@@ -6,6 +6,13 @@ The format is based on [Keep a
 Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## 0.5.0 - 2021-12-03
+
+### Changed
+
+- Removed usages of `infinity` and `-infinity` postgres fragments
+    - This fixes a query error when ordering by a nillable integer-type column
+
 ## 0.4.0 - 2021-03-17
 
 ### Added

README.md

@@ -7,8 +7,6 @@ A minimalistic keyset pagination library for Ecto queries
 **N.B.**: you probably don't want to use Fob. Fob makes a number of assumptions
 about the queries passed to it, such as:
 
-- they'll be run against a modern PostgreSQL database
-    - fob uses the `-infinity` and `infinity` features of PostgreSQL
 - each schema will have only one primary key
 - each query must be ordered by at least the schema's primary key
 - the primary keys is always the final ordering condition

lib/fob.ex

@@ -75,26 +75,10 @@ defmodule Fob do
          },
          acc
        )
-       when direction in [:asc, :asc_nulls_last] do
+       when direction in [:asc, :asc_nulls_last, :desc_nulls_last] do
     dynamic([{t, table}], field(t, ^column) |> is_nil() and ^acc)
   end
 
-  defp apply_keyset_comparison(
-         %PageBreak{
-           direction: :asc_nulls_first,
-           column: column,
-           table: table,
-           value: nil
-         },
-         acc
-       ) do
-    dynamic(
-      [{t, table}],
-      field(t, ^column) > fragment("'-infinity'") or
-        (field(t, ^column) |> is_nil() and ^acc)
-    )
-  end
-
   defp apply_keyset_comparison(
          %PageBreak{
            direction: direction,
@@ -104,26 +88,14 @@ defmodule Fob do
          },
          acc
        )
-       when direction in [:desc, :desc_nulls_first] do
+       when direction in [:desc, :desc_nulls_first, :asc_nulls_first] do
     dynamic(
       [{t, table}],
-      field(t, ^column) < fragment("'infinity'") or
+      not is_nil(field(t, ^column)) or
         (field(t, ^column) |> is_nil() and ^acc)
     )
   end
 
-  defp apply_keyset_comparison(
-         %PageBreak{
-           direction: :desc_nulls_last,
-           column: column,
-           table: table,
-           value: nil
-         },
-         acc
-       ) do
-    dynamic([{t, table}], field(t, ^column) |> is_nil() and ^acc)
-  end
-
   # --- value is non-nil
 
   defp apply_keyset_comparison(

lib/fob/cursor.ex

@@ -1,6 +1,11 @@
 defmodule Fob.Cursor do
   @moduledoc """
-  TODO
+  A cursor data structure that can be used to fetch the next pages
+
+  The cursor data structure is somewhat similar to a `Stream` except that it
+  cannot wrap a stateful process and does not require any life-cycle hooks.
+  The next pages of data can be fetched by calling `next/1` successively,
+  passing in the updated `t:t/0` data structure each time.
   """
 
   if Version.match?(System.version(), ">= 1.8.0") do

priv/repo/migrations/20210226170626_create_nillable_schema.exs

@@ -5,6 +5,7 @@ defmodule Fob.Repo.Migrations.CreateNillableSchema do
     create table(:nillable_schema) do
       # a field named "date" of type :date
       add :date, :date
+      add :count, :integer
     end
   end
 end

test/fob/nillable_test.exs

@@ -6,10 +6,12 @@ defmodule Fob.NillableTest do
 
   This is a common gripe for keyset pagination and is mostly a problem from
   a SQL comparison perspective: `f.foo > nil` doesn't really work as you might
-  expect. To remedy this, we test equality with `is_nil/1` and do comparisons
+  expect. To remedy this, we used to test equality with `is_nil/1` and do comparisons
   against the postgres-specific `fragment("'-infinity'")` and
   `fragment("'infinity'")` (depending on the direction of the sort of the
-  column).
+  column). However, this doesn't work for numerics in even modern versions of
+  postgres. Instead we optimize to checking `is_nil(column) == false`, which
+  works out to effectively be the same check.
   """
 
   alias Fob.Cursor
@@ -22,27 +24,27 @@ defmodule Fob.NillableTest do
   setup c do
     records =
       [
-        ~D[2020-02-12],
-        ~D[2020-02-13],
-        ~D[2020-02-14],
-        ~D[2020-02-15],
-        ~D[2020-02-16],
+        {~D[2020-02-12], 1},
+        {~D[2020-02-13], 2},
+        {~D[2020-02-14], 3},
+        {~D[2020-02-15], 4},
+        {~D[2020-02-16], 5},
         # ---
-        ~D[2020-02-17],
-        nil,
-        nil,
-        nil,
-        nil,
+        {~D[2020-02-17], 6},
+        {nil, nil},
+        {nil, nil},
+        {nil, nil},
+        {nil, nil},
         # ---
-        nil,
-        nil,
-        nil,
-        nil,
-        nil
+        {nil, nil},
+        {nil, nil},
+        {nil, nil},
+        {nil, nil},
+        {nil, nil}
       ]
       |> Enum.with_index()
-      |> Enum.map(fn {date, index} ->
-        %{id: index, date: date}
+      |> Enum.map(fn {{date, count}, index} ->
+        %{id: index, date: date, count: count}
       end)
 
     Multi.new()
@@ -52,7 +54,7 @@ defmodule Fob.NillableTest do
     :ok
   end
 
-  test "we can get all pages when sorting ascending (default)", c do
+  test "we can get all pages when sorting ascending (default) by date", c do
     cursor =
       Cursor.new(
         order_by(c.schema, asc: :date, asc: :id),
@@ -88,6 +90,60 @@ defmodule Fob.NillableTest do
     {[], _cursor} = Cursor.next(cursor)
   end
 
+  test "we can get all pages when sorting descending by integer type", c do
+    cursor =
+      Cursor.new(
+        order_by(c.schema, desc: :count, asc: :id),
+        c.repo,
+        nil,
+        5
+      )
+
+    {records, cursor} = Cursor.next(cursor)
+
+    assert records |> Enum.map(& &1.id) == Enum.to_list(6..10)
+    assert records |> Enum.map(& &1.count) == repeat(nil, 5)
+
+    {records, cursor} = Cursor.next(cursor)
+
+    assert records |> Enum.map(& &1.id) == [11, 12, 13, 14, 5]
+    assert records |> Enum.map(& &1.count) == repeat(nil, 4) ++ [6]
+
+    {records, cursor} = Cursor.next(cursor)
+
+    assert records |> Enum.map(& &1.id) == Enum.to_list(4..0)
+    assert records |> Enum.map(& &1.count) == Enum.to_list(5..1)
+
+    {[], _cursor} = Cursor.next(cursor)
+  end
+
+  test "we can get all pages when sorting ascending by integer type", c do
+    cursor =
+      Cursor.new(
+        order_by(c.schema, asc: :count, asc: :id),
+        c.repo,
+        nil,
+        5
+      )
+
+    {records, cursor} = Cursor.next(cursor)
+
+    assert records |> Enum.map(& &1.count) == Enum.to_list(1..5)
+    assert records |> Enum.map(& &1.id) == Enum.to_list(0..4)
+
+    {records, cursor} = Cursor.next(cursor)
+
+    assert records |> Enum.map(& &1.count) == [6 | repeat(nil, 4)]
+    assert records |> Enum.map(& &1.id) == Enum.to_list(5..9)
+
+    {records, cursor} = Cursor.next(cursor)
+
+    assert records |> Enum.map(& &1.count) == repeat(nil, 5)
+    assert records |> Enum.map(& &1.id) == Enum.to_list(10..14)
+
+    {[], _cursor} = Cursor.next(cursor)
+  end
+
   test "we can get all pages when sorting ascending (nulls last)", c do
     cursor =
       Cursor.new(
@@ -267,4 +323,6 @@ defmodule Fob.NillableTest do
 
     {[], _cursor} = Cursor.next(cursor)
   end
+
+  defp repeat(value, times), do: for(_n <- 1..times, do: value)
 end

test/support/nillable_schema.ex

@@ -8,5 +8,6 @@ defmodule NillableSchema do
   schema "nillable_schema" do
     # a field named "date" of type :date
     field :date, :date
+    field :count, :integer
   end
 end