fix(pat-5034): c# - include order-by when computing group-by

patch-tech · Dec 19, 2023 · 818a4a7 · 818a4a7
1 parent c0b4f4b
commit 818a4a7
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 11 deletions.
diff --git a/static/csharp/Dpm/DpmAgentQueryFactory.cs b/static/csharp/Dpm/DpmAgentQueryFactory.cs
@@ -73,11 +73,19 @@ public static Query MakeQuery(Table query)
           dpmQuery.Select.Add(MakeSelectExpression(item));
         }
 
-        // Process any groupings defined in selection.
-        if (query.Selection.Where((x) => x.GetType().Name == typeof(AggregateFieldExpr<>).Name).Any())
+        // Process any groupings defined in selection and order by.
+        var selectionSet = new HashSet<String>(query.Selection.Select(x => x.Name));
+        var expandedSelection = query.Selection.ToList();
+        foreach (var (expr, _dir) in query.Ordering ?? Array.Empty<Ordering>()) {
+          if (!selectionSet.Contains(expr.Name)) {
+            expandedSelection.Add(expr);
+          }
+        }
+
+        if (expandedSelection.Where((x) => x.GetType().Name == typeof(AggregateFieldExpr<>).Name).Any())
         {
           // Group by the non-aggregate selections.
-          var grouping = query.Selection.Where((x) => x.GetType().Name != typeof(AggregateFieldExpr<>).Name);
+          var grouping = expandedSelection.Where((x) => x.GetType().Name != typeof(AggregateFieldExpr<>).Name);
           foreach (var item in grouping)
           {
             dpmQuery.GroupBy.Add(MakeGroupByExpression(item));

diff --git a/static/csharp/DpmTest/TestQueryFormulation.cs b/static/csharp/DpmTest/TestQueryFormulation.cs
@@ -56,15 +56,16 @@ public void TestQuery()
                     startedAt.DayOfWeek.As("Start DOW")
                     )
                 .Filter(name.Like("%ammy%") | price > 10.0f | tags.HasAny(new string[] { "foo", "bar" }))
-                .OrderBy((price.Max(), Direction.DESC))
+                // The OrderBy includes a field expression that's not in the Select.
+                .OrderBy((price.Max(), Direction.DESC), (startedAt.Week, Direction.ASC))
                 .Limit(10);
             var dpmQuery = DpmAgentQueryFactory.MakeQuery(query);
             var wantQueryStr =
                 "{ \"id\": { \"packageId\": \"1124-111\" }, \"selectFrom\": \"my_table\", " +
                 "\"select\": [ { \"argument\": { \"field\": { \"fieldName\": \"name\" } }, " +
                 "\"alias\": \"Name\" }, { \"argument\": { \"aggregate\": { \"op\": \"MAX\", " +
-                "\"argument\": { \"field\": { \"fieldName\": \"price\" } } } }, \"alias\": \"Max price\" }, " +
-                "{ \"argument\": { \"derived\": { \"op\": \"MONTH\", \"argument\": { " +
+                "\"argument\": { \"field\": { \"fieldName\": \"price\" } } } }, \"alias\": \"Max " +
+                "price\" }, { \"argument\": { \"derived\": { \"op\": \"MONTH\", \"argument\": { " +
                 "\"field\": { \"fieldName\": \"startedAt\" } } } }, \"alias\": \"Start month\" " +
                 "}, { \"argument\": { \"derived\": { \"op\": \"DAY_OF_WEEK\", \"argument\": { " +
                 "\"field\": { \"fieldName\": \"startedAt\" } } } }, \"alias\": \"Start DOW\" } " +
@@ -79,11 +80,14 @@ public void TestQuery()
                 "[ { \"field\": { \"fieldName\": \"name\" } }, { \"derived\": { \"op\": " +
                 "\"MONTH\", \"argument\": { \"field\": { \"fieldName\": \"startedAt\" } } } }, { " +
                 "\"derived\": { \"op\": \"DAY_OF_WEEK\", \"argument\": { \"field\": { " +
-                "\"fieldName\": \"startedAt\" } } } } ], \"orderBy\": [ { \"argument\": { " +
-                "\"aggregate\": { \"op\": \"MAX\", \"argument\": { \"field\": { \"fieldName\": " +
-                "\"price\" } } } }, \"direction\": \"DESC\" } ], \"limit\": \"10\", " +
-                "\"clientVersion\": { \"client\": \"CSHARP\", \"datasetVersion\": \"0.1.0\", " +
-                "\"codeVersion\": \"0.1.0\" } }";
+                "\"fieldName\": \"startedAt\" } } } }, { \"derived\": { \"op\": \"WEEK\", " +
+                "\"argument\": { \"field\": { \"fieldName\": \"startedAt\" } } } } ], " +
+                "\"orderBy\": [ { \"argument\": { \"aggregate\": { \"op\": \"MAX\", " +
+                "\"argument\": { \"field\": { \"fieldName\": \"price\" } } } }, \"direction\": " +
+                "\"DESC\" }, { \"argument\": { \"derived\": { \"op\": \"WEEK\", \"argument\": { " +
+                "\"field\": { \"fieldName\": \"startedAt\" } } } }, \"direction\": \"ASC\" } ], " +
+                "\"limit\": \"10\", \"clientVersion\": { \"client\": \"CSHARP\", " +
+                "\"datasetVersion\": \"0.1.0\", \"codeVersion\": \"0.1.0\" } }";
             Assert.AreEqual(wantQueryStr, dpmQuery.ToString());
         }
     }