Open
Conversation
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <210500244+JerryNixon@users.noreply.github.com>
…eout to all MCP tools, add tests Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…test file Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: anushakolan <45540936+anushakolan@users.noreply.github.com>
Co-authored-by: anushakolan <45540936+anushakolan@users.noreply.github.com>
…gate-records-tool-fixes
…g performance by offloading computations to the database
Replace in-memory aggregation tests (PerformAggregation, ApplyPagination) with SQL expression generation tests (BuildAggregateExpression, BuildQuotedTableRef, DecodeCursorOffset). All 13 spec examples and 5 blog scenarios now validate SQL patterns instead of in-memory computation. 89 tests pass. Build and format clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- DecodeCursorOffset now rejects negative values (returns 0) - Add max validation for 'first' parameter (100000 limit) - Prevents integer overflow on first+1 and invalid SQL OFFSET - Add tests for both edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace custom SQL string building with engine's SqlQueryStructure + GroupByMetadata + queryBuilder.Build(structure) pattern. This uses the same AggregationColumn, AggregationOperation, and Predicate types that the engine's GraphQL aggregation path uses. Removed methods: BuildAggregateSql, BuildAggregateExpression, BuildQuotedTableRef, BuildWhereClause, BuildHavingClause, AppendPagination. These are now handled by the engine's query builder. Updated both test files to remove references to removed methods. All 69 aggregate tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix COUNT(*): Use primary key column (PK NOT NULL, so COUNT(pk) COUNT(*)) instead of AggregationColumn with empty schema/table/'*' which produced invalid SQL like count([].[*]) - Fix TOP + OFFSET/FETCH conflict: Remove TOP N when pagination is used since SQL Server forbids both in the same query - Add database type validation: Return error for PostgreSQL/MySQL/ CosmosDB since engine only supports aggregation for MsSql/DWSQL - Add HAVING validation: Reject having without groupby - Add tests for star-field-with-avg, distinct-count-star, and having-without-groupby validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 8 tests covering all 5 scenarios from the DAB MCP blog post (devblogs.microsoft.com/azure-sql/data-api-builder-mcp-questions): 1. Strategic customer importance (sum/groupby/orderby desc/first 1) 2. Product discontinuation (sum/groupby/orderby asc/first 1) 3. Quarterly performance (avg/groupby/having gt/orderby desc) 4. Revenue concentration (sum/complex filter/multi-groupby/having) 5. Risk exposure (sum/filter/multi-groupby/having gt) Each test verifies the exact blog JSON payload passes input validation, plus tests for schema completeness, describe_entities instruction, and alias convention documentation. 80 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove redundant parameter listings from Description (already in InputSchema). Description now covers only: workflow steps, rules not expressed elsewhere, and response alias convention. Parameter descriptions simplified to one sentence each, removing repeated phrases like 'from describe_entities' and 'ONLY applies when groupby is provided' (stated once in groupby description). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validate field and groupby field names immediately after metadata resolution, before authorization or query building. Invalid field names now return a FieldNotFound error with model-friendly guidance to call describe_entities for valid field names. - Add McpErrorHelpers.FieldNotFound() with entity name, field name, parameter name, and describe_entities guidance - Move field existence checks before auth in AggregateRecordsTool - Remove redundant late validation (already caught early) - Add tests for FieldNotFound error type and message content 82 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename abbreviated variable names to their full, readable forms: funcElfunctionElement, fieldElfieldElement, distinctEldistinctElement, filterElfilterElement, orderbyElorderbyElement, firstElfirstElement, afterElafterElement, groupbyElgroupbyElement, ggroupbyItem, gValgroupbyFieldName, gFieldgroupbyField, havingElhavingElement, havingOpshavingOperators, havingInhavingInValues, aggTypeaggregationType, aggColumnaggregationColumn, predOppredicateOperation, ophavingOperator, predpredicate, backingColbackingColumn, backingGColbackingGroupbyColumn, timeoutExtimeoutException, taskExtaskCanceledException, dbExdbException, argExargumentException/dabException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DAB config already has MaxResponseSize property that handles this downstream through structure.Limit(). The engine applies the configured limit automatically, making this artificial cap redundant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: souvikghosh04 <210500244+souvikghosh04@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
Aniruddh25
approved these changes
Mar 8, 2026
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
…/data-api-builder into Usr/sogh/aggregate_records
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why make this change?
Closes [Enh]: add
aggregate_recordsDML tool to MCP server #3178Adds
aggregate_recordsas a new DML tool to the MCP server, enabling models to answer common aggregation questions like "How many products are there?" and "What is our most expensive product?"Continuation of work from Add aggregate_records DML tool and query-timeout to MCP server #3179 (stale PR with resolved review comments).
What is this change?
aggregate_recordsDML tool (AggregateRecordsTool.cs) that generates SQL-level aggregation queries (COUNT,AVG,SUM,MIN,MAX) with support forDISTINCT, OData$filter(WHERE),GROUP BY,HAVINGoperators (eq,neq,gt,gte,lt,lte,in),ORDER BY(asc/desc), and cursor-based pagination (first/after) — all per the spec in [Enh]: addaggregate_recordsDML tool to MCP server #3178.query-timeout configuration for MCP runtime options — allows setting a per-query timeout (1–600 seconds, default 30s) via
McpRuntimeOptions.QueryTimeout, validated at startup.DmlToolsConfiggainsAggregateRecords/UserProvidedAggregateRecords;McpRuntimeOptionsConverterFactoryhandles query-timeout serialization;ConfigureOptions.csupdated for CLI configure support; JSON schema updated.How was this tested?
AggregateRecordsToolTests.cs(all 13 spec examples + edge cases),McpQueryTimeoutTests.cs,EntityLevelDmlToolConfigurationTests.cs,McpToolRegistryTests.csAggregateRecordsToolTests.cs(unit),McpTelemetryTests.cs,RequestParserUnitTests.cs,SqlQueryExecutorUnitTests.cs