Repository navigation
Fix nested collection take/skip to paginate per record - #32
Open
kac-axelor wants to merge 2 commits into
Open
kac-axelor wants to merge 2 commits into
kac-axelor wants to merge 2 commits into
Conversation
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.
A take/skip inside a collection select was applied as a LIMIT on the single batched query that loads the collection for the whole page of parents, capping the combined batch instead of each record's list. Nested take/skip are now stripped from the batch query and applied per record: a row_number() window partitioned by the parent filters the ids (matched as (parent, item) pairs so shared many-to-many items cannot leak across windows); when the nested where/order traverses another collection, the filtered lists are sliced in memory instead, since the multiplied rows would corrupt the ranking. Negative take selects from the end of each list, mirroring the top-level semantics via the inverted NULL-aware ordering. A nested cursor is dropped alongside, and multi-valued join detection now walks the whole join chain.
kac-axelor
force-pushed
the
nested-collection-pagination
branch
from
June 11, 2026 00:11
e1ae70e to
9235775
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #31.
A
take/skipinside a collection select is meant to limit each record'sown list — "every contact with its newest address". Instead the limit was
shared by all the records of the page combined, so some records silently came
back with incomplete or empty lists. This PR makes the nested limit mean what
callers expect, for every relation kind, including negative
take("the lastN of each list").
How
Related records are loaded with one batched query for the whole page of
parents, so a plain
LIMITthere can never be per-record. Instead:take/skipare stripped from the batch query and enforced by aranked filter: a
row_number()window partitioned by the parent ranks eachrecord's items by the nested ordering, and only ids inside the requested
window survive. The filter matches
(parent, item)pairs, so a sharedmany-to-many item inside one record's window cannot leak into another's.
takeSkipBounds): thetop-level query and the in-memory fallback compute the same
[start, end)window from it, and the ranked filter realizes the identical rule in SQL —
a negative take anchors its window with the partition's
count(*), clampedat the list start exactly like the in-memory rule.
wherereaches through another collection, the multipliedjoin rows are deduplicated inside the ranked subquery (
DISTINCTover the(parent, item, order values)tuples) — valid whenever the ordering refersonly to the item's own columns. Only an
orderBythat itself traverses acollection (per-item ambiguous) falls back to fetching the filtered lists
and slicing per record in memory. Multi-valued join detection walks the
whole join chain rather than only the first level.
findOne, and the GraphQL nestedconnection resolver) keeps the full top-level semantics for its collections
— nested
cursorpagination and_count/_cursormetadata included —since the batch is exactly that record's list.
take/skipnow gets the unique-ordering tiebreaker from the parser(previously only page queries did), so limited windows are deterministic
at every nesting level even without an explicit
orderBy.Behavior notes
cursoris ignored when nestedtake/skipis present, and nested items carry no_count/_cursormetadata; single-record pages keep both (relay connections rely on this).
_hasPrev/_hasNextare now cursor-aware: the old count-based formulaignored the cursor filter and could report
hasNextat the end of a list.take: nullfrom untyped callers is treated as "no limit" insteadof producing an empty window.
Tests
17 e2e tests in the nested-pagination suite — per-record take / skip /
negative take (including window clamping past the list start, on both query
paths) / ordering / default id ordering (with physical row order shuffled to
prove it), filters through single-valued and collection relations, ordering
through a collection, two-level pagination, shared many-to-many adversarial
cases,
take: null, and single-record page metadata with nested cursorpaging — plus a GraphQL relay round-trip (edge cursors,
totalCount,hasNextPage, paging withafter). Full e2e suite green.Stacked on #24 — this branch builds on
orderby-null-sortingand consumesits ordering primitives; until #24 merges the diff shows its commits too.
Review only the last commit (
9235775). Will rebase ontodevafter #24lands.