Cursor pagination with nullable fields

[balysv]

Is your feature request related to a problem? Please describe.
I have a DB schema with nullable TEXT / VARCHAR fields I'd like to sort and paginate on using cursors. However, as cursor pagination is currently implemented in the Ecto adapter (source), it uses the > or < operators which would always remove null values from the result set (at least in PostgreSQL).

Describe the solution you'd like
Enhance the sorting capabilities to allow for sorting data with null values. This can, of course, only work if an additional sorting column like id is provided. The generated query would look something like:

SELECT * FROM users
WHERE (
  name > :last_seen_val OR
  (name = :last_seen_val AND id > :last_seen_id) OR
  (name IS NULL AND :last_seen_val IS NULL AND id > :last_seen_id)
)
ORDER BY name ASC NULLS LAST, id ASC
LIMIT 100;

Describe alternatives you've considered
I've looked into custom fields, compound fields (not supported for cursors) or simply ditching the library and rewriting the pagination logic myself (which I'd like not to).

Additional context
I am not 100% sure if I should instead file a bug report but after investigating the code I see that this functionality not supported by the library so opting for a request. If there's a config I'm missing in how to do this, let me know!

I also pushed a repo with tests reproducing the issue: https://github.com/balysv/flop-cursor-pagination. The test case is:

defmodule FlopTest.User do
  use Ecto.Schema
  import Ecto.Changeset

  @derive {
    Flop.Schema,
    filterable: [],
    sortable: [:id, :name],
    default_order: %{
      order_by: [:id],
      order_directions: [:asc]
    },
    default_pagination_type: :offset,
    pagination_types: [:first, :last, :offset]
  }

  schema "users" do
    field(:name, :string)

    timestamps()
  end

  def changeset(user, attrs) do
    user
    |> cast(attrs, [:name])
    |> validate_required([])
  end
end
defmodule FlopTest.UserTest do
  use FlopTest.DataCase
  import ExUnit.Case

  alias FlopTest.User

  setup do
    [nil, nil, "a", "b", "c", "d", nil, nil]
    |> Enum.each(fn name ->
      Repo.insert(%User{name: name})
    end)
  end

  test "page ends in `nil`" do
    query = from(u in User)

    {:ok, {results, meta}} =
      Flop.validate_and_run(
        query,
        %{first: 5, order_by: [:name, :id], order_directions: [:asc]},
        for: User
      )


    # Results: [{"a", 3}, {"b", 4}, {"c", 5}, {"d", 6}, {nil, 7}]
    assert Enum.map(results, & &1.name) == ["a", "b", "c", "d", nil]

    {:ok, {results, meta}} =
      Flop.validate_and_run(
        query,
        %{first: 5, after: meta.end_cursor, order_by: [:name, :id], order_directions: [:asc]},
        for: User
      )

    # Results: [{"a", 3}, {"b", 4}, {"c", 5}, {"d", 6}, {nil, 7}]
    # but instead it should be: [{nil, 7}, {nil, 8}]
    assert Enum.map(results, & &1.name) == [nil, nil]
  end

  test "page ends in value, `nil`s to follow" do
    query = from(u in User)

    {:ok, {results, meta}} =
      Flop.validate_and_run(
        query,
        %{first: 3, order_by: [:name, :id], order_directions: [:asc]},
        for: User
      )

    # Results: page1: [{"a", 3}, {"b", 4}, {"c", 5}]
    assert Enum.map(results, & &1.name) == ["a", "b", "c"]

    {:ok, {results, meta}} =
      Flop.validate_and_run(
        query,
        %{first: 100, after: meta.end_cursor, order_by: [:name, :id], order_directions: [:asc]},
        for: User
      )

    # Results: page2: [{"d", 6}] - all `nils` are filtered out
    assert Enum.map(results, & &1.name) == ["d", nil, nil]
  end
end

[woylie]

