Fix cursor pagination dropping rows when sorting by a nullable column#68869
Open
steveahnahn wants to merge 2 commits into
Open
Fix cursor pagination dropping rows when sorting by a nullable column#68869steveahnahn wants to merge 2 commits into
steveahnahn wants to merge 2 commits into
Conversation
Cursor (keyset) paginated REST list endpoints silently dropped rows when the sort column was nullable. The keyset predicate and the generated ORDER BY disagreed on where NULLs sort, so once a page boundary fell on the NULL/non-NULL edge every row on one side of it was skipped with no error. This is silent data loss from the public API under a common sort such as start_date. Cross-dialect NULLS FIRST/LAST is not portable (unsupported on MySQL and old SQLite), so the cursor path now pins NULL placement with a portable CASE-based null-rank key shared by both the keyset ORDER BY and the keyset predicate, so the two can no longer disagree on any backend. The rank follows the column's sort direction, so NULLs order as the largest value (last when ascending, first when descending), matching PostgreSQL's default. PostgreSQL result order is therefore unchanged; SQLite and MySQL NULL ordering shifts to align with it. The cursor token format is unchanged (the rank is derived, not encoded) and offset pagination is untouched.
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.
Cursor (keyset) paginated REST list endpoints (
GET /dags/{dag_id}/dagRunsandGET .../taskInstances) silently dropped rows when sorted by a nullable column such asstart_date,end_date,duration, orstate. The keyset predicate and the generatedORDER BYdisagreed on where NULLs sort, so once a page boundary fell on the NULL/non-NULL edge, every row on one side of it was skipped, with no error.This is reachable from the shipping web UI: the Task Instances and Dag Runs lists paginate by cursor and let you sort by clicking a column header, so sorting by Start Date while some queued (not yet started) task instances are present makes rows silently disappear from the grid.
Fix
NULLS FIRST/LASTis not portable (unsupported on MySQL and older SQLite), so the cursor path pins NULL placement with a portableCASE-based null-rank key shared by both the keysetORDER BYand the keyset predicate, so they can no longer disagree on any backend. The rank follows the column's sort direction, so NULLs sort as the largest value (last when ascending, first when descending), matching PostgreSQL's default. PostgreSQL result ordering is therefore unchanged; SQLite and MySQL NULL ordering shifts to align with PostgreSQL.The
CASEinORDER BYmeans the sort on a nullable column cannot use a plain column index. This is the cost of cross-backend portability; a PostgreSQL-onlyNULLS LASTfast path could be a future optimization.Tests
taskInstancesanddagRuns): paginating by a nullable column returns every row only after the fix.closes: #68858
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8) following the guidelines