Remove redundant selectors in QueryLayer, only sort by ID when pagination enabled (#1735)

Author: bkoelman

src/JsonApiDotNetCore/Queries/QueryLayer.cs

@@ -11,6 +11,8 @@ namespace JsonApiDotNetCore.Queries;
 [PublicAPI]
 public sealed class QueryLayer
 {
+    internal bool IsEmpty => Filter == null && Sort == null && Pagination?.PageSize == null && (Selection == null || Selection.IsEmpty);
+
     public ResourceType ResourceType { get; }
 
     public IncludeExpression? Include { get; set; }

src/JsonApiDotNetCore/Queries/QueryLayerComposer.cs

@@ -173,13 +173,20 @@ private QueryLayer ComposeTopLayer(ImmutableArray<ExpressionInScope> constraints
         _paginationContext.PageSize = topPagination.PageSize;
         _paginationContext.PageNumber = topPagination.PageNumber;
 
-        return new QueryLayer(resourceType)
+        var topLayer = new QueryLayer(resourceType)
         {
             Filter = GetFilter(expressionsInTopScope, resourceType),
             Sort = GetSort(expressionsInTopScope, resourceType),
             Pagination = topPagination,
             Selection = GetSelectionForSparseAttributeSet(resourceType)
         };
+
+        if (topLayer is { Pagination.PageSize: not null, Sort: null })
+        {
+            topLayer.Sort = CreateSortById(resourceType);
+        }
+
+        return topLayer;
     }
 
     private IncludeExpression ComposeChildren(QueryLayer topLayer, ImmutableArray<ExpressionInScope> constraints)
@@ -237,17 +244,22 @@ private IImmutableSet<IncludeElementExpression> ProcessIncludeSet(IImmutableSet<
                 ResourceType resourceType = includeElement.Relationship.RightType;
                 bool isToManyRelationship = includeElement.Relationship is HasManyAttribute;
 
-                var child = new QueryLayer(resourceType)
+                var subLayer = new QueryLayer(resourceType)
                 {
                     Filter = isToManyRelationship ? GetFilter(expressionsInCurrentScope, resourceType) : null,
                     Sort = isToManyRelationship ? GetSort(expressionsInCurrentScope, resourceType) : null,
                     Pagination = isToManyRelationship ? GetPagination(expressionsInCurrentScope, resourceType) : null,
                     Selection = GetSelectionForSparseAttributeSet(resourceType)
                 };
 
-                selectors.IncludeRelationship(includeElement.Relationship, child);
+                if (subLayer is { Pagination.PageSize: not null, Sort: null })
+                {
+                    subLayer.Sort = CreateSortById(resourceType);
+                }
 
-                IImmutableSet<IncludeElementExpression> updatedChildren = ProcessIncludeSet(includeElement.Children, child, relationshipChain, constraints);
+                selectors.IncludeRelationship(includeElement.Relationship, subLayer);
+
+                IImmutableSet<IncludeElementExpression> updatedChildren = ProcessIncludeSet(includeElement.Children, subLayer, relationshipChain, constraints);
 
                 if (!ReferenceEquals(includeElement.Children, updatedChildren))
                 {
@@ -256,9 +268,30 @@ private IImmutableSet<IncludeElementExpression> ProcessIncludeSet(IImmutableSet<
             }
         }
 
+        EliminateRedundantSelectors(parentLayer);
+
         return updatesInChildren.Count == 0 ? includeElementsEvaluated : ApplyIncludeElementUpdates(includeElementsEvaluated, updatesInChildren);
     }
 
+    private static void EliminateRedundantSelectors(QueryLayer parentLayer)
+    {
+        if (parentLayer.Selection != null)
+        {
+            foreach ((ResourceType resourceType, FieldSelectors selectors) in parentLayer.Selection.ToArray())
+            {
+                if (selectors.ContainsOnlyRelationships && selectors.Values.OfType<QueryLayer>().All(subLayer => subLayer.IsEmpty))
+                {
+                    parentLayer.Selection.Remove(resourceType);
+                }
+            }
+
+            if (parentLayer.Selection.IsEmpty)
+            {
+                parentLayer.Selection = null;
+            }
+        }
+    }
+
     private static ImmutableHashSet<IncludeElementExpression> ApplyIncludeElementUpdates(IImmutableSet<IncludeElementExpression> includeElements,
         Dictionary<IncludeElementExpression, IImmutableSet<IncludeElementExpression>> updatesInChildren)
     {
@@ -507,23 +540,21 @@ protected virtual IImmutableSet<IncludeElementExpression> GetIncludeElements(IIm
         return _resourceDefinitionAccessor.OnApplyFilter(resourceType, filter);
     }
 
-    protected virtual SortExpression GetSort(IReadOnlyCollection<QueryExpression> expressionsInScope, ResourceType resourceType)
+    protected virtual SortExpression? GetSort(IReadOnlyCollection<QueryExpression> expressionsInScope, ResourceType resourceType)
     {
         ArgumentNullException.ThrowIfNull(expressionsInScope);
         ArgumentNullException.ThrowIfNull(resourceType);
 
         SortExpression? sort = expressionsInScope.OfType<SortExpression>().FirstOrDefault();
 
-        sort = _resourceDefinitionAccessor.OnApplySort(resourceType, sort);
-
-        if (sort == null)
-        {
-            AttrAttribute idAttribute = GetIdAttribute(resourceType);
-            var idAscendingSort = new SortElementExpression(new ResourceFieldChainExpression(idAttribute), true);
-            sort = new SortExpression(ImmutableArray.Create(idAscendingSort));
-        }
+        return _resourceDefinitionAccessor.OnApplySort(resourceType, sort);
+    }
 
-        return sort;
+    private SortExpression CreateSortById(ResourceType resourceType)
+    {
+        AttrAttribute idAttribute = GetIdAttribute(resourceType);
+        var idAscendingSort = new SortElementExpression(new ResourceFieldChainExpression(idAttribute), true);
+        return new SortExpression(ImmutableArray.Create(idAscendingSort));
     }
 
     protected virtual PaginationExpression GetPagination(IReadOnlyCollection<QueryExpression> expressionsInScope, ResourceType resourceType)

test/DapperTests/IntegrationTests/QueryStrings/FilterTests.cs

@@ -78,7 +78,6 @@ SELECT COUNT(*)
                 FROM "Tags" AS t1
                 LEFT JOIN "RgbColors" AS t2 ON t1."Id" = t2."TagId"
                 WHERE t2."Id" = @p1
-                ORDER BY t1."Id"
                 """));
 
             command.Parameters.Should().HaveCount(1);
@@ -144,7 +143,6 @@ SELECT COUNT(*)
                 FROM "Tags" AS t1
                 LEFT JOIN "RgbColors" AS t2 ON t1."Id" = t2."TagId"
                 WHERE t2."Id" IN (@p1, @p2)
-                ORDER BY t1."Id"
                 """));
 
             command.Parameters.Should().HaveCount(2);
@@ -662,7 +660,6 @@ SELECT COUNT(*)
                 SELECT t1."Id", t1."FirstName", t1."LastName"
                 FROM "People" AS t1
                 WHERE (NOT (t1."FirstName" = @p1)) OR (t1."FirstName" IS NULL)
-                ORDER BY t1."Id"
                 """));
 
             command.Parameters.Should().HaveCount(1);
@@ -867,7 +864,6 @@ SELECT COUNT(*)
                 SELECT t1."Id", t1."Name"
                 FROM "Tags" AS t1
                 WHERE (t1."Name" LIKE '%A\%%' ESCAPE '\') OR (t1."Name" LIKE '%A\_%' ESCAPE '\') OR (t1."Name" LIKE '%A\\%' ESCAPE '\') OR (t1."Name" LIKE '%A''%') OR (t1."Name" LIKE '%\%\_\\''%' ESCAPE '\')
-                ORDER BY t1."Id"
                 """));
 
             command.Parameters.Should().BeEmpty();
