Fixed: Preserve ordering when executing batched paging queries. (#7994)

ChilliCream · Feb 5, 2025 · 6c393d3 · 6c393d3
1 parent b8fcc1c
commit 6c393d3
Show file tree

Hide file tree

Showing 18 changed files with 191 additions and 30 deletions.
diff --git a/src/GreenDonut/src/GreenDonut.Data.EntityFramework/Expressions/ExpressionHelpers.cs b/src/GreenDonut/src/GreenDonut.Data.EntityFramework/Expressions/ExpressionHelpers.cs
@@ -441,11 +441,16 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
             // we are not interested in nested order by calls
             if (node.Method.DeclaringType == typeof(Queryable) && node.Method.Name == nameof(Queryable.Select))
             {
+                // We first visit our parent. When we visit an expression
+                // like "OrderBy().Select()", we want to visit the OrderBy first.
+                var source = Visit(node.Arguments[0]);
+
                 var previousState = _insideSelectProjection;
                 _insideSelectProjection = true;
-                var result = base.VisitMethodCall(node);
+                var projection = Visit(node.Arguments[1]);
                 _insideSelectProjection = previousState;
-                return result;
+
+                return node.Update(null, [source, projection]);
             }
 
             if (node.Method.DeclaringType == typeof(Queryable)

diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/IntegrationTests.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/IntegrationTests.cs
@@ -211,6 +211,34 @@ ... on Brand {
         MatchSnapshot(result, interceptor);
     }
 
+    [Fact]
+    public async Task Query_Products_First_2_And_Brand()
+    {
+        // arrange
+        using var interceptor = new TestQueryInterceptor();
+
+        // act
+        var result = await ExecuteAsync(
+            """
+            {
+                products(first: 2) {
+                    nodes {
+                        name
+                        brand {
+                            name
+                        }
+                    }
+                }
+            }
+
+            """,
+            interceptor);
+
+        // assert
+        MatchSnapshot(result, interceptor);
+    }
+
+
     private static ServiceProvider CreateServer(string connectionString)
     {
         var services = new ServiceCollection();

diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Services/ProductService.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Services/ProductService.cs
@@ -1,9 +1,10 @@
 using GreenDonut.Data;
+using HotChocolate.Data.Data;
 using HotChocolate.Data.Models;
 
 namespace HotChocolate.Data.Services;
 
-public class ProductService(IProductBatchingContext batchingContext)
+public class ProductService(CatalogContext context, IProductBatchingContext batchingContext)
 {
     public async Task<Product?> GetProductByIdAsync(
         int id,
@@ -13,6 +14,12 @@ public class ProductService(IProductBatchingContext batchingContext)
             .With(query)
             .LoadAsync(id, cancellationToken);
 
+    public async Task<Page<Product>> GetProductsAsync(
+        PagingArguments pagingArgs,
+        QueryContext<Product>? query = null,
+        CancellationToken cancellationToken = default)
+        => await context.Products.With(query, DefaultOrder).ToPageAsync(pagingArgs, cancellationToken);
+
     public async Task<Page<Product>> GetProductsByBrandAsync(
         int brandId,
         PagingArguments pagingArgs,
@@ -22,4 +29,7 @@ public async Task<Page<Product>> GetProductsByBrandAsync(
             .With(pagingArgs, query)
             .LoadAsync(brandId, cancellationToken)
             ?? Page<Product>.Empty;
+
+    private static SortDefinition<Product> DefaultOrder(SortDefinition<Product> sort)
+        => sort.IfEmpty(o => o.AddDescending(t => t.Name)).AddAscending(t => t.Id);
 }
diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Brands/BrandNode.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Brands/BrandNode.cs
@@ -4,7 +4,7 @@
 using HotChocolate.Types;
 using HotChocolate.Types.Pagination;
 
-namespace HotChocolate.Data.Types;
+namespace HotChocolate.Data.Types.Brands;
 
 [ObjectType<Brand>]
 public static partial class BrandNode

diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Brands/BrandQueries.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Brands/BrandQueries.cs
@@ -5,7 +5,7 @@
 using HotChocolate.Types.Pagination;
 using HotChocolate.Types.Relay;
 
-namespace HotChocolate.Data.Types;
+namespace HotChocolate.Data.Types.Brands;
 
 [QueryType]
 public static class BrandQueries
@@ -20,7 +20,7 @@ public static async Task<Connection<Brand>> GetBrandsAsync(
         => await brandService.GetBrandsAsync(pagingArgs, query, cancellationToken).ToConnectionAsync();
 
     [NodeResolver]
-    public static async Task<Brand?> GetBrandAsync(
+    public static async Task<Brand?> GetBrandByIdAsync(
         int id,
         QueryContext<Brand> query,
         BrandService brandService,

diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Brands/BrandSortInputType.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Brands/BrandSortInputType.cs
@@ -1,7 +1,7 @@
 using HotChocolate.Data.Models;
 using HotChocolate.Data.Sorting;
 
-namespace HotChocolate.Data.Types;
+namespace HotChocolate.Data.Types.Brands;
 
 public sealed class BrandSortInputType : SortInputType<Brand>
 {

diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Products/ProductNode.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Products/ProductNode.cs
@@ -1,18 +1,21 @@
 using GreenDonut.Data;
 using HotChocolate.Data.Models;
 using HotChocolate.Data.Services;
+using HotChocolate.Execution.Processing;
 using HotChocolate.Types;
 using HotChocolate.Types.Relay;
 
-namespace HotChocolate.Data.Types;
+namespace HotChocolate.Data.Types.Products;
 
 [ObjectType<Product>]
 public static partial class ProductNode
 {
+    // [BindMember(nameof(Product.Brand))]
     public static async Task<Brand?> GetBrandAsync(
-        [Parent(requires: nameof(Product.BrandId))]  Product product,
-        BrandService brandService,
+        [Parent(requires: nameof(Product.BrandId))] Product product,
         QueryContext<Brand> query,
+        ISelection selection,
+        BrandService brandService,
         CancellationToken cancellationToken)
         => await brandService.GetBrandByIdAsync(product.BrandId, query, cancellationToken);
 

diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Products/ProductQueries.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Products/ProductQueries.cs
@@ -0,0 +1,22 @@
+using GreenDonut.Data;
+using HotChocolate.Data.Models;
+using HotChocolate.Data.Services;
+using HotChocolate.Types;
+using HotChocolate.Types.Pagination;
+
+namespace HotChocolate.Data.Types.Products;
+
+
+[QueryType]
+public static partial class ProductQueries
+{
+    [UsePaging]
+    [UseFiltering]
+    [UseSorting]
+    public static async Task<Connection<Product>> GetProductsAsync(
+        PagingArguments pagingArgs,
+        QueryContext<Product> query,
+        ProductService productService,
+        CancellationToken cancellationToken)
+        => await productService.GetProductsAsync(pagingArgs, query, cancellationToken).ToConnectionAsync();
+}
diff --git a/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Products/ProductSortInputType.cs b/src/HotChocolate/Data/test/Data.PostgreSQL.Tests/Types/Products/ProductSortInputType.cs
@@ -1,7 +1,7 @@
 using HotChocolate.Data.Models;
 using HotChocolate.Data.Sorting;
 
-namespace HotChocolate.Data.Types;
+namespace HotChocolate.Data.Types.Products;
 
 public sealed class ProductSortInputType : SortInputType<Product>
 {

diff --git a/...olate/Data/test/Data.PostgreSQL.Tests/__snapshots__/IntegrationTests.CreateSchema.graphql b/...olate/Data/test/Data.PostgreSQL.Tests/__snapshots__/IntegrationTests.CreateSchema.graphql
@@ -90,8 +90,9 @@ type Query {
   node("ID of the object." id: ID!): Node @cost(weight: "10")
   "Lookup nodes by a list of IDs."
   nodes("The list of node IDs." ids: [ID!]!): [Node]! @cost(weight: "10")
+  products("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String where: ProductFilterInput @cost(weight: "10") order: [ProductSortInput!] @cost(weight: "10")): ProductsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], slicingArgumentDefaultValue: 10, sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false) @cost(weight: "10")
   brands("Returns the first _n_ elements from the list." first: Int "Returns the elements in the list that come after the specified cursor." after: String "Returns the last _n_ elements from the list." last: Int "Returns the elements in the list that come before the specified cursor." before: String where: BrandFilterInput @cost(weight: "10")): BrandsConnection @listSize(assumedSize: 50, slicingArguments: [ "first", "last" ], slicingArgumentDefaultValue: 10, sizedFields: [ "edges", "nodes" ], requireOneSlicingArgument: false) @cost(weight: "10")
-  brand(id: ID!): Brand @cost(weight: "10")
+  brandById(id: ID!): Brand @cost(weight: "10")
 }
 
 input BooleanOperationFilterInput {

diff --git a/...sts/__snapshots__/IntegrationTests.Query_Brands_First_2_And_Products_First_2.md b/...sts/__snapshots__/IntegrationTests.Query_Brands_First_2_And_Products_First_2.md
@@ -75,6 +75,6 @@ LEFT JOIN (
     ) AS p2
     WHERE p2.row <= 3
 ) AS p3 ON p1."BrandId" = p3."BrandId"
-ORDER BY p1."BrandId"
+ORDER BY p1."BrandId", p3."BrandId", p3."Id"
 ```
 
diff --git a/...shots__/IntegrationTests.Query_Brands_First_2_And_Products_First_2_Name_Desc.md b/...shots__/IntegrationTests.Query_Brands_First_2_And_Products_First_2_Name_Desc.md
@@ -13,8 +13,8 @@
           "products": {
             "nodes": [
               {
-                "id": "UHJvZHVjdDoxMg==",
-                "name": "Powder Pro Snowboard"
+                "id": "UHJvZHVjdDo0OA==",
+                "name": "Trailblazer 45L Backpack"
               },
               {
                 "id": "UHJvZHVjdDoyMw==",
@@ -69,12 +69,12 @@ FROM (
 LEFT JOIN (
     SELECT p2."Id", p2."Name", p2."BrandId"
     FROM (
-        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Id") AS row
+        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Name" DESC, p0."Id") AS row
         FROM "Products" AS p0
         WHERE p0."BrandId" = ANY (@__brandIds_0)
     ) AS p2
     WHERE p2.row <= 3
 ) AS p3 ON p1."BrandId" = p3."BrandId"
-ORDER BY p1."BrandId"
+ORDER BY p1."BrandId", p3."BrandId", p3."Name" DESC, p3."Id"
 ```
 
diff --git a/...egrationTests.Query_Brands_First_2_And_Products_First_2_Name_Desc_Brand_Name.md b/...egrationTests.Query_Brands_First_2_And_Products_First_2_Name_Desc_Brand_Name.md
@@ -12,8 +12,8 @@
           "products": {
             "nodes": [
               {
-                "id": "UHJvZHVjdDoxMg==",
-                "name": "Powder Pro Snowboard",
+                "id": "UHJvZHVjdDo0OA==",
+                "name": "Trailblazer 45L Backpack",
                 "brand": {
                   "name": "Zephyr"
                 }
@@ -79,13 +79,13 @@ FROM (
 LEFT JOIN (
     SELECT p2."Id", p2."Name", p2."BrandId"
     FROM (
-        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Id") AS row
+        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Name" DESC, p0."Id") AS row
         FROM "Products" AS p0
         WHERE p0."BrandId" = ANY (@__brandIds_0)
     ) AS p2
     WHERE p2.row <= 3
 ) AS p3 ON p1."BrandId" = p3."BrandId"
-ORDER BY p1."BrandId"
+ORDER BY p1."BrandId", p3."BrandId", p3."Name" DESC, p3."Id"
 ```
 
 ## Query 3

diff --git a/...ests.Query_Brands_First_2_And_Products_First_2_Name_Desc_Brand_Name__net_8_0.md b/...ests.Query_Brands_First_2_And_Products_First_2_Name_Desc_Brand_Name__net_8_0.md
@@ -12,8 +12,8 @@
           "products": {
             "nodes": [
               {
-                "id": "UHJvZHVjdDoxMg==",
-                "name": "Powder Pro Snowboard",
+                "id": "UHJvZHVjdDo0OA==",
+                "name": "Trailblazer 45L Backpack",
                 "brand": {
                   "name": "Zephyr"
                 }
@@ -79,13 +79,13 @@ FROM (
 LEFT JOIN (
     SELECT t1."Id", t1."Name", t1."BrandId"
     FROM (
-        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Id") AS row
+        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Name" DESC, p0."Id") AS row
         FROM "Products" AS p0
         WHERE p0."BrandId" = ANY (@__brandIds_0)
     ) AS t1
     WHERE t1.row <= 3
 ) AS t0 ON t."BrandId" = t0."BrandId"
-ORDER BY t."BrandId"
+ORDER BY t."BrandId", t0."BrandId", t0."Name" DESC, t0."Id"
 ```
 
 ## Query 3

diff --git a/...ntegrationTests.Query_Brands_First_2_And_Products_First_2_Name_Desc__net_8_0.md b/...ntegrationTests.Query_Brands_First_2_And_Products_First_2_Name_Desc__net_8_0.md
@@ -13,8 +13,8 @@
           "products": {
             "nodes": [
               {
-                "id": "UHJvZHVjdDoxMg==",
-                "name": "Powder Pro Snowboard"
+                "id": "UHJvZHVjdDo0OA==",
+                "name": "Trailblazer 45L Backpack"
               },
               {
                 "id": "UHJvZHVjdDoyMw==",
@@ -69,12 +69,12 @@ FROM (
 LEFT JOIN (
     SELECT t1."Id", t1."Name", t1."BrandId"
     FROM (
-        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Id") AS row
+        SELECT p0."Id", p0."Name", p0."BrandId", ROW_NUMBER() OVER(PARTITION BY p0."BrandId" ORDER BY p0."Name" DESC, p0."Id") AS row
         FROM "Products" AS p0
         WHERE p0."BrandId" = ANY (@__brandIds_0)
     ) AS t1
     WHERE t1.row <= 3
 ) AS t0 ON t."BrandId" = t0."BrandId"
-ORDER BY t."BrandId"
+ORDER BY t."BrandId", t0."BrandId", t0."Name" DESC, t0."Id"
 ```
 
diff --git a/...pshots__/IntegrationTests.Query_Brands_First_2_And_Products_First_2__net_8_0.md b/...pshots__/IntegrationTests.Query_Brands_First_2_And_Products_First_2__net_8_0.md
@@ -75,6 +75,6 @@ LEFT JOIN (
     ) AS t1
     WHERE t1.row <= 3
 ) AS t0 ON t."BrandId" = t0."BrandId"
-ORDER BY t."BrandId"
+ORDER BY t."BrandId", t0."BrandId", t0."Id"
 ```
 
diff --git a/...greSQL.Tests/__snapshots__/IntegrationTests.Query_Products_First_2_And_Brand.md b/...greSQL.Tests/__snapshots__/IntegrationTests.Query_Products_First_2_And_Brand.md
@@ -0,0 +1,46 @@
+# Query_Products_First_2_And_Brand
+
+## Result
+
+```json
+{
+  "data": {
+    "products": {
+      "nodes": [
+        {
+          "name": "Zero Gravity Ski Goggles",
+          "brand": {
+            "name": "Gravitator"
+          }
+        },
+        {
+          "name": "Zenith Cycling Jersey",
+          "brand": {
+            "name": "B&R"
+          }
+        }
+      ]
+    }
+  }
+}
+```
+
+## Query 1
+
+```sql
+-- @__p_0='3'
+SELECT p."Name", p."BrandId", p."Id"
+FROM "Products" AS p
+ORDER BY p."Name" DESC, p."Id"
+LIMIT @__p_0
+```
+
+## Query 2
+
+```sql
+-- @__ids_0={ '2', '5' } (DbType = Object)
+SELECT b."Id", b."Name"
+FROM "Brands" AS b
+WHERE b."Id" = ANY (@__ids_0)
+```
+