Indeed, the implementation assumes that all cursor fields are not nullable. I'm a bit afraid of how the function will look like with the necessary changes - the function starts here, and it would also need to apply the *_nulls_first / *_nulls_last directions correctly. If you want to give it go, I'm open to PRs. Even just comprehensive tests for this would be helpful.

[spicychickensauce]

I'm also running into this issue.
For me it's worse, as it's causing issues with duplicate ids on an infinite scroll pagination liveview.

I thought I could work around it by using order_by: [:nullable_name, :inserted_at, :id], but even that leads to the same issue -.-

While this issue exists, is there a workaround?
Is there a way to generate SQL kinda like this: ORDER BY coalesce(name, inserted_at) ASC, id ASC?

[woylie]

I'm also running into this issue. For me it's worse, as it's causing issues with duplicate ids on an infinite scroll pagination liveview.

I thought I could work around it by using order_by: [:nullable_name, :inserted_at, :id], but even that leads to the same issue -.-

While this issue exists, is there a workaround? Is there a way to generate SQL kinda like this: ORDER BY coalesce(name, inserted_at) ASC, id ASC?

For cursor pagination, Flop needs to define both the where clause and the order by clause based on the cursor. For the order by clause, you could define an alias field, but it's not possible to use it in the where clause, because Postgres doesn't support it. For the where clause, you could define a custom field, but Flop doesn't support sorting by custom fields at the moment. There is an open PR for sorting by custom fields: #549. Once that is ready and extended to support custom fields in cursor pagination, you might be able to work around this limitation.

[spicychickensauce]

Thanks for the super quick response! Looks like I'm already subscribing to that PR, as I need this anyway.
But yeah, the current state of the PR wouldn't be enough for me, as it doesn't implement pagination.

Unfortunately my SQL knowledge is abysmal, so I can't help out.

For now I'll work around the issue by not using streams and doing an Enum.uniq_by around the results. Doesn't resolve the vanishing entries, but at least I don't get any duplicate id bugs like this.

[spicychickensauce]

I found a much better workaround for my situation:

fallback = ~N[1970-01-01 00:42:00]
query =
  from(e in list_entry_query(),
    select_merge: %{used_at: e.used_at |> coalesce(^fallback)}
  )

from(subquery(query))
|> Flop.validate_and_run(params, for: Entry)
|> map_flop_results(fn e ->
  %{e | used_at: if(e.used_at == fallback, do: nil, else: e.used_at)}
end)

with

defp map_flop_results({:ok, {data, meta}}, fun) do
  {:ok, {data |> Enum.map(fun), meta}}
end

defp map_flop_results(res, _fun), do: res

i.e. use a subquery to make the field non null using a fallback value and then map the value back to nil.

And if you need nulls_first, you can use a fallback value that sorts first.

---

Quite ugly solution, but at least it produces correct results.

[woylie]

Interesting. You might also be able to do a lateral join (with parent_as) that selects the nullable fields with a fallback value and define join fields for each of them. Then you wouldn't need a map_flop_results function.

[woylie]

Interesting. You might also be able to do a lateral join (with parent_as) that selects the nullable fields with a fallback value and define join fields for each of them. Then you wouldn't need a map_flop_results function.

Here's an example of what I mean in a different context: #579 (comment)

[spicychickensauce]

Interesting. You might also be able to do a lateral join (with parent_as) that selects the nullable fields with a fallback value and define join fields for each of them. Then you wouldn't need a map_flop_results function.

Thanks, I have never heard of a lateral join before.
I tried, but couldn't make it work, I get: (Postgrex.Error) ERROR 42803 (grouping_error) column "s3.used_at" must appear in the GROUP BY clause or be used in an aggregate function.

I do not understand lateral joins, so I probably did something wrong. Here is the query I've tried:

from(e in list_entry_query(),
  as: :entry,
  inner_lateral_join:
    subquery(
      from(ee in Entry,
        where: ee.id == parent_as(:entry).id,
        select: %{used_at_2: ee.used_at |> coalesce(^fallback)}
      )
    ),
  as: :extra
)

You don't need to try to help me more with this, I'm happy enough with my workaround for now.