@@ -1177,7 +1173,6 @@ SELECT 1
                     LEFT JOIN "People" AS t3 ON t2."AssigneeId" = t3."Id"
                     WHERE (t1."Id" = t2."OwnerId") AND (NOT (t3."Id" IS NULL)) AND (t3."FirstName" IS NULL)
                 )
-                ORDER BY t1."Id"
                 """));
 
             command.Parameters.Should().BeEmpty();

test/DapperTests/IntegrationTests/QueryStrings/IncludeTests.cs

@@ -165,7 +165,7 @@ SELECT COUNT(*)
                 INNER JOIN "People" AS t3 ON t1."OwnerId" = t3."Id"
                 LEFT JOIN "TodoItems" AS t4 ON t3."Id" = t4."AssigneeId"
                 LEFT JOIN "Tags" AS t5 ON t1."Id" = t5."TodoItemId"
-                ORDER BY t1."Priority", t1."LastModifiedAt" DESC, t4."Priority", t4."LastModifiedAt" DESC, t5."Id"
+                ORDER BY t1."Priority", t1."LastModifiedAt" DESC, t4."Priority", t4."LastModifiedAt" DESC
                 """));
 
             command.Parameters.Should().BeEmpty();
@@ -231,7 +231,7 @@ SELECT COUNT(*)
                 FROM "TodoItems" AS t1
                 LEFT JOIN "Tags" AS t2 ON t1."Id" = t2."TodoItemId"
                 LEFT JOIN "RgbColors" AS t3 ON t2."Id" = t3."TagId"
