Skip to content

Fixed relative cursors to handle nullable types. #8230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sleepertassly
Copy link

@sleepertassly sleepertassly commented Apr 6, 2025

🧭 Paging Cursor Enhancements: Nullable Keys and Null Ordering Support

Summary

This PR introduces enhancements to the paging logic to correctly handle sorting and filtering when cursor keys are nullable. The main change is that the WHERE condition used in pagination must now adapt based on three factors:

  1. Whether the cursor key is nullable.
  2. Whether the actual cursor value is null or not.
  3. Whether the database sorts null values first or last.

🔍 Why This Is Needed

When performing cursor-based pagination on fields that can be null, it's essential to construct the WHERE clause correctly. Incorrect assumptions about null ordering or the presence of null values can result in missing or duplicated records across pages.


🔧 Implementation Details

To support this behavior, the following changes were made:

  • Cursor Key Metadata

    • Cursor keys now include a new property (IsNullable) to indicate whether they are nullable.
  • Null Ordering in Cursor

    • Cursors now carry a NullsFirst flag to indicate how null values are ordered.
    • For subsequent pages, this flag is inherited from the previous cursor.
    • For the first(last) page, the flag is calculated based on whether the first(last) item contains a null value:
      • If a null is found → we can determine the null ordering: nulls first(nulls last).
      • If not → we assume the opposite ordering.
        • If this assumption is incorrect, it does not affect correctness, because it implies there are no null values in the dataset, making null ordering irrelevant.

The concept of handling nullable types in relative cursor-based pagination is inspired by [this StackOverflow discussion].


✅ Benefits

  • Correct pagination over nullable fields.
  • Eliminates edge cases where null records might be skipped or duplicated.
  • Supports both nulls first and nulls last configurations in the database.

⚠️ Side Effects / Limitations

If our assumption about null ordering is incorrect, secondary ordering keys may appear to be sorted in reverse (opposite to the database's actual null handling behavior), particularly when paginating backward from the last page.

This only affects the visual or logical order of null values relative to each other and does not break pagination—all items will still be included in the correct pages.

…ly fails to highlight a flaw in the implementation. The algorithm expects that null values are smaller than any other value when ordering.
@CLAassistant
Copy link

CLAassistant commented Apr 6, 2025

CLA assistant check
All committers have signed the CLA.

@michaelstaib
Copy link
Member

The PR looks very good.

I'm planning to add this extra ORDER BY condition before all nullable cursor key, but that hasn't been implemented yet.
I'm still thinking about the performance impact. The computed orderby expression OrderBy(t => t.FoundedDate == null) which moves all null values to the beginning can significantly slow down sorting, but I believe it's still faster than OFFSET/LIMIT pagination.

This is another aspect we were also looking into ... especially as with postgres null order can be configured to be at the beginning of a set or at the and of a set.

I will copy over some new tests that we had in our work branch for nullable cursor values. Ping me on slack once you have solved the null ordering issue and we can see to get this one merged.

@michaelstaib
Copy link
Member

Ping me once you need help, want another review or are ready to merge.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/GreenDonut/src/GreenDonut.Data.EntityFramework/Expressions/ExpressionHelpers.cs:73

  • The variable 'forward' is not defined in the current scope; please define it or pass it as a parameter to ensure correct directional logic in the expression building.
var greaterThan = forward ? key.Direction == CursorKeyDirection.Ascending : key.Direction == CursorKeyDirection.Descending;

@michaelstaib michaelstaib requested a review from Copilot May 13, 2025 05:56
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances cursor-based pagination by introducing nullable key support and properly handling null ordering in the underlying SQL operations. Key changes include updating the serializer interfaces and implementations to support nullable types, revising the page info and formatter to propagate a new nulls-first flag, and modifying the expression builders and query extensions to correctly apply these new semantics.

Reviewed Changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/LongCursorKeySerializer.cs Updated to support nullable long values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/IntCursorKeySerializer.cs Updated to support nullable int values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/ICursorKeySerializer.cs Modified interface to include IsNullable and use CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/GuidCursorKeySerializer.cs Updated to support nullable Guid values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/FloatCursorKeySerializer.cs Updated to support nullable float values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/DoubleCursorKeySerializer.cs Updated to support nullable double values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/DecimalCursorKeySerializer.cs Updated to support nullable decimal values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/DateTimeOffsetCursorKeySerializer.cs Updated to support nullable DateTimeOffset values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/DateTimeCursorKeySerializer.cs Updated to support nullable DateTime values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/DateOnlyCursorKeySerializer.cs Updated to support nullable DateOnly values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/CompareToResolver.cs Altered to return a CursorKeyCompareMethod with new array initialization syntax.
src/GreenDonut/src/GreenDonut.Data/Cursors/Serializers/BoolCursorKeySerializer.cs Updated to support nullable bool values and return CursorKeyCompareMethod.
src/GreenDonut/src/GreenDonut.Data/Cursors/CursorParser.cs Adjusted parsing to extract the nulls-first flag from formatted page info.
src/GreenDonut/src/GreenDonut.Data/Cursors/CursorPageInfo.cs Modified to include a nulls-first property throughout construction and deconstruction.
src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKeyCompareMethod.cs New class encapsulating the compare method and its owner type.
src/GreenDonut/src/GreenDonut.Data/Cursors/CursorKey.cs Changed to use CursorKeyCompareMethod and added an IsNullable property.
src/GreenDonut/src/GreenDonut.Data/Cursors/CursorFormatter.cs Updated to format the new nulls-first flag in cursor strings.
src/GreenDonut/src/GreenDonut.Data/Cursors/Cursor.cs Extended cursor record to carry the nulls-first flag.
src/GreenDonut/src/GreenDonut.Data.EntityFramework/Extensions/PagingQueryableExtensions.cs Adjusted page creation logic to propagate and compute the nulls-first flag.
src/GreenDonut/src/GreenDonut.Data.EntityFramework/Expressions/ExpressionHelpers.cs Revised expression construction logic to account for nullable keys and null ordering.
Comments suppressed due to low confidence (1)

src/GreenDonut/src/GreenDonut.Data.EntityFramework/Expressions/ExpressionHelpers.cs:73

  • The variable 'forward' is used here without a prior declaration, which will result in a runtime error. Please declare and initialize 'forward' appropriately or pass it as a parameter to determine the direction.
var greaterThan = forward ? key.Direction == CursorKeyDirection.Ascending : key.Direction == CursorKeyDirection.Descending;

@glen-84
Copy link
Collaborator

glen-84 commented May 23, 2025

Hey @sleepertassly – just checking in to see if you're planning on getting this PR over the line?

@sleepertassly
Copy link
Author

Hi @glen-84 — yes, I'm working on it. I estimate it will take about 5–6 more hours to finish, but I’m not sure when I’ll have that much free time this week or next.

For the cursor to work properly across all databases, we need additional information: whether the database places nulls at the beginning or end of the sorted list.
My initial idea — to determine this behavior from the first page of results — didn’t work out.

I’m now working on a solution where we explicitly force nulls to appear at either the beginning or end by adding an extra ordering condition for all nullable fields.
For example: ORDER BY (name IS NULL) ASC, name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants