User/an/add aggregation support to postgresql - #3741
Conversation
…sSQL and PostgreSQL behavior
There was a problem hiding this comment.
Pull request overview
This PR enables end-to-end GraphQL groupBy + aggregation support for PostgreSQL by wiring Postgres into the GraphQL schema’s aggregation feature gate and updating the Postgres SQL query builder to emit aggregation columns and GROUP BY/HAVING clauses. It also re-enables the previously ignored PostgreSQL aggregation integration tests and updates the PostgreSQL test schema.
Changes:
- Enable GraphQL aggregation support for
DatabaseType.PostgreSQLin the schema builder. - Extend
PostgresQueryBuilderSQL generation to include aggregation columns plusGROUP BY/HAVING/conditionalORDER BY. - Un-ignore and update PostgreSQL GraphQL aggregation tests; extend PostgreSQL test schema with
date_only_table.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Service.GraphQLBuilder/Queries/QueryBuilder.cs | Enables aggregation/groupBy schema features for PostgreSQL. |
| src/Core/Resolvers/PostgresQueryBuilder.cs | Adds aggregation + group-by/having clause generation to Postgres SELECT queries. |
| src/Core/Resolvers/BaseTSqlQueryBuilder.cs | Moves shared group-by/having/order-by/aggregation helpers out of the T-SQL-only base. |
| src/Core/Resolvers/BaseSqlQueryBuilder.cs | Centralizes shared group-by/having/order-by/aggregation helper methods for all SQL builders. |
| src/Service.Tests/SqlTests/GraphQLQueryTests/PostgreSqlGraphQLQueryTests.cs | Re-enables and updates PostgreSQL aggregation integration tests and expected SQL baselines. |
| src/Service.Tests/DatabaseSchema-PostgreSql.sql | Adds/drop support for date_only_table test data. |
Comments suppressed due to low confidence (2)
src/Service.Tests/SqlTests/GraphQLQueryTests/PostgreSqlGraphQLQueryTests.cs:633
- This GROUP BY test’s expected SQL omits ORDER BY, but the assertion compares arrays by index. Postgres row order for GROUP BY without ORDER BY is not guaranteed, so the test can be flaky. Add ORDER BY on the group-by fields to make the expected result deterministic.
FROM stocks_price
GROUP BY categoryid, pieceid
HAVING SUM(price) > 50 AND COUNT(pieceid) <= 100
) AS table0";
src/Service.Tests/SqlTests/GraphQLQueryTests/PostgreSqlGraphQLQueryTests.cs:653
- This GROUP BY expected SQL has no ORDER BY, but the test compares expected vs actual by position. Since Postgres doesn’t guarantee GROUP BY output order, add an ORDER BY on the group-by keys to avoid nondeterministic failures.
FROM stocks_price
GROUP BY categoryid, pieceid
) AS table0";
| FROM stocks_price | ||
| GROUP BY categoryid, pieceid | ||
| HAVING SUM(price) > 50 | ||
| ) AS table0"; |
| @@ -193,6 +193,92 @@ protected virtual string Build(AggregationColumn column, bool useAlias = false) | |||
| return $"{column.Type.ToString()}({columnName}) {appendAlias}"; | |||
There was a problem hiding this comment.
fix PR title to reflect what the change is doing. remove user/an/add branch name from title.
Why make this change?
Closes #2850. This PR adds support for group by and aggregation in PostgreSQL in GraphQL queries.
What is this change?
Groupby and aggregation types/enums were added to the GraphQL schema in PostgreSQL, but they essentially were orphaned because the user couldn't utilize them in their GraphQL query. This PR hooks up the groupby/aggregation schema types/enums into a GraphQL input query.
How was this tested?
Sample Request(s)
Previously, a query such as the one above would fail because the
groupByfield would not have been attached to thestocks_pricesobject type. This PR adds thegroupByfield to the relevant list query entity types. ThegroupByfield serves as a window to the rest of the groupBy/aggregation query.