-                ORDER BY t1."Priority", t1."LastModifiedAt" DESC, t2."Id"
+                ORDER BY t1."Priority", t1."LastModifiedAt" DESC
                 """));
 
             command.Parameters.Should().BeEmpty();

test/DapperTests/IntegrationTests/QueryStrings/SortTests.cs

@@ -349,7 +349,7 @@ ORDER BY (
                     SELECT COUNT(*)
                     FROM "Tags" AS t3
                     WHERE t2."Id" = t3."TodoItemId"
-                ) DESC, t2."Id", t4."Id"
+                ) DESC, t2."Id"
                 """));
 
             command.Parameters.Should().HaveCount(1);
@@ -415,7 +415,7 @@ SELECT COUNT(*)
                 SELECT t1."Id", t1."FirstName", t1."LastName", t2."Id", t2."CreatedAt", t2."Description", t2."DurationInHours", t2."LastModifiedAt", t2."Priority"
                 FROM "People" AS t1
                 LEFT JOIN "TodoItems" AS t2 ON t1."Id" = t2."OwnerId"
-                ORDER BY t1."Id", (
+                ORDER BY (
                     SELECT COUNT(*)
                     FROM "Tags" AS t3
                     WHERE t2."Id" = t3."TodoItemId"

test/DapperTests/IntegrationTests/QueryStrings/SparseFieldSets.cs

@@ -215,7 +215,6 @@ SELECT COUNT(*)
                 FROM "TodoItems" AS t1
                 LEFT JOIN "Tags" AS t2 ON t1."Id" = t2."TodoItemId"
                 WHERE t1."Id" = @p1
-                ORDER BY t2."Id"
                 """));
 
             command.Parameters.Should().HaveCount(1);
@@ -400,7 +399,6 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
                 FROM "TodoItems" AS t1
                 LEFT JOIN "Tags" AS t2 ON t1."Id" = t2."TodoItemId"
                 WHERE t1."Id" = @p1
-                ORDER BY t2."Id"
                 """));
 
             command.Parameters.Should().HaveCount(1);

test/DapperTests/IntegrationTests/ReadWrite/Relationships/FetchRelationshipTests.cs

@@ -168,7 +168,6 @@ SELECT COUNT(*)
                 FROM "TodoItems" AS t1
                 LEFT JOIN "Tags" AS t2 ON t1."Id" = t2."TodoItemId"
                 WHERE t1."Id" = @p1
-                ORDER BY t2."Id"
                 """));
 
             command.Parameters.Should().HaveCount(1);

test/DapperTests/IntegrationTests/ReadWrite/Resources/FetchResourceTests.cs

@@ -246,7 +246,6 @@ SELECT COUNT(*)
                 FROM "TodoItems" AS t1
                 LEFT JOIN "Tags" AS t2 ON t1."Id" = t2."TodoItemId"
                 WHERE t1."Id" = @p1
-                ORDER BY t2."Id"
                 """));
 
             command.Parameters.Should().HaveCount(1);

test/DapperTests/IntegrationTests/Sql/SubQueryInJoinTests.cs

@@ -62,7 +62,6 @@ SELECT COUNT(*)
                 SELECT t1."Id", t1."FirstName", t1."LastName", t2."Id", t2."LastUsedAt", t2."UserName"
                 FROM "People" AS t1
                 LEFT JOIN "LoginAccounts" AS t2 ON t1."AccountId" = t2."Id"
-                ORDER BY t1."Id"
                 """));
 
             command.Parameters.Should().BeEmpty();
@@ -111,7 +110,7 @@ SELECT COUNT(*)
                 SELECT t1."Id", t1."FirstName", t1."LastName", t2."Id", t2."CreatedAt", t2."Description", t2."DurationInHours", t2."LastModifiedAt", t2."Priority"
                 FROM "People" AS t1
                 LEFT JOIN "TodoItems" AS t2 ON t1."Id" = t2."OwnerId"
-                ORDER BY t1."Id", t2."Priority", t2."LastModifiedAt" DESC
+                ORDER BY t2."Priority", t2."LastModifiedAt" DESC
                 """));
 
             command.Parameters.Should().BeEmpty();
