Skip to content

Make keyset cursor pagination NULL-aware#1331

Open
SachaProbo wants to merge 1 commit into
mainfrom
SachaProbo/null-cursor
Open

Make keyset cursor pagination NULL-aware#1331
SachaProbo wants to merge 1 commit into
mainfrom
SachaProbo/null-cursor

Conversation

@SachaProbo

@SachaProbo SachaProbo commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Cursor.SQLFragment emitted a seek predicate with no NULL handling, so once a page boundary landed on a NULL order value the WHERE matched no rows and pagination terminated early, silently dropping the remaining rows.

Rewrite SQLFragment to emit explicit NULLS placement and a null-aware, type-agnostic seek predicate, mirroring the direction and NULLS placement on the Head/Tail inversion so backward paging stays an exact inverse. For NOT NULL columns it reduces to the previous predicate.

Add white-box pagination tests that evaluate the generated SQL with three-valued logic and walk the whole set forward and backward across the NULL boundary.


Summary by cubic

Makes keyset cursor pagination NULL-aware so pages don’t truncate when the sort column has NULLs. Adds explicit NULLS placement and a null-aware seek; backward paging mirrors forward exactly.

Tests +785 -0

  • Add Postgres-backed e2e walk over adversarial datasets (NULL blocks, ties) to confirm forward/backward paging reconstruct full order.
  • Lock exact SQL and named args across direction/position/key cases; @cursor_field_value appears only for non-NULL seeks.
  • Add unit tests for CursorKey round-trip and invalid formats, OrderDirection text marshaling, and Page trimming/HasNext/HasPrev for Head/Tail.

Service +67 -21

  • Rewrite Cursor.SQLFragment via a template that sets ASC/DESC, NULLS FIRST/LAST, and id tiebreak; Tail flips both for symmetry.
  • Use a null-aware, type-agnostic seek; bind @cursor_field_value only when non-NULL.

Other +2 -2

  • Run the new e2e suite in CI and Makefile by including ./e2e/page/....

Written for commit 00d4d5f. Summary will update on new commits.

Review in cubic

@SachaProbo SachaProbo force-pushed the SachaProbo/null-cursor branch 5 times, most recently from fdd28b0 to b0cce4e Compare June 16, 2026 13:15
@SachaProbo SachaProbo marked this pull request as ready for review June 16, 2026 13:18

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

@SachaProbo SachaProbo requested a review from a team June 16, 2026 16:16
@SachaProbo SachaProbo force-pushed the SachaProbo/null-cursor branch 5 times, most recently from cfee0e2 to 4b58313 Compare June 16, 2026 16:58
Comment thread pkg/page/cursor.go
package page

import (
"strings"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add test with real pg

@SachaProbo SachaProbo force-pushed the SachaProbo/null-cursor branch 5 times, most recently from 957e758 to e2cf8d6 Compare June 16, 2026 20:02
Cursor.SQLFragment emitted a seek predicate with no NULL handling, so once
a page boundary landed on a NULL order value the WHERE matched no rows and
pagination terminated early, silently dropping the remaining rows.

Rewrite SQLFragment to render the predicate from a template that emits
explicit NULLS placement and a null-aware, type-agnostic seek, mirroring
the direction and NULLS placement on the Head/Tail inversion so backward
paging stays an exact inverse. For NOT NULL columns it reduces to the
previous predicate.

Cover the whole cursor stack with tests: a real-Postgres pagination walk
(e2e/page) that drives the production SQLFragment/SQLArguments forward and
backward through every NULL edge case, plus unit tests for the generated
SQL and arguments, cursor-key serialization, page trimming, and order
direction parsing.

Signed-off-by: Sacha Al Himdani <sacha@getprobo.com>
@SachaProbo SachaProbo force-pushed the SachaProbo/null-cursor branch from e2cf8d6 to 00d4d5f Compare June 16, 2026 20:10
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.

2 participants