Handle nullable navigations in keyset pagination WHERE clause

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

@@ -154,6 +154,7 @@ private static Expression BuildEqualToKeyExpr(
         Expression cursorExpr)
     {
         var keyExpr = ReplaceParameter(cursorKey.Expression, parameter);
+        keyExpr = LiftValueTypeIfNullable(keyExpr, keyIsNullable);
 
         // Access the value of the key if it is a nullable value type.
         var keyValueExpr = cursorKey.Expression.ReturnType.IsValueType && keyIsNullable
@@ -197,6 +198,7 @@ private static Expression BuildGreaterThanKeyExpr(
         Expression cursorExpr)
     {
         var keyExpr = ReplaceParameter(cursorKey.Expression, parameter);
+        keyExpr = LiftValueTypeIfNullable(keyExpr, keyIsNullable);
 
         // Access the value of the key if it is a nullable value type.
         var keyValueExpr =
@@ -255,6 +257,7 @@ private static Expression BuildLessThanKeyExpr(
         Expression cursorExpr)
     {
         var keyExpr = ReplaceParameter(cursorKey.Expression, parameter);
+        keyExpr = LiftValueTypeIfNullable(keyExpr, keyIsNullable);
 
         // Access the value of the key if it is a nullable value type.
         var keyValueExpr =
@@ -306,38 +309,37 @@ private static Expression BuildLessThanKeyExpr(
 
     private static bool IsNullable(LambdaExpression expression)
     {
-        if (expression.ReturnType.IsValueType)
+        // A nullable value-type return (for example an explicit cast to int?).
+        if (expression.ReturnType.IsValueType
+            && Nullable.GetUnderlyingType(expression.ReturnType) is not null)
         {
-            return Nullable.GetUnderlyingType(expression.ReturnType) is not null;
+            return true;
         }
 
-        var member = expression.Body switch
+        var current = StripConvert(expression.Body);
+
+        // (a ?? b) is null only when the fallback b is null.
+        if (current is BinaryExpression { NodeType: ExpressionType.Coalesce, Right: var right })
         {
-            MemberExpression { Member: PropertyInfo or FieldInfo } m => m.Member,
-            BinaryExpression
-            {
-                NodeType: ExpressionType.Coalesce,
-                Right: MemberExpression { Member: PropertyInfo or FieldInfo } m
-            } => m.Member,
-            _ => null
-        };
+            current = StripConvert(right);
+        }
 
-        if (member is not null)
+        // The key value is nullable if the leaf or any intermediate navigation along
+        // the member-access path is nullable (for example x.Meter.Id is null when
+        // x.Meter is null).
+        while (current is MemberExpression { Member: PropertyInfo or FieldInfo } member)
         {
-            var state = member switch
+            if (IsMemberNullable(member))
             {
-                PropertyInfo p => GetNullabilityInfoState(p),
-                FieldInfo f => GetNullabilityInfoState(f),
-                _ => throw new InvalidOperationException()
-            };
+                return true;
+            }
 
-            return state switch
+            if (member.Expression is null)
             {
-                NullabilityState.Nullable => true,
-                // Unknown means the assembly was compiled without NRT annotations;
-                // treat as non-nullable (safe default).
-                _ => false
-            };
+                break;
+            }
+
+            current = StripConvert(member.Expression);
         }
 
         // For computed key expressions (method calls, concatenation, etc.) we cannot inspect
@@ -623,6 +625,53 @@ private static Expression CreateAndConvertParameter<T>(T value)
         return lambda.Body;
     }
 
+    /// <summary>
+    /// Determines whether a member access can yield <c>null</c>: a value-type member
+    /// only when it is <see cref="Nullable{T}"/>, and a reference-type member according
+    /// to its nullable reference type annotation.
+    /// </summary>
+    private static bool IsMemberNullable(MemberExpression member)
+    {
+        if (member.Type.IsValueType)
+        {
+            return Nullable.GetUnderlyingType(member.Type) is not null;
+        }
+
+        // Unknown means the assembly was compiled without NRT annotations;
+        // treat as non-nullable (safe default).
+        return member.Member switch
+        {
+            PropertyInfo p => GetNullabilityInfoState(p) == NullabilityState.Nullable,
+            FieldInfo f => GetNullabilityInfoState(f) == NullabilityState.Nullable,
+            _ => false
+        };
+    }
+
+    /// <summary>
+    /// Removes any <see cref="ExpressionType.Convert"/> or
+    /// <see cref="ExpressionType.ConvertChecked"/> wrappers from an expression and
+    /// returns the underlying operand.
+    /// </summary>
+    private static Expression StripConvert(Expression expression)
+        => expression is UnaryExpression
+        {
+            NodeType: ExpressionType.Convert or ExpressionType.ConvertChecked
+        } unary
+            ? StripConvert(unary.Operand)
+            : expression;
+
+    /// <summary>
+    /// Lifts a non-nullable value-type key (for example <c>int</c> for
+    /// <c>x.Meter.Id</c>) to its <see cref="Nullable{T}"/> form when the key is
+    /// nullable because of an intermediate navigation, so the key can be compared
+    /// against <c>null</c>. Keys that are already nullable or are reference types
+    /// are returned unchanged.
+    /// </summary>
+    private static Expression LiftValueTypeIfNullable(Expression keyExpr, bool keyIsNullable)
+        => keyIsNullable && keyExpr.Type.IsValueType && Nullable.GetUnderlyingType(keyExpr.Type) is null
+            ? Expression.Convert(keyExpr, typeof(Nullable<>).MakeGenericType(keyExpr.Type))
+            : keyExpr;
+
     private static Expression BuildEqualComparison(
         CursorKey cursorKey,
         Expression keyExpr,

src/GreenDonut/test/GreenDonut.Data.EntityFramework.Tests/PagingHelperPostgreSqlNullableTests.cs

@@ -313,6 +313,62 @@ await context.Records
             (await Assert.ThrowsAsync<InvalidOperationException>(Act)).Message);
     }
 
+    [Fact]
+    public async Task Paging_NullableNavigation_ValueTypeLeaf_Pages_Across_Null_Boundary()
+    {
+        // arrange
+        var connectionString = CreateConnectionString();
+        await SeedItemsAsync(connectionString);
+        await using var context = new NullableTestsContext(Provider.PostgreSql, connectionString);
+
+        // act
+        // Sort by a non-nullable value-type leaf (Detail.Number) behind a nullable
+        // navigation; the second page is requested after a null-navigation boundary.
+        var arguments = new PagingArguments(2) { NullOrdering = NullOrdering.NativeNullsLast };
+        var page1 = await context.Items
+            .OrderBy(x => x.Detail!.Number)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        arguments = arguments with { After = page1.CreateEndCursor() };
+        var page2 = await context.Items
+            .OrderBy(x => x.Detail!.Number)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        // assert
+        Assert.Equal([1, 2], page1.Select(x => x.Id));
+        Assert.Equal([3, 4], page2.Select(x => x.Id));
+    }
+
+    [Fact]
+    public async Task Paging_NullableNavigation_ReferenceLeaf_Pages_Across_Null_Boundary()
+    {
+        // arrange
+        var connectionString = CreateConnectionString();
+        await SeedItemsAsync(connectionString);
+        await using var context = new NullableTestsContext(Provider.PostgreSql, connectionString);
+
+        // act
+        // Sort by a non-nullable reference leaf (Detail.Name) behind a nullable
+        // navigation; the second page is requested after a null-navigation boundary.
+        var arguments = new PagingArguments(2) { NullOrdering = NullOrdering.NativeNullsLast };
+        var page1 = await context.Items
+            .OrderBy(x => x.Detail!.Name)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        arguments = arguments with { After = page1.CreateEndCursor() };
+        var page2 = await context.Items
+            .OrderBy(x => x.Detail!.Name)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        // assert
+        Assert.Equal([1, 2], page1.Select(x => x.Id));
+        Assert.Equal([3, 4], page2.Select(x => x.Id));
+    }
+
     private static async Task SeedAsync(string connectionString)
     {
         await using var context = new NullableTestsContext(Provider.PostgreSql, connectionString);
@@ -359,4 +415,20 @@ private static async Task SeedAsync(string connectionString)
 
         await context.SaveChangesAsync();
     }
+
+    private static async Task SeedItemsAsync(string connectionString)
+    {
+        await using var context = new NullableTestsContext(Provider.PostgreSql, connectionString);
+        await context.Database.EnsureCreatedAsync();
+
+        // Only the first item has a navigation; the rest are null and sort last,
+        // so paging crosses a null-navigation boundary between page 1 and page 2.
+        context.Items.AddRange(
+            new Item { Id = 1, Detail = new Detail { Number = 10, Name = "a" } },
+            new Item { Id = 2 },
+            new Item { Id = 3 },
+            new Item { Id = 4 });
+
+        await context.SaveChangesAsync();
+    }
 }

