diff --git a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs index 6b28db546e..9c41b884b0 100644 --- a/src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs +++ b/src/Azure.DataApiBuilder.Mcp/BuiltInTools/AggregateRecordsTool.cs @@ -955,6 +955,7 @@ private static string ApplyOrderByAndPagination( ? $"{args.Function.ToUpperInvariant()}(DISTINCT {quotedCol})" : $"{args.Function.ToUpperInvariant()}({quotedCol})"; string orderByClause = $" ORDER BY {orderByAggExpr} {direction}"; + string paginationClause = string.Empty; if (args.First.HasValue) { @@ -967,37 +968,24 @@ private static string ApplyOrderByAndPagination( string limitParam = BaseQueryStructure.GetEncodedParamName(structure.Counter.Next()); structure.Parameters.Add(limitParam, new DbConnectionParam(fetchCount)); - string paginationClause = $" OFFSET {offsetParam} ROWS FETCH NEXT {limitParam} ROWS ONLY"; + paginationClause = $" OFFSET {offsetParam} ROWS FETCH NEXT {limitParam} ROWS ONLY"; // Remove TOP N from the SELECT clause (TOP conflicts with OFFSET/FETCH) sql = Regex.Replace(sql, @"SELECT TOP \d+", "SELECT"); - - // Insert ORDER BY + pagination before FOR JSON PATH - int jsonPathIdx = sql.IndexOf(" FOR JSON PATH", StringComparison.OrdinalIgnoreCase); - if (jsonPathIdx > 0) - { - sql = sql.Insert(jsonPathIdx, orderByClause + paginationClause); - } - else - { - sql += orderByClause + paginationClause; - } } - else + + int insertionIndex = sql.IndexOf(" FOR JSON PATH", StringComparison.OrdinalIgnoreCase); + if (insertionIndex < 0 && queryBuilder is DwSqlQueryBuilder) { - // Without pagination: insert ORDER BY before FOR JSON PATH - int jsonPathIdx = sql.IndexOf(" FOR JSON PATH", StringComparison.OrdinalIgnoreCase); - if (jsonPathIdx > 0) - { - sql = sql.Insert(jsonPathIdx, orderByClause); - } - else - { - sql += orderByClause; - } + // STRING_AGG wraps the grouped SELECT. Order and page inside that + // derived table, where the projected aggregation alias is in scope. + insertionIndex = sql.LastIndexOf($") AS {queryBuilder.QuoteIdentifier(structure.SourceAlias)}", StringComparison.Ordinal); + orderByClause = $" ORDER BY {queryBuilder.QuoteIdentifier(ComputeAlias(args.Function, args.Field))} {direction}"; } - return sql; + return insertionIndex > 0 + ? sql.Insert(insertionIndex, orderByClause + paginationClause) + : sql + orderByClause + paginationClause; } #endregion diff --git a/src/Service.Tests/Mcp/AggregateRecordsToolTests.cs b/src/Service.Tests/Mcp/AggregateRecordsToolTests.cs index 817e442cdc..8e40b27ee8 100644 --- a/src/Service.Tests/Mcp/AggregateRecordsToolTests.cs +++ b/src/Service.Tests/Mcp/AggregateRecordsToolTests.cs @@ -349,6 +349,79 @@ public void ApplyOrderByAndPagination_RewritesSql( Assert.AreEqual(paginate ? 2 : 0, structure.Parameters.Count); } + [DataTestMethod] + [DataRow("sum", false)] + [DataRow("avg", false)] + [DataRow("min", false)] + [DataRow("max", false)] + [DataRow("count", false)] + [DataRow("sum", true)] + [DataRow("avg", true)] + [DataRow("min", true)] + [DataRow("max", true)] + [DataRow("count", true)] + public void ApplyOrderByAndPagination_DwSqlOrdersInsideGroupedQuery(string function, bool useJsonFunctions) + { + foreach (bool paginate in new[] { false, true }) + { + foreach (bool distinct in new[] { false, true }) + { + foreach (string direction in new[] { "asc", "desc" }) + { + SqlQueryStructure structure = CreateUninitializedQueryStructure(); + structure.IsListQuery = true; + ViewDefinition definition = new(); + definition.Columns.Add("book_category", new ColumnDefinition { SystemType = typeof(string) }); + definition.Columns.Add("book_price", new ColumnDefinition { SystemType = typeof(decimal) }); + DatabaseView view = new("dbo", "books") { ViewDefinition = definition }; + Mock metadata = new(); + metadata.Setup(x => x.GetSourceDefinition("Book")).Returns(definition); + metadata.Setup(x => x.TryGetBackingColumn("Book", "category", out It.Ref.IsAny)) + .Returns((string _, string _, out string? backing) => + { + backing = "book_category"; + return true; + }); + SetBackingField(structure, "EntityName", "Book"); + SetBackingField(structure, "MetadataProvider", metadata.Object); + SetBackingField(structure, "DatabaseObject", view); + SetBackingField(structure, "JoinQueries", new Dictionary()); + SetBackingField(structure, "Joins", new List()); + SetBackingField(structure, "Predicates", new List()); + SetBackingField(structure, "FilterPredicates", string.Empty); + SetBackingField(structure, "DbPolicyPredicatesForOperations", new Dictionary()); + SetBackingField(structure, "PaginationMetadata", new PaginationMetadata(structure)); + typeof(SqlQueryStructure).GetField("_limit", BindingFlags.Instance | BindingFlags.NonPublic)! + .SetValue(structure, (uint?)10); + string field = function == "count" ? "*" : "price"; + AggregateRecordsTool.AggregateArguments args = CreateAggregateArguments( + function: function, field: field, isCountStar: field == "*", + distinct: distinct, orderby: direction, + first: paginate ? 9 : null, groupby: new() { "category" }) + with { UserProvidedOrderby = direction == "asc" }; + string alias = AggregateRecordsTool.ComputeAlias(function, field); + InvokePrivate("BuildAggregationStructure", + args, structure, view, "book_price", alias, "Book", metadata.Object); + DwSqlQueryBuilder builder = new(enableNto1JoinOpt: useJsonFunctions); + + string sql = InvokePrivate("ApplyOrderByAndPagination", + builder.Build(structure), args, structure, builder, "book_price"); + + string order = useJsonFunctions + ? $"{function.ToUpperInvariant()}({(distinct ? "DISTINCT " : string.Empty)}[table0].[book_price])" + : $"[{alias}]"; + string paging = paginate ? " OFFSET @param0 ROWS FETCH NEXT @param1 ROWS ONLY" : string.Empty; + string suffix = useJsonFunctions ? " FOR JSON PATH, INCLUDE_NULL_VALUES" : ") AS [table0]"; + StringAssert.EndsWith(sql, $" ORDER BY {order} {direction.ToUpperInvariant()}{paging}{suffix}"); + StringAssert.Contains(sql, "GROUP BY [table0].[book_category]"); + StringAssert.Contains(sql, $" AS [{alias}]"); + Assert.AreEqual(!paginate, sql.Contains("SELECT TOP 10")); + Assert.AreEqual(paginate ? 2 : 0, structure.Parameters.Count); + } + } + } + } + #endregion #region Configuration Tests