Skip to content

Add explicit NULL ordering and NULL-aware cursor pagination - #24

Open
kac-axelor wants to merge 1 commit into
axelor:devfrom
kac-axelor:orderby-null-sorting
Open

kac-axelor wants to merge 1 commit into
axelor:devfrom
kac-axelor:orderby-null-sorting

Conversation

@kac-axelor

Copy link
Copy Markdown
Contributor

SQL has two orthogonal concepts for ordering nullable columns: the direction (ASC/DESC) and the NULL placement (NULLS FIRST/LAST). Postgres applies a default — ASC → NULLS LAST, DESC → NULLS FIRST — but callers often need explicit control, e.g. "most recent first, items without a date at the bottom" (DESC NULLS LAST).

The OrderBy type previously exposed only "ASC" and "DESC", so callers could not pin NULL placement independently of direction. This PR adds four explicit variants and plumbs them through the parser, the GraphQL schema, and TypeORM's OrderByCondition.

While extending the cursor-pagination code for the new variants, several pre-existing bugs surfaced in the plain ASC/DESC path. Cursor pagination over nullable columns was silently dropping or misordering rows; this PR fixes those too.


Changes

1. Extend the OrderBy enum

OrderBy gains ASC_NULLS_FIRST, ASC_NULLS_LAST, DESC_NULLS_FIRST, DESC_NULLS_LAST. The companion ORDER_OPS / ORDER_INVERTED tables in parser/types.ts are extended so existing comparison and inversion logic works for the new values — including symmetric flipping of both direction and null position for backward pagination.

2. Translate to TypeORM's OrderByCondition

A new toSqlOrder helper in repository/query/utils.ts maps the enum string to TypeORM's { order, nulls } shape. createSelectQuery and the backward-pagination subquery route through it so the generated SQL contains real NULLS FIRST/LAST clauses.

3. Surface the values in GraphQL

The OrderBy enum in graphql-schema.ts gains the four new values. Because every per-entity OrderBy input and JsonOrderByValue.order reuse this single enum, the new values become available on every orderable field automatically. The schema change is purely additive — existing ASC/DESC clients keep working.

4. NULL-aware cursor predicate (bug fix)

parser/cursor.ts is rewritten to be NULL-correct. Three pre-existing bugs are fixed:

  • Multi-key tie-break used =, which is unknown for NULL = NULL → rows were silently dropped. Now uses IS NOT DISTINCT FROM.
  • Trailing-NULL rows were never reached, because key > :cursor is unknown when the row's key is NULL. When NULLs sit on the "after" side of the current direction, the predicate now adds OR key IS NULL.
  • A NULL cursor value produced an empty page. Now handled explicitly: IS NOT NULL when NULLs are at the start of our direction, FALSE when at the end.

NULL placement is derived from the enum (explicit NULLS_FIRST/LAST wins; otherwise the Postgres default), and flipped for backward pagination so previous-page boundaries are symmetric with forward.

5. Tests

  • Feature tests for forward and backward cursors with each new enum value.
  • Regression tests for the bugs above, using only plain "ASC" / "DESC" — these fail against the old code.
  • GraphQL schema test asserts the four new values are present.

6. README

"Sorting and Ordering" section gains an example, the list of six supported values, and a one-liner about the default fallback.


Notes

The new SQL syntax (NULLS FIRST/LAST, IS NOT DISTINCT FROM) is Postgres-specific. Goovee ORM targets Postgres, so this is fine.

The cursor bug was scoped to nullable columns. Cursors keyed on id (the most common case, as shown in the README) were unaffected, which is likely why the bug survived this long.

Extend OrderBy with ASC_NULLS_FIRST/LAST and DESC_NULLS_FIRST/LAST,
exposed through the GraphQL OrderBy enum and translated to SQL
NULLS FIRST/LAST.

Fix cursor pagination over nullable columns:
- Tie-break uses IS NOT DISTINCT FROM so multi-key cursors do not
  drop rows when a key is NULL.
- Keyset predicate includes trailing NULLs when they sit after the
  cursor, and handles a NULL cursor value explicitly.
- Plain ASC/DESC fall back to the Postgres default (ASC -> NULLS LAST,
  DESC -> NULLS FIRST); backward pagination flips null position with
  direction.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant