[Bug] Cursor based pagination with :before option does not work on queries with order_by

[tomekowal]

Summary

I am deriving Flop.Schema with following options:

  @derive {
    Flop.Schema,
    filterable: [],
    sortable: [:id],
    default_limit: 10,
    max_limit: 100,
    default_order: %{order_by: [:id], order_directions: [:desc]},
    default_pagination_type: :first
  }

When I first load the page, I get the following struct:

%Flop.Meta{
  backend: nil,
  current_offset: nil,
  current_page: nil,
  end_cursor: "g3QAAAABdwJpZGHQ",              # decoded it is %{id: 208}
  next_offset: nil,
  next_page: nil,
  page_size: 10,
  previous_offset: nil,
  previous_page: nil,
  schema: OctoBackend.P2m.IssuingScheme.Payments.Payment,
  start_cursor: "g3QAAAABdwJpZGHZ",            # decoded it is %{id: 217}
  total_count: nil,
  total_pages: nil,
  errors: [],
  flop: %Flop{
    after: nil,
    before: nil,
    first: 10,
    last: nil,
    limit: nil,
    offset: nil,
    order_by: [:id],
    order_directions: [:desc],
    page: nil,
    page_size: nil,
    decoded_cursor: nil,
    filters: []
  },
  has_next_page?: true,
  has_previous_page?: false,
  opts: [for: OctoBackend.P2m.IssuingScheme.Payments.Payment],
  params: %{}
}

I am creating prev and next buttons myself, for Next, I am putting %{after: meta.end_cursor} and for Prev I am using %{before: meta.start_cursor}.

The next button works fine, but when I click Prev, I get the following:

%Flop.Meta{
  backend: nil,
  current_offset: nil,
  current_page: nil,
  end_cursor: "g3QAAAABdwJpZGHZ",          # decoded it is %{id: 217}
  next_offset: nil,
  next_page: nil,
  page_size: 10,
  previous_offset: nil,
  previous_page: nil,
  schema: OctoBackend.P2m.IssuingScheme.Payments.Payment,
  start_cursor: "g3QAAAABdwJpZGHQ",       # decoded it is %{id: 208}
  total_count: nil,
  total_pages: nil,
  errors: [],
  flop: %Flop{
    after: nil,
    before: "g3QAAAABdwJpZGGx",                # decoded it is  %{id: 177}}
    first: nil,
    last: 10,
    limit: nil,
    offset: nil,
    order_by: [:id],
    order_directions: [:desc],
    page: nil,
    page_size: nil,
    decoded_cursor: nil,
    filters: []
  },
  has_next_page?: true,
  has_previous_page?: true,
  opts: [for: OctoBackend.P2m.IssuingScheme.Payments.Payment],
  params: %{}
}

So, it seems I am correctly trying to get 10 rows before id 177, but I get results from 208 to 217.
The order of those results is also reversed. 208, 209 ... 217 (even though it should :desc by :id).

Steps to reproduce

1. Create a schema with derive options specified above
2. Insert more than 10 records
3. Run Flop pagination with :before option

Expected behaviour

I get records starting with the specified id in the predefined order

Actual behaviour

I get records that are not adjacent to the the id in cursor in reversed order.

Elixir/Erlang version

Erlang/OTP 26 [erts-14.2.5.3] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit]

Elixir 1.16.3 (compiled with Erlang/OTP 26)

Flop and Ecto versions

Flop 0.26.1
Ecto 3.12.4

Additional context

No response

[woylie]

Hi @tomekowal,

I'm using cursor pagination in several production applications and haven't seen any issues with it. With the information you posted, I can't help you pinpoint the issue. A complete running example would be helpful.

Here's the result of a quick test that goes from the first page (a) to the next 10 results after the end cursor (b), and from there to the previous 10 results before the start cursor (c). As you can see, the result set of (a) is equal to the one of (c), as expected.

Schema:

  @derive {
    Flop.Schema,
    filterable: [],
    sortable: [:id],
    default_limit: 10,
    max_limit: 100,
    default_order: %{order_by: [:id], order_directions: [:desc]},
    default_pagination_type: :first
  }

Query function:

  def list_fruits(params) do
    Flop.validate_and_run!(Fruit, params, for: Fruit)
  end

Queries:

iex(18)> first_ten_no_cursor = list_fruits(%{first: 10})
[debug] QUERY OK source="fruits" db=4.0ms idle=150.8ms
SELECT f0."id", f0."name", f0."inserted_at", f0."updated_at" FROM "fruits" AS f0 ORDER BY f0."id" DESC LIMIT $1 [11]
↳ Flop.run/3, at: lib/flop.ex:685
{[
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 25,
     name: "Ziziphus",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 24,
     name: "Yangmei",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 23,
     name: "Xigua",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 22,
     name: "White Sapote",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 21,
     name: "Vanilla Orchid",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 20,
     name: "Ugni",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 19,
     name: "Tamarillo",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 18,
     name: "Salak",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 17,
     name: "Rambutan",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 16,
     name: "Quince",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   }
 ],
 %Flop.Meta{
   backend: nil,
   current_offset: nil,
   current_page: nil,
   end_cursor: "g3QAAAABdwJpZGEQ",
   next_offset: nil,
   next_page: nil,
   page_size: 10,
   previous_offset: nil,
   previous_page: nil,
   schema: FlopCursorTest.Fruits.Fruit,
   start_cursor: "g3QAAAABdwJpZGEZ",
   total_count: nil,
   total_pages: nil,
   errors: [],
   flop: %Flop{
     after: nil,
     before: nil,
     first: 10,
     last: nil,
     limit: nil,
     offset: nil,
     order_by: [:id],
     order_directions: [:desc],
     page: nil,
     page_size: nil,
     decoded_cursor: nil,
     filters: []
   },
   has_next_page?: true,
   has_previous_page?: false,
   opts: [for: FlopCursorTest.Fruits.Fruit],
   params: %{}
 }}
iex(19)> {next_ten_result, next_ten_meta} = list_fruits(%{first: 10, after: first_ten_meta.end_cursor})
[debug] QUERY OK source="fruits" db=3.3ms idle=1724.4ms
SELECT f0."id", f0."name", f0."inserted_at", f0."updated_at" FROM "fruits" AS f0 WHERE (f0."id" < $1::bigint) ORDER BY f0."id" DESC LIMIT $2 [16, 11]
↳ Flop.run/3, at: lib/flop.ex:685
{[
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 15,
     name: "Pitaya",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 14,
     name: "Osage Orange",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 13,
     name: "Noni",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 12,
     name: "Mamey Sapote",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 11,
     name: "Langsat",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 10,
     name: "Kiwano",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 9,
     name: "Jabuticaba",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 8,
     name: "Imbu",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 7,
     name: "Horned Melon",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 6,
     name: "Gac",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   }
 ],
 %Flop.Meta{
   backend: nil,
   current_offset: nil,
   current_page: nil,
   end_cursor: "g3QAAAABdwJpZGEG",
   next_offset: nil,
   next_page: nil,
   page_size: 10,
   previous_offset: nil,
   previous_page: nil,
   schema: FlopCursorTest.Fruits.Fruit,
   start_cursor: "g3QAAAABdwJpZGEP",
   total_count: nil,
   total_pages: nil,
   errors: [],
   flop: %Flop{
     after: "g3QAAAABdwJpZGEQ",
     before: nil,
     first: 10,
     last: nil,
     limit: nil,
     offset: nil,
     order_by: [:id],
     order_directions: [:desc],
     page: nil,
     page_size: nil,
     decoded_cursor: nil,
     filters: []
   },
   has_next_page?: true,
   has_previous_page?: true,
   opts: [for: FlopCursorTest.Fruits.Fruit],
   params: %{}
 }}
iex(20)> {and_back_result, and_back_meta} = list_fruits(%{last: 10, before: next_ten_meta.start_cursor})
[debug] QUERY OK source="fruits" db=3.1ms idle=1723.7ms
SELECT f0."id", f0."name", f0."inserted_at", f0."updated_at" FROM "fruits" AS f0 WHERE (f0."id" > $1::bigint) ORDER BY f0."id" LIMIT $2 [15, 11]
↳ Flop.run/3, at: lib/flop.ex:699
{[
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 25,
     name: "Ziziphus",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 24,
     name: "Yangmei",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 23,
     name: "Xigua",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 22,
     name: "White Sapote",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 21,
     name: "Vanilla Orchid",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 20,
     name: "Ugni",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 19,
     name: "Tamarillo",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 18,
     name: "Salak",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 17,
     name: "Rambutan",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   },
   %FlopCursorTest.Fruits.Fruit{
     __meta__: #Ecto.Schema.Metadata<:loaded, "fruits">,
     id: 16,
     name: "Quince",
     inserted_at: ~U[2024-11-24 12:30:43Z],
     updated_at: ~U[2024-11-24 12:30:43Z]
   }
 ],
 %Flop.Meta{
   backend: nil,
   current_offset: nil,
   current_page: nil,
   end_cursor: "g3QAAAABdwJpZGEQ",
   next_offset: nil,
   next_page: nil,
   page_size: 10,
   previous_offset: nil,
   previous_page: nil,
   schema: FlopCursorTest.Fruits.Fruit,
   start_cursor: "g3QAAAABdwJpZGEZ",
   total_count: nil,
   total_pages: nil,
   errors: [],
   flop: %Flop{
     after: nil,
     before: "g3QAAAABdwJpZGEP",
     first: nil,
     last: 10,
     limit: nil,
     offset: nil,
     order_by: [:id],
     order_directions: [:desc],
     page: nil,
     page_size: nil,
     decoded_cursor: nil,
     filters: []
   },
   has_next_page?: true,
   has_previous_page?: false,
   opts: [for: FlopCursorTest.Fruits.Fruit],
   params: %{}
 }}
iex(21)> and_back_result == first_ten_result
true

[tomekowal]

Hi! I am sorry, the issue was on my side.

I debugged some more and I've found the problem.
When trying out Flop, I didn't remove |> order_by(desc: :id) from the original query.
Flop.order_by applies the ordering by appending it to the query.
https://github.com/woylie/flop/blob/main/lib/flop.ex#L1046
So, when original query was:

#Ecto.Query<from p0 in Payment,
 order_by: [desc: p0.id]>

the one with applied ordering was:

#Ecto.Query<from p0 in Payment,
 order_by: [desc: p0.id], order_by: [asc: p0.id]>

(which does not make much sense).

Would it make sense to detect during cursor based pagination that the original query had ordering applied and log a warning that queries with order_by might mess with pagination?

[woylie]

I guess logging a warning would make sense. This would have to happen in Flop.paginate/3 in the function bodies that handle first and last.

[tomekowal]

We might do it on the adatper level in apply_order_by. I can't imagine a situation where I want to apply ordering and it makes sense to append to query with existing ordering.
We might even take it a step further and also remove the existing order_by before applying the one requested by flop. Modifying queries like that is not supported by Ecto.