@@ -160,7 +159,7 @@ SELECT COUNT(*)
                 SELECT t1."Id", t1."FirstName", t1."LastName", t2."Id", t2."CreatedAt", t2."Description", t2."DurationInHours", t2."LastModifiedAt", t2."Priority"
                 FROM "People" AS t1
                 LEFT JOIN "TodoItems" AS t2 ON t1."Id" = t2."OwnerId"
-                ORDER BY t1."Id", t2."Description"
+                ORDER BY t2."Description"
                 """));
 
             command.Parameters.Should().BeEmpty();
@@ -209,7 +208,7 @@ SELECT COUNT(*)
                 SELECT t1."Id", t1."FirstName", t1."LastName", t2."Id", t2."CreatedAt", t2."Description", t2."DurationInHours", t2."LastModifiedAt", t2."Priority"
                 FROM "People" AS t1
                 LEFT JOIN "TodoItems" AS t2 ON t1."Id" = t2."OwnerId"
-                ORDER BY t1."Id", (
+                ORDER BY (
                     SELECT COUNT(*)
                     FROM "Tags" AS t3
                     WHERE t2."Id" = t3."TodoItemId"
@@ -263,7 +262,7 @@ SELECT COUNT(*)
                 FROM "People" AS t1
                 LEFT JOIN "TodoItems" AS t2 ON t1."Id" = t2."OwnerId"
                 LEFT JOIN "Tags" AS t4 ON t2."Id" = t4."TodoItemId"
-                ORDER BY t1."Id", (
+                ORDER BY (
                     SELECT COUNT(*)
                     FROM "Tags" AS t3
                     WHERE t2."Id" = t3."TodoItemId"
@@ -326,11 +325,11 @@ SELECT COUNT(*)
                     SELECT COUNT(*)
                     FROM "Tags" AS t4
                     WHERE t3."Id" = t4."TodoItemId"
-                ), t5."Id", (
+                ), (
                     SELECT COUNT(*)
                     FROM "Tags" AS t7
                     WHERE t6."Id" = t7."TodoItemId"
-                ), t8."Id"
+                )
                 """));
 
             command.Parameters.Should().BeEmpty();
@@ -383,7 +382,7 @@ LEFT JOIN (
                     FROM "TodoItems" AS t2
                     WHERE t2."Description" = @p1
                 ) AS t3 ON t1."Id" = t3."OwnerId"
-                ORDER BY t1."Id", t3."Priority", t3."LastModifiedAt" DESC
+                ORDER BY t3."Priority", t3."LastModifiedAt" DESC
                 """));
 
             command.Parameters.Should().HaveCount(1);
@@ -441,7 +440,7 @@ SELECT 1
                         WHERE t2."Id" = t3."TodoItemId"
                     )
                 ) AS t4 ON t1."Id" = t4."OwnerId"
-                ORDER BY t1."Id", t4."Priority", t4."LastModifiedAt" DESC
+                ORDER BY t4."Priority", t4."LastModifiedAt" DESC
                 """));
 
             command.Parameters.Should().BeEmpty();
@@ -498,7 +497,7 @@ SELECT COUNT(*)
                         WHERE t2."Id" = t3."TodoItemId"
                     ) > @p1
                 ) AS t4 ON t1."Id" = t4."OwnerId"
-                ORDER BY t1."Id", t4."Priority", t4."LastModifiedAt" DESC
+                ORDER BY t4."Priority", t4."LastModifiedAt" DESC
                 """));
 
             command.Parameters.Should().HaveCount(1);
@@ -554,7 +553,7 @@ LEFT JOIN (
                     LEFT JOIN "Tags" AS t4 ON t2."Id" = t4."TodoItemId"
                     WHERE t2."Description" = @p1
                 ) AS t5 ON t1."Id" = t5."OwnerId"
-                ORDER BY t1."Id", (
+                ORDER BY (
                     SELECT COUNT(*)
                     FROM "Tags" AS t3
                     WHERE t5."Id" = t3."TodoItemId"
@@ -620,7 +619,7 @@ WHERE NOT (t5."Name" = @p2)
                     ) AS t6 ON t2."Id" = t6."TodoItemId"
                     WHERE NOT (t2."Description" = @p1)
                 ) AS t7 ON t1."Id" = t7."OwnerId"
-                ORDER BY t1."Id", (
+                ORDER BY (
                     SELECT COUNT(*)
                     FROM "Tags" AS t3
                     WHERE t7."Id" = t3."TodoItemId"