Skip to content

Fix DWSQL MCP grouped aggregate ordering scope - #3884

Open
Souvik Ghosh (souvikghosh04) with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-invalid-order-by-sum
Open

Souvik Ghosh (souvikghosh04) with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-invalid-order-by-sum

Conversation

Copilot AI commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Why make this change?

MCP aggregate_records appends numeric aggregate ordering outside DWSQL’s grouped derived table, where raw measure columns are no longer available. Fabric Warehouse rejects these queries with Invalid column name.

What is this change?

  • Ordering: Insert alias-based ORDER BY inside the grouped query for DWSQL’s STRING_AGG path.
  • Pagination: Place OFFSET/FETCH in the same scope.
  • Compatibility: Preserve SQL Server and optimized DWSQL query behavior.

How was this tested?

  • Integration Tests
  • Unit Tests

Sample Request(s)

{
  "method": "tools/call",
  "params": {
    "name": "aggregate_records",
    "arguments": {
      "entity": "my_view",
      "function": "sum",
      "field": "amount",
      "groupby": ["category"]
    }
  }
}

The grouped query uses ORDER BY [sum_amount] DESC before the derived-table boundary.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Co-authored-by: souvikghosh04 <210500244+souvikghosh04@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The unordered outer STRING_AGG can violate the requested sort order and cause pagination to skip or repeat groups, requiring matching WITHIN GROUP ordering.

1 open issue
What changed in this PR

This PR addresses #3695 by moving MCP grouped aggregate ordering into the correct DWSQL query scope, avoiding references to raw columns outside the grouped derived table.

Changes:

  • Uses aggregate aliases for ordering inside DWSQL’s STRING_AGG derived table.
  • Places pagination in the same scope while retaining the existing FOR JSON PATH handling.
  • Adds query-generation tests covering aggregate functions, sort directions, distinct values, pagination, and both DWSQL query paths.
File Description
src/​Service.Tests/​Mcp/​AggregateRecordsToolTests.cs Adds DWSQL grouped ordering and pagination regression tests.
src/​Azure.DataApiBuilder.Mcp/​BuiltInTools/​AggregateRecordsTool.cs Adjusts ordering and pagination placement for DWSQL grouped queries.

Review effort: Balanced

Comment on lines +982 to +983
insertionIndex = sql.LastIndexOf($") AS {queryBuilder.QuoteIdentifier(structure.SourceAlias)}", StringComparison.Ordinal);
orderByClause = $" ORDER BY {queryBuilder.QuoteIdentifier(ComputeAlias(args.Function, args.Field))} {direction}";
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review In Progress

4 participants