src/GreenDonut/test/GreenDonut.Data.EntityFramework.Tests/PagingHelperSqlServerNullableTests.cs

@@ -266,6 +266,64 @@ public async Task Paging_NullableReference_Descending_Cursor_Value_Null()
         snapshot.MatchMarkdownSnapshot();
     }
 
+    [Fact]
+    public async Task Paging_NullableNavigation_ValueTypeLeaf_Pages_Across_Null_Boundary()
+    {
+        // arrange
+        var connectionString = CreateConnectionString();
+        await SeedItemsAsync(connectionString);
+        await using var context = new NullableTestsContext(Provider.SqlServer, connectionString);
+
+        // act
+        // Sort by a non-nullable value-type leaf (Detail.Number) behind a nullable
+        // navigation; with nulls first the second page is requested after a
+        // null-navigation boundary.
+        var arguments = new PagingArguments(2) { NullOrdering = NullOrdering.NativeNullsFirst };
+        var page1 = await context.Items
+            .OrderBy(x => x.Detail!.Number)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        arguments = arguments with { After = page1.CreateEndCursor() };
+        var page2 = await context.Items
+            .OrderBy(x => x.Detail!.Number)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        // assert
+        Assert.Equal([2, 3], page1.Select(x => x.Id));
+        Assert.Equal([4, 1], page2.Select(x => x.Id));
+    }
+
+    [Fact]
+    public async Task Paging_NullableNavigation_ReferenceLeaf_Pages_Across_Null_Boundary()
+    {
+        // arrange
+        var connectionString = CreateConnectionString();
+        await SeedItemsAsync(connectionString);
+        await using var context = new NullableTestsContext(Provider.SqlServer, connectionString);
+
+        // act
+        // Sort by a non-nullable reference leaf (Detail.Name) behind a nullable
+        // navigation; with nulls first the second page is requested after a
+        // null-navigation boundary.
+        var arguments = new PagingArguments(2) { NullOrdering = NullOrdering.NativeNullsFirst };
+        var page1 = await context.Items
+            .OrderBy(x => x.Detail!.Name)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        arguments = arguments with { After = page1.CreateEndCursor() };
+        var page2 = await context.Items
+            .OrderBy(x => x.Detail!.Name)
+            .ThenBy(x => x.Id)
+            .ToPageAsync(arguments, Xunit.TestContext.Current.CancellationToken);
+
+        // assert
+        Assert.Equal([2, 3], page1.Select(x => x.Id));
+        Assert.Equal([4, 1], page2.Select(x => x.Id));
+    }
+
     private static async Task SeedAsync(string connectionString)
     {
         await using var context = new NullableTestsContext(Provider.SqlServer, connectionString);
@@ -312,4 +370,20 @@ private static async Task SeedAsync(string connectionString)
 
         await context.SaveChangesAsync();
     }
+
+    private static async Task SeedItemsAsync(string connectionString)
+    {
+        await using var context = new NullableTestsContext(Provider.SqlServer, connectionString);
+        await context.Database.EnsureCreatedAsync();
+
+        // Only the first item has a navigation; the rest are null and sort first,
+        // so paging crosses a null-navigation boundary between page 1 and page 2.
+        context.Items.AddRange(
+            new Item { Id = 1, Detail = new Detail { Number = 10, Name = "a" } },
+            new Item { Id = 2 },
+            new Item { Id = 3 },
+            new Item { Id = 4 });
+
+        await context.SaveChangesAsync();
+    }
 }

src/GreenDonut/test/GreenDonut.Data.EntityFramework.Tests/TestContext/NullableTestsContext.cs

@@ -1,3 +1,4 @@
+using System.ComponentModel.DataAnnotations.Schema;
 using Microsoft.EntityFrameworkCore;
 
 namespace GreenDonut.Data.TestContext;
@@ -20,6 +21,8 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
     }
 
     public DbSet<Record> Records { get; set; } = null!;
+
+    public DbSet<Item> Items { get; set; } = null!;
 }
 
 public class Record
@@ -30,6 +33,25 @@ public class Record
     public string? String { get; set; }
 }
 
+public class Item
+{
+    [DatabaseGenerated(DatabaseGeneratedOption.None)]
+    public int Id { get; set; }
+
+    public int? DetailId { get; set; }
+
+    public Detail? Detail { get; set; }
+}
+
+public class Detail
+{
+    public int Id { get; set; }
+
+    public int Number { get; set; }
+
+    public string Name { get; set; } = null!;
+}
+
 public enum Provider
 {
     PostgreSql,