lolopinto / ent

MIT License
51 stars 6 forks source link

GraphQL pagination doesn't use all order by clauses #1751

Open Swahvay opened 8 months ago

Swahvay commented 8 months ago

When using a CustomClauseQuery with more than one orderby field, the pagination query only uses the first one in this filter. For instance, if my orderby is:

[
  { column: 'birth_year', direction: 'DESC' },
  { column: 'birth_month', direction: 'DESC' },
  { column: 'birth_day', direction: 'DESC' },
]

The generated SQL is

ORDER BY
  birth_year DESC,
  birth_month DESC,
  birth_date DESC,
  id DESC

But on pagination the WHERE clause added for the cursor is

(
  birth_year < $1
  OR (
    birth_year = $2
    AND id < $3
  )
)

When it should add a where clause of

(
  birth_year < $1
  OR (
    birth_year = $2
    AND birth_month < $3
  )
  OR (
    birth_year = $4
    AND birth_month = $5
    AND birth_date < $6
  )
  OR (
    birth_year = $7
    AND birth_month = $8
    AND birth_date = $9
    AND id < $10
  )
)
lolopinto commented 8 months ago

hmm

When it should add a where clause of

why is the where clause in the pagination this complicate?


(
  birth_year < $1
  OR (
    birth_year = $2
    AND birth_month < $3
  )
  OR (
    birth_year = $4
    AND birth_month = $5
    AND birth_date < $6
  )
  OR (
    birth_year = $7
    AND birth_month = $8
    AND birth_date = $9
    AND id < $10
  )
)
Swahvay commented 8 months ago

It's my understanding that pagination works by using the where clause as a sort of starting point in an ordered list. So, the basic example of ORDER BY id, to paginate, you want WHERE id > {lastID}. So if we order by another clause, like birth_year, then we want to do WHERE birth_year > {lastRowBirthID}, but we also need OR (birth_year = {lastRowBirthID} AND id > {lastRowID}). This seems like a super FB way to do this, relying on the Ent framework's architectural decisions, but it makes sense. It just needs to account for all order by clauses.