From 2a3d60389cd256f1e7e431292d974cb10c362a52 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:42:35 -0700 Subject: [PATCH 1/6] Fix GraphQL aggregation features disabled when runtime.graphql config section is absent (#3450) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why make this change? GraphQL aggregation features (`groupBy`, `sum`, `avg`, `min`, `max`, `count`) were silently disabled for users whose config lacked an explicit `runtime.graphql` section, even though `EnableAggregation` defaults to `true`. ## What is this change? **Bug fix: `RuntimeConfig.EnableAggregation` logic inversion** The property used `&&` logic, returning `false` when `Runtime` or `Runtime.GraphQL` was `null`. This is inconsistent with every analogous property (e.g. `IsGraphQLEnabled`) which use `||` logic to treat absence as "use default (enabled)": ```csharp // Before (broken): returns false when runtime.graphql section is absent public bool EnableAggregation => Runtime is not null && Runtime.GraphQL is not null && Runtime.GraphQL.EnableAggregation; // After (fixed): returns true when section is absent, consistent with IsGraphQLEnabled public bool EnableAggregation => Runtime is null || Runtime.GraphQL is null || Runtime.GraphQL.EnableAggregation; ``` **Additional fixes:** - `GraphQLSchemaCreator.OnConfigChanged` now updates `_isAggregationEnabled` on hot-reload (was omitted, causing stale state) - `schemas/dab.draft.schema.json`: added `enable-aggregation` to `runtime.graphql` properties — it was missing despite `additionalProperties: false`, meaning any config explicitly setting the flag would fail schema validation **Test refactoring (per code review feedback):** - Extracted a `LoadConfig` private helper in `RuntimeConfigLoaderTests` to eliminate repeated mock-filesystem setup across all `EnableAggregation` tests - Merged `EnableAggregation_WhenExplicitlyDisabled_ReturnsFalse` and `EnableAggregation_WhenExplicitlyEnabled_ReturnsTrue` into a single parameterized `[DataTestMethod]` (`EnableAggregation_WhenExplicitlySet_ReturnsConfiguredValue`) using `[DataRow(true)]` and `[DataRow(false)]` - Merged `Build_WithMssqlAndAggregationEnabled_AddsGroupByToConnectionType` and `Build_WithPostgreSqlAndAggregationEnabled_DoesNotAddGroupByToConnectionType` into a single parameterized `[DataTestMethod]` (`Build_WithAggregationEnabled_GroupByPresenceMatchesDatabaseSupport`) covering MSSQL, DWSQL (groupBy present), PostgreSQL, and MySQL (groupBy absent) ## How was this tested? - [ ] Integration Tests - [x] Unit Tests - `RuntimeConfigLoaderTests`: `EnableAggregation` defaults to `true` when `runtime` or `runtime.graphql` sections are absent; a single parameterized test (`EnableAggregation_WhenExplicitlySet_ReturnsConfiguredValue`) covers both `true` and `false` explicit values using `[DataRow]`. A shared `LoadConfig` helper eliminates repeated mock-filesystem setup. - `QueryBuilderTests`: a single parameterized test (`Build_WithAggregationEnabled_GroupByPresenceMatchesDatabaseSupport`) covers MSSQL and DWSQL (groupBy present) and PostgreSQL and MySQL (groupBy absent), plus a test confirming groupBy is omitted when aggregation is disabled. ## Sample Request(s) With a config that has no explicit `runtime.graphql` section, the following now works as expected for MSSQL/DWSQL entities: ```graphql { books { groupBy(fields: [price]) { fields { price } aggregations { count sum(field: price) avg(field: price) max(field: price) min(field: price) } } } } ``` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Aniruddh25 <3513779+Aniruddh25@users.noreply.github.com> Co-authored-by: Aniruddh Munde Co-authored-by: Souvik Ghosh (cherry picked from commit 9391685f0b70ae9bf5a9ecb233b65d097a89b8f0) --- schemas/dab.draft.schema.json | 5 + src/Config/ObjectModel/RuntimeConfig.cs | 8 +- src/Core/Services/GraphQLSchemaCreator.cs | 4 +- .../Configuration/RuntimeConfigLoaderTests.cs | 100 ++++++++++++++++++ .../GraphQLBuilder/QueryBuilderTests.cs | 86 +++++++++++++++ 5 files changed, 199 insertions(+), 4 deletions(-) diff --git a/schemas/dab.draft.schema.json b/schemas/dab.draft.schema.json index 7394ff7268..9970d98179 100644 --- a/schemas/dab.draft.schema.json +++ b/schemas/dab.draft.schema.json @@ -246,6 +246,11 @@ "description": "Maximum allowed depth of a GraphQL query. Only positive integers are enforced. Default: null (no limit). Use -1 to explicitly remove a previously set limit.", "default": null }, + "enable-aggregation": { + "$ref": "#/$defs/boolean-or-string", + "description": "Allow enabling/disabling aggregation (groupBy, sum, avg, min, max, count) for supported database types (MSSQL, DWSQL).", + "default": true + }, "multiple-mutations": { "type": "object", "description": "Configuration properties for multiple mutation operations", diff --git a/src/Config/ObjectModel/RuntimeConfig.cs b/src/Config/ObjectModel/RuntimeConfig.cs index 0f971293ee..cac4f8fddc 100644 --- a/src/Config/ObjectModel/RuntimeConfig.cs +++ b/src/Config/ObjectModel/RuntimeConfig.cs @@ -222,12 +222,14 @@ Runtime.GraphQL is null || public string DefaultDataSourceName { get; set; } /// - /// Retrieves the value of runtime.graphql.aggregation.enabled property if present, default is true. + /// Retrieves the value of runtime.graphql.enable-aggregation property if present, default is true. + /// Returns true when runtime section is absent, when graphql section is absent, + /// or when enable-aggregation is explicitly set to true. /// [JsonIgnore] public bool EnableAggregation => - Runtime is not null && - Runtime.GraphQL is not null && + Runtime is null || + Runtime.GraphQL is null || Runtime.GraphQL.EnableAggregation; [JsonIgnore] diff --git a/src/Core/Services/GraphQLSchemaCreator.cs b/src/Core/Services/GraphQLSchemaCreator.cs index 4e063f08a7..a7b51d8827 100644 --- a/src/Core/Services/GraphQLSchemaCreator.cs +++ b/src/Core/Services/GraphQLSchemaCreator.cs @@ -80,12 +80,14 @@ public GraphQLSchemaCreator( /// /// Executed when a hot-reload event occurs. Pulls the latest /// runtimeconfig object from the provider and updates the flag indicating - /// whether multiple create operations are enabled, and the entities based on the new config. + /// whether multiple create operations are enabled, whether aggregation is enabled, + /// and the entities based on the new config. /// protected void OnConfigChanged(object? sender, HotReloadEventArgs args) { RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig(); _isMultipleCreateOperationEnabled = runtimeConfig.IsMultipleCreateOperationEnabled(); + _isAggregationEnabled = runtimeConfig.EnableAggregation; _entities = runtimeConfig.Entities; } diff --git a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs index d4470a0017..ab709c84f3 100644 --- a/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs +++ b/src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs @@ -309,4 +309,104 @@ public async Task ChildConfigLoadFailureHaltsParentConfigLoading() } } } + + /// + /// Tests that EnableAggregation returns true by default when runtime.graphql section is absent. + /// This is a regression test for the bug where EnableAggregation returned false (disabled) + /// when Runtime.GraphQL was null, even though the default value for EnableAggregation is true. + /// + [TestMethod] + public void EnableAggregation_WhenGraphQLSectionAbsent_DefaultsToTrue() + { + // Arrange: a minimal config with no runtime.graphql section + string configJson = @"{ + ""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"", + ""data-source"": { + ""database-type"": ""mssql"", + ""connection-string"": ""Server=tcp:127.0.0.1,1433;"" + }, + ""runtime"": { + ""host"": { + ""authentication"": { ""provider"": ""StaticWebApps"" } + } + }, + ""entities"": {} + }"; + + RuntimeConfig runtimeConfig = LoadConfig(configJson); + + Assert.IsNull(runtimeConfig.Runtime?.GraphQL, "GraphQL section should be null for this config."); + Assert.IsTrue(runtimeConfig.EnableAggregation, + "EnableAggregation should default to true when runtime.graphql section is absent."); + } + + /// + /// Tests that EnableAggregation returns true by default when runtime section is absent. + /// + [TestMethod] + public void EnableAggregation_WhenRuntimeSectionAbsent_DefaultsToTrue() + { + // Arrange: a minimal config with no runtime section at all + string configJson = @"{ + ""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"", + ""data-source"": { + ""database-type"": ""mssql"", + ""connection-string"": ""Server=tcp:127.0.0.1,1433;"" + }, + ""entities"": {} + }"; + + RuntimeConfig runtimeConfig = LoadConfig(configJson); + + Assert.IsNull(runtimeConfig.Runtime, "Runtime section should be null for this config."); + Assert.IsTrue(runtimeConfig.EnableAggregation, + "EnableAggregation should default to true when runtime section is absent."); + } + + /// + /// Tests that EnableAggregation honours the explicit value set in the config file. + /// + [DataTestMethod] + [DataRow(true, DisplayName = "Explicit true is respected")] + [DataRow(false, DisplayName = "Explicit false is respected")] + public void EnableAggregation_WhenExplicitlySet_ReturnsConfiguredValue(bool explicitValue) + { + string configJson = $@"{{ + ""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"", + ""data-source"": {{ + ""database-type"": ""mssql"", + ""connection-string"": ""Server=tcp:127.0.0.1,1433;"" + }}, + ""runtime"": {{ + ""graphql"": {{ + ""enabled"": true, + ""enable-aggregation"": {explicitValue.ToString().ToLower()} + }}, + ""host"": {{ + ""authentication"": {{ ""provider"": ""StaticWebApps"" }} + }} + }}, + ""entities"": {{}} + }}"; + + RuntimeConfig runtimeConfig = LoadConfig(configJson); + + Assert.AreEqual(explicitValue, runtimeConfig.EnableAggregation, + $"EnableAggregation should be {explicitValue} when explicitly set to {explicitValue} in config."); + } + + /// + /// Loads a from a JSON string using a mock file system. + /// + private static RuntimeConfig LoadConfig(string configJson) + { + IFileSystem fs = new MockFileSystem(new Dictionary + { + { "dab-config.json", new MockFileData(configJson) } + }); + + FileSystemRuntimeConfigLoader loader = new(fs); + Assert.IsTrue(loader.TryLoadConfig("dab-config.json", out RuntimeConfig config), "Config should load successfully."); + return config; + } } diff --git a/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs b/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs index c257a86054..93a3df9778 100644 --- a/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs +++ b/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs @@ -18,6 +18,16 @@ public class QueryBuilderTests { private const int NUMBER_OF_ARGUMENTS = 4; + /// + /// GQL schema for a Book entity with numeric fields, used for aggregation tests. + /// + private const string BOOK_WITH_NUMERIC_FIELDS_GQL = @" +type Book @model(name:""Book"") { + id: ID! + price: Float! + title: String +}"; + private Dictionary _entityPermissions; /// @@ -37,6 +47,14 @@ public void SetupEntityPermissionsMap() ); } + private static Dictionary CreateBookEntityPermissions() + { + return GraphQLTestHelpers.CreateStubEntityPermissionsMap( + new string[] { "Book" }, + new EntityActionOperation[] { EntityActionOperation.Read }, + new string[] { "anonymous" }); + } + [DataTestMethod] [TestCategory("Query Generation")] [TestCategory("Single item access")] @@ -538,6 +556,74 @@ public void GenerateReturnType_IncludesGroupByField() Assert.AreEqual("BookGroupBy", groupByType.Name.Value, "should return GroupBy type"); } + /// + /// Tests that the return type does NOT include the groupBy field when aggregation is disabled. + /// + [TestMethod] + [TestCategory("Query Builder - Return Type")] + public void GenerateReturnType_ExcludesGroupByField_WhenAggregationDisabled() + { + // Arrange + NameNode entityName = new("Book"); + + // Act + ObjectTypeDefinitionNode returnType = QueryBuilder.GenerateReturnType(entityName, isAggregationEnabled: false); + + // Assert + FieldDefinitionNode groupByField = returnType.Fields.FirstOrDefault(f => f.Name.Value == "groupBy"); + Assert.IsNull(groupByField, "groupBy field should NOT exist when aggregation is disabled"); + } + + /// + /// Tests that QueryBuilder.Build correctly adds or omits the groupBy field on the + /// connection type based on whether the database type is in + /// . + /// MSSQL and DWSQL are supported; other types (e.g. PostgreSQL) are not. + /// + [DataTestMethod] + [DataRow(DatabaseType.MSSQL, true, DisplayName = "MSSQL: groupBy field present when aggregation enabled")] + [DataRow(DatabaseType.DWSQL, true, DisplayName = "DWSQL: groupBy field present when aggregation enabled")] + [DataRow(DatabaseType.PostgreSQL, false, DisplayName = "PostgreSQL: groupBy field absent (not in AggregationEnabledDatabaseTypes)")] + [DataRow(DatabaseType.MySQL, false, DisplayName = "MySQL: groupBy field absent (not in AggregationEnabledDatabaseTypes)")] + [TestCategory("Query Builder - Aggregation")] + public void Build_WithAggregationEnabled_GroupByPresenceMatchesDatabaseSupport( + DatabaseType dbType, + bool expectGroupBy) + { + // Arrange + DocumentNode root = Utf8GraphQLParser.Parse(BOOK_WITH_NUMERIC_FIELDS_GQL); + Dictionary entityNameToDatabaseType = new() + { + { "Book", dbType } + }; + + // Act + DocumentNode queryRoot = QueryBuilder.Build( + root, + entityNameToDatabaseType, + new(new Dictionary { { "Book", GraphQLTestHelpers.GenerateEmptyEntity() } }), + inputTypes: new(), + entityPermissionsMap: CreateBookEntityPermissions(), + _isAggregationEnabled: true + ); + + // Assert: find BookConnection type + ObjectTypeDefinitionNode bookConnection = queryRoot.Definitions + .OfType() + .FirstOrDefault(d => d.Name.Value == "BookConnection"); + Assert.IsNotNull(bookConnection, "BookConnection type should exist"); + + FieldDefinitionNode groupByField = bookConnection.Fields.FirstOrDefault(f => f.Name.Value == "groupBy"); + if (expectGroupBy) + { + Assert.IsNotNull(groupByField, $"groupBy field should exist on BookConnection for {dbType}"); + } + else + { + Assert.IsNull(groupByField, $"groupBy field should NOT exist on BookConnection for {dbType} (not in AggregationEnabledDatabaseTypes)"); + } + } + public static ObjectTypeDefinitionNode GetQueryNode(DocumentNode queryRoot) { return (ObjectTypeDefinitionNode)queryRoot.Definitions.First(d => d is ObjectTypeDefinitionNode node && node.Name.Value == "Query"); From 089864683de1d6e890f1cc29d98f91f53b2527cf Mon Sep 17 00:00:00 2001 From: Arjun Narendra Date: Thu, 23 Jul 2026 10:07:45 -0700 Subject: [PATCH 2/6] Database policy support for PUT/PATCH operations - PostgreSQL (#3694) ## Why make this change? As per the behaviour expected from PUT/PATCH operations with database policies discussed in #1430, implement db policy support for PostgreSQL to fix #1372. ## What is this change? 1. Prior to this change, there was only one database policy for each operation. Since now database policies will be supported for both insert (or create)/update actions via PUT/PATCH operations, these 2 operations can have 2 database policies defined for them, one for each action. 2. The query generated by `PostgresQueryBuilder.Build(SqlUpsertQueryStructure structure)` is modified to accommodate create/update policies while also keeping intact the normal upsert behavior expected (try update, then insert). 3. The method `IQueryExecutor.GetMultipleResultSetsIfAnyAsync` has been provided another implementation specific to PostgreSql in `PostgreSqlExecutor`. The `DbDataReader` instance for the query being executed for the PUT/PATCH operation will always contain two result sets. 4. Different scenarios are added to the method `PostgreSqlExecutor.GetMultipleResultSetsIfAnyAsync` to throw appropriate exceptions (Forbidden/Authorization failure - 403 and NotFound - 404). Appropriate comments are added within the code to demonstrate each case. ## How was this tested? Integration Tests - Done --------- Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> (cherry picked from commit 57a195c16dc95897e3a2e90fb3bf5802fd9bc42f) --- config-generators/postgresql-commands.txt | 3 +- .../Configurations/RuntimeConfigValidator.cs | 3 +- src/Core/Resolvers/PostgreSqlExecutor.cs | 83 +++++++++++++++++++ src/Core/Resolvers/PostgresQueryBuilder.cs | 58 ++++++++++--- src/Core/Resolvers/SqlMutationEngine.cs | 69 ++++++--------- ...ingRuntimeConfigForPostgreSql.verified.txt | 15 ++-- .../Patch/PostgreSqlPatchApiTests.cs | 35 +++----- .../RestApiTests/Put/PostgreSqlPutApiTests.cs | 35 +++----- .../UnitTests/ConfigValidationUnitTests.cs | 6 +- src/Service.Tests/dab-config.PostgreSql.json | 5 +- 10 files changed, 203 insertions(+), 109 deletions(-) diff --git a/config-generators/postgresql-commands.txt b/config-generators/postgresql-commands.txt index 07eb8aaa74..f249103253 100644 --- a/config-generators/postgresql-commands.txt +++ b/config-generators/postgresql-commands.txt @@ -58,8 +58,9 @@ update Publisher --config "dab-config.PostgreSql.json" --permissions "database_p update Publisher --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:create" update Publisher --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:update" --policy-database "@item.id ne 1234" update Stock --config "dab-config.PostgreSql.json" --permissions "authenticated:create,read,update,delete" --rest commodities --graphql true --relationship stocks_price --target.entity stocks_price --cardinality one -update Stock --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:create,read" update Stock --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:update" --policy-database "@item.pieceid ne 1" +update Stock --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:create" --policy-database "@item.pieceid ne 6 and @item.piecesAvailable gt 0" +update Stock --config "dab-config.PostgreSql.json" --permissions "database_policy_tester:read" update Stock --config "dab-config.PostgreSql.json" --permissions "test_role_with_noread:create,update,delete" update Stock --config "dab-config.PostgreSql.json" --permissions "test_role_with_excluded_fields:create,update,delete" update Stock --config "dab-config.PostgreSql.json" --permissions "test_role_with_excluded_fields:read" --fields.exclude "categoryName" diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index 0672eebc8f..481dcef5e8 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -46,7 +46,8 @@ public class RuntimeConfigValidator : IConfigValidator private static readonly HashSet _databaseTypesSupportingCreatePolicy = [ DatabaseType.MSSQL, - DatabaseType.DWSQL + DatabaseType.DWSQL, + DatabaseType.PostgreSQL ]; // Error messages for user-delegated authentication configuration. diff --git a/src/Core/Resolvers/PostgreSqlExecutor.cs b/src/Core/Resolvers/PostgreSqlExecutor.cs index 70fa0f1079..4130cd1378 100644 --- a/src/Core/Resolvers/PostgreSqlExecutor.cs +++ b/src/Core/Resolvers/PostgreSqlExecutor.cs @@ -2,11 +2,13 @@ // Licensed under the MIT License. using System.Data.Common; +using System.Net; using Azure.Core; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; +using Azure.DataApiBuilder.Service.Exceptions; using Azure.Identity; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -146,6 +148,87 @@ private static bool ShouldManagedIdentityAccessBeAttempted(NpgsqlConnectionStrin return string.IsNullOrEmpty(builder.Password); } + /// + public override async Task GetMultipleResultSetsIfAnyAsync( + DbDataReader dbDataReader, List? args = null) + { + // RS1: COUNT of rows matching PK (no policy) — used to distinguish + // "row doesn't exist" from "row exists but policy blocked". + DbResultSet resultSetWithCountOfRowsWithGivenPk = await ExtractResultSetFromDbDataReaderAsync(dbDataReader); + DbResultSetRow? resultSetRowWithCountOfRowsWithGivenPk = resultSetWithCountOfRowsWithGivenPk.Rows.FirstOrDefault(); + int numOfRecordsWithGivenPK; + bool isFallbackToUpdate; + + if (resultSetRowWithCountOfRowsWithGivenPk is not null && + resultSetRowWithCountOfRowsWithGivenPk.Columns.TryGetValue(PostgresQueryBuilder.COUNT_ROWS_WITH_GIVEN_PK, out object? rowsWithGivenPK) && + resultSetRowWithCountOfRowsWithGivenPk.Columns.TryGetValue(PostgresQueryBuilder.IS_FALLBACK_TO_UPDATE, out object? fallbackToUpdate)) + { + // PostgreSQL COUNT(*) returns Int64; convert to int. + numOfRecordsWithGivenPK = Convert.ToInt32(rowsWithGivenPK!); + isFallbackToUpdate = Convert.ToBoolean(fallbackToUpdate!); + } + else + { + throw new DataApiBuilderException( + message: $"Neither insert nor update could be performed.", + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + } + + // RS2: UPDATE result, or UPDATE+INSERT CTE result. + DbResultSet dbResultSet = await dbDataReader.NextResultAsync() + ? await ExtractResultSetFromDbDataReaderAsync(dbDataReader) + : throw new DataApiBuilderException( + message: $"Neither insert nor update could be performed.", + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + + if (numOfRecordsWithGivenPK == 1) // Row existed — we attempted an UPDATE. + { + if (dbResultSet.Rows.Count == 0) + { + // Row exists but UPDATE returned no rows — update policy blocked it. + throw new DataApiBuilderException( + message: DataApiBuilderException.AUTHORIZATION_FAILURE, + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure); + } + } + else if (dbResultSet.Rows.Count == 0) + { + // If true, the row simply didn't exist — return 404 (same as MsSql's null-RS2 path). + // If false, the INSERT ran but create policy blocked it — return 403. + + if (isFallbackToUpdate) + { + if (args is not null && args.Count > 1) + { + string prettyPrintPk = args[0]; + string entityName = args[1]; + + throw new DataApiBuilderException( + message: $"Cannot perform INSERT and could not find {entityName} " + + $"with primary key {prettyPrintPk} to perform UPDATE on.", + statusCode: HttpStatusCode.NotFound, + subStatusCode: DataApiBuilderException.SubStatusCodes.ItemNotFound); + } + + throw new DataApiBuilderException( + message: $"Neither insert nor update could be performed.", + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + } + + // Row didn't exist but INSERT returned no rows — create policy blocked it. + throw new DataApiBuilderException( + message: DataApiBuilderException.AUTHORIZATION_FAILURE, + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure); + } + + return dbResultSet; + } + /// /// Determines if the saved default azure credential's access token is valid and not expired. /// diff --git a/src/Core/Resolvers/PostgresQueryBuilder.cs b/src/Core/Resolvers/PostgresQueryBuilder.cs index 244b1a45b8..00ac571149 100644 --- a/src/Core/Resolvers/PostgresQueryBuilder.cs +++ b/src/Core/Resolvers/PostgresQueryBuilder.cs @@ -17,6 +17,8 @@ public class PostgresQueryBuilder : BaseSqlQueryBuilder, IQueryBuilder private const string UPSERT_IDENTIFIER_COLUMN_NAME = "___upsert_op___"; private const string INSERT_UPSERT = "inserted"; private const string UPDATE_UPSERT = "updated"; + public const string COUNT_ROWS_WITH_GIVEN_PK = "cnt_rows_to_update"; + public const string IS_FALLBACK_TO_UPDATE = "is_fallback_to_update"; private static DbCommandBuilder _builder = new NpgsqlCommandBuilder(); @@ -67,11 +69,17 @@ public string Build(SqlQueryStructure structure) /// public string Build(SqlInsertStructure structure) { - string insertQuery = $"INSERT INTO {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} "; + string tableName = $"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}"; + string dbPolicyPredicates = JoinPredicateStrings(structure.GetDbPolicyForOperation(EntityActionOperation.Create)); + string insertQuery = $"INSERT INTO {tableName} "; + if (structure.InsertColumns.Any()) { - insertQuery += $"({Build(structure.InsertColumns)}) " + - $"VALUES ({string.Join(", ", (structure.Values))}) "; + string insertColumns = Build(structure.InsertColumns); + string insertValues = dbPolicyPredicates.Equals(BASE_PREDICATE) + ? $"({insertColumns}) VALUES ({string.Join(", ", structure.Values)})" + : $"({insertColumns}) SELECT {insertColumns} FROM (SELECT {string.Join(", ", structure.InsertColumns.Zip(structure.Values, (col, val) => $"{val} AS {QuoteIdentifier(col)}"))}) AS T WHERE {dbPolicyPredicates}"; + insertQuery += insertValues; } else { @@ -117,25 +125,53 @@ public string Build(SqlUpsertQueryStructure structure) { // https://stackoverflow.com/questions/42668720/check-if-postgres-query-inserted-or-updated-via-upsert // relying on xmax to detect insert vs update breaks for views - string updatePredicates = JoinPredicateStrings(Build(structure.Predicates), structure.GetDbPolicyForOperation(EntityActionOperation.Update)); - string updateQuery = $"UPDATE {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} " + + string tableName = $"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}"; + string pkPredicates = Build(structure.Predicates); + string isFallbackToUpdateSqlLiteral = structure.IsFallbackToUpdate ? "TRUE" : "FALSE"; + + // RS1: COUNT of rows matching PK (no policy) — used to distinguish + // "row doesn't exist" from "row exists but policy blocked" in the executor. + string countQuery = $"SELECT COUNT(*) AS {COUNT_ROWS_WITH_GIVEN_PK}, " + + $"{isFallbackToUpdateSqlLiteral} AS {IS_FALLBACK_TO_UPDATE} " + + $"FROM {tableName} WHERE {pkPredicates}"; + + string updatePredicates = JoinPredicateStrings(pkPredicates, structure.GetDbPolicyForOperation(EntityActionOperation.Update)); + string updateQuery = $"UPDATE {tableName} " + $"SET {Build(structure.UpdateOperations, ", ")} " + $"WHERE {updatePredicates} " + $"RETURNING {Build(structure.OutputColumns)}, '{UPDATE_UPSERT}' AS {UPSERT_IDENTIFIER_COLUMN_NAME}"; if (structure.IsFallbackToUpdate) { - return updateQuery + ";"; + // RS2: UPDATE only — no INSERT branch for autogen PK or missing required columns. + return $"{countQuery}; {updateQuery};"; } else { - return $"WITH update_cte AS ( {updateQuery} ), insert_cte AS ( " + - $"INSERT INTO {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} ({Build(structure.InsertColumns)}) " + - $"SELECT {string.Join(", ", (structure.Values))} " + - $"WHERE NOT EXISTS (SELECT 1 FROM update_cte) " + + // INSERT only runs when row doesn't exist (pkPredicates match nothing) + // AND the create policy (if any) is satisfied. + string insertPredicates = JoinPredicateStrings( + $"NOT EXISTS (SELECT 1 FROM {tableName} WHERE {pkPredicates})", + structure.GetDbPolicyForOperation(EntityActionOperation.Create)); + + // Alias each value with its column name so that policy predicates referencing + // column names (e.g. "pieceid" != @param) can be resolved in the WHERE clause. + // Using SELECT ... FROM (SELECT @p1 AS col1, ...) AS T avoids both the VALUES(NULL) + // type inference issue and the unnamed-column resolution issue. + string namedValues = string.Join(", ", + structure.InsertColumns.Zip(structure.Values, + (col, val) => $"{val} AS {QuoteIdentifier(col)}")); + + // RS2: CTE that attempts UPDATE first; falls through to INSERT only when row is absent. + string cteQuery = $"WITH update_cte AS ( {updateQuery} ), insert_cte AS ( " + + $"INSERT INTO {tableName} ({Build(structure.InsertColumns)}) " + + $"SELECT {Build(structure.InsertColumns)} FROM (SELECT {namedValues}) AS T " + + $"WHERE {insertPredicates} " + $"RETURNING {Build(structure.OutputColumns)}, '{INSERT_UPSERT}' AS {UPSERT_IDENTIFIER_COLUMN_NAME} ) " + - $"SELECT {BuildListOfLabels(structure.OutputColumns)}, {UPSERT_IDENTIFIER_COLUMN_NAME} FROM update_cte UNION " + + $"SELECT {BuildListOfLabels(structure.OutputColumns)}, {UPSERT_IDENTIFIER_COLUMN_NAME} FROM update_cte UNION ALL " + $"SELECT {BuildListOfLabels(structure.OutputColumns)}, {UPSERT_IDENTIFIER_COLUMN_NAME} FROM insert_cte;"; + + return $"{countQuery}; {cteQuery}"; } } diff --git a/src/Core/Resolvers/SqlMutationEngine.cs b/src/Core/Resolvers/SqlMutationEngine.cs index 970a58bb51..d67e731103 100644 --- a/src/Core/Resolvers/SqlMutationEngine.cs +++ b/src/Core/Resolvers/SqlMutationEngine.cs @@ -738,52 +738,37 @@ await PerformMutationOperation( parameters: parameters, sqlMetadataProvider: sqlMetadataProvider); - if (mutationResultRow is null || mutationResultRow.Columns.Count == 0) + if (mutationResultRow is null) + { + HttpStatusCode statusCode = context.OperationType is EntityActionOperation.Insert + ? HttpStatusCode.InternalServerError + : HttpStatusCode.NotFound; + + // Ideally this case should not happen, however may occur due to unexpected reasons, + // like the DbDataReader being null. We throw an exception + // which will be returned as an UnexpectedError. + throw new DataApiBuilderException( + message: "An unexpected error occurred while trying to execute the query.", + statusCode: statusCode, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + } + + if (mutationResultRow.Columns.Count == 0) { if (context.OperationType is EntityActionOperation.Insert) { - if (mutationResultRow is null) - { - // Ideally this case should not happen, however may occur due to unexpected reasons, - // like the DbDataReader being null. We throw an exception - // which will be returned as an UnexpectedError. - throw new DataApiBuilderException( - message: "An unexpected error occurred while trying to execute the query.", - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); - } - - if (mutationResultRow.Columns.Count == 0) - { - throw new DataApiBuilderException( - message: "Could not insert row with given values.", - statusCode: HttpStatusCode.Forbidden, - subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure - ); - } + throw new DataApiBuilderException( + message: "Could not insert row with given values.", + statusCode: HttpStatusCode.Forbidden, + subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure + ); } - else - { - if (mutationResultRow is null) - { - // Ideally this case should not happen, however may occur due to unexpected reasons, - // like the DbDataReader being null. We throw an exception - // which will be returned as an UnexpectedError - throw new DataApiBuilderException(message: "An unexpected error occurred while trying to execute the query.", - statusCode: HttpStatusCode.NotFound, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); - } - - if (mutationResultRow.Columns.Count == 0) - { - // This code block is reached when Update or UpdateIncremental operation does not successfully find the record to - // update. An exception is thrown which will be returned as a 404 NotFound response. - throw new DataApiBuilderException(message: "No Update could be performed, record not found", - statusCode: HttpStatusCode.NotFound, - subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); - } - } + // This code block is reached when Update or UpdateIncremental operation does not successfully find the record to + // update. An exception is thrown which will be returned as a 404 NotFound response. + throw new DataApiBuilderException(message: "No Update could be performed, record not found", + statusCode: HttpStatusCode.NotFound, + subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); } // The role with which the REST request is executed can have database policies defined for the read action. @@ -2272,7 +2257,7 @@ private void AuthorizeEntityAndFieldsForMutation( /// } /// } /// - /// + /// /// Example 2 - Multiple items creation: /// /// mutation { diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt index f7d781fe64..05f042b619 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt @@ -331,16 +331,19 @@ Role: database_policy_tester, Actions: [ { - Action: Update, - Policy: { - Database: @item.pieceid ne 1 - } + Action: Read }, { - Action: Create + Action: Create, + Policy: { + Database: @item.pieceid ne 6 and @item.piecesAvailable gt 0 + } }, { - Action: Read + Action: Update, + Policy: { + Database: @item.pieceid ne 1 + } } ] }, diff --git a/src/Service.Tests/SqlTests/RestApiTests/Patch/PostgreSqlPatchApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Patch/PostgreSqlPatchApiTests.cs index 0a808bae58..5598992037 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Patch/PostgreSqlPatchApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Patch/PostgreSqlPatchApiTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -142,6 +141,19 @@ SELECT to_jsonb(subq) AS data ) AS subq " }, + { + "PatchOneInsertWithDatabasePolicy", + @" + SELECT to_jsonb(subq) AS data + FROM ( + SELECT categoryid, pieceid, ""categoryName"", ""piecesAvailable"", ""piecesRequired"" + FROM " + _Composite_NonAutoGenPK_TableName + @" + WHERE categoryid = 0 AND pieceid = 7 AND ""categoryName"" = 'SciFi' + AND ""piecesAvailable"" = 4 AND ""piecesRequired"" = 0 + AND (pieceid != 6 AND ""piecesAvailable"" > 0) + ) AS subq + " + }, { "PatchOne_Update_Default_Test", @" @@ -311,27 +323,6 @@ await base.PatchOneViewBadRequestTest( } #region overridden tests - - [TestMethod] - [Ignore] - public override Task PatchOneUpdateWithUnsatisfiedDatabasePolicy() - { - throw new NotImplementedException(); - } - - [TestMethod] - [Ignore] - public override Task PatchOneInsertWithUnsatisfiedDatabasePolicy() - { - throw new NotImplementedException(); - } - - [TestMethod] - [Ignore] - public override Task PatchOneInsertWithDatabasePolicy() - { - throw new NotImplementedException(); - } #endregion #region Test Fixture Setup diff --git a/src/Service.Tests/SqlTests/RestApiTests/Put/PostgreSqlPutApiTests.cs b/src/Service.Tests/SqlTests/RestApiTests/Put/PostgreSqlPutApiTests.cs index e9f8bcaac1..aa61570f3c 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Put/PostgreSqlPutApiTests.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Put/PostgreSqlPutApiTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -61,6 +60,19 @@ SELECT to_jsonb(subq) AS data ) AS subq " }, + { + "PutOneInsertWithDatabasePolicy", + @" + SELECT to_jsonb(subq) AS data + FROM ( + SELECT categoryid, pieceid, ""categoryName"", ""piecesAvailable"", ""piecesRequired"" + FROM " + _Composite_NonAutoGenPK_TableName + @" + WHERE categoryid = 0 AND pieceid = 7 AND ""categoryName"" = 'SciFi' + AND ""piecesAvailable"" = 4 AND ""piecesRequired"" = 0 + AND (pieceid != 6 AND ""piecesAvailable"" > 0) + ) AS subq + " + }, { "PutOneUpdateAccessibleRowWithDatabasePolicy", @" @@ -410,27 +422,6 @@ public static async Task SetupAsync(TestContext context) #endregion #region overridden tests - - [TestMethod] - [Ignore] - public override Task PutOneInsertWithDatabasePolicy() - { - throw new NotImplementedException(); - } - - [TestMethod] - [Ignore] - public override Task PutOneWithUnsatisfiedDatabasePolicy() - { - throw new NotImplementedException(); - } - - [TestMethod] - [Ignore] - public override Task PutOneInsertInTableWithFieldsInDbPolicyNotPresentInBody() - { - throw new NotImplementedException(); - } #endregion [TestCleanup] diff --git a/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs b/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs index 05561e4cf9..99252a7ba6 100644 --- a/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs +++ b/src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs @@ -179,12 +179,12 @@ public void InvalidActionSpecifiedForARole(string dbPolicy, EntityActionOperatio /// /// Test that permission configuration validation fails when a database policy - /// is defined for the Create operation for mysql/postgresql and passes for mssql. + /// is defined for the Create operation for mysql and passes for mssql/postgresql. /// /// Database policy. /// Whether an error is expected. [DataTestMethod] - [DataRow(DatabaseType.PostgreSQL, "1 eq @item.col1", true, DisplayName = "Database Policy defined for Create fails for PostgreSQL")] + [DataRow(DatabaseType.PostgreSQL, "1 eq @item.col1", false, DisplayName = "Database Policy defined for Create passes for PostgreSQL")] [DataRow(DatabaseType.PostgreSQL, null, false, DisplayName = "Database Policy set as null for Create passes on PostgreSQL.")] [DataRow(DatabaseType.PostgreSQL, "", false, DisplayName = "Database Policy left empty for Create passes for PostgreSQL.")] [DataRow(DatabaseType.PostgreSQL, " ", false, DisplayName = "Database Policy only whitespace for Create passes for PostgreSQL.")] @@ -2792,7 +2792,7 @@ public void ValidateRuntimeBaseRouteSettings( /// This test checks that the final config used by runtime engine doesn't lose the directory information /// if provided by the user. /// It also validates that if config file is provided by the user, it will be used directly irrespective of - /// environment variable being set or not. + /// environment variable being set or not. /// When user doesn't provide a config file, we check if environment variable is set and if it is, we use /// the config file specified by the environment variable, else we use the default config file. /// diff --git a/src/Service.Tests/dab-config.PostgreSql.json b/src/Service.Tests/dab-config.PostgreSql.json index 48f9700754..b9bfae0ec3 100644 --- a/src/Service.Tests/dab-config.PostgreSql.json +++ b/src/Service.Tests/dab-config.PostgreSql.json @@ -324,7 +324,10 @@ } }, { - "action": "create" + "action": "create", + "policy": { + "database": "@item.pieceid ne 6 and @item.piecesAvailable gt 0" + } }, { "action": "read" From 7bfeaeed7fc07a3171b5e0e1aaff0b16012f4081 Mon Sep 17 00:00:00 2001 From: Arjun Narendra Date: Tue, 28 Jul 2026 22:53:23 -0700 Subject: [PATCH 3/6] Add support for DateTime filters in PostgreSQL (#3728) ## Why make this change? This pull request is to address https://github.com/Azure/data-api-builder/issues/3094, which is a bug that happens because certain date/time SQL parameters are not being parsed to their appropriate .NET type (`DateTime` or `DateTimeOffset`) and are instead being treated as raw text. This is leading to type conversion issues when executing the underlying SQL query. ## What is this change? I have provided an overridden implementation of `MakeDbConnectionParam` in `BaseSqlQueryStructure.cs`. This method does some extra pre-processing for PostgreSQL parameters specifically by parsing each parameter into its appropriate system type before calling the base class implementation of `MakeDbConnectionParam` in `BaseQueryStructure.cs`. The system type of each parameter is retrieved from metadata of the underlying column that this parameter corresponds to via the method `GetColumnSystemType`. This change fixes the relevant issue because now parameters which correspond to an underlying `datetime` column in the schema are cast to their appropriate .NET type instead of falling through as text before building the final SQL query. Furthermore, this change is limited in scope to apply this parameter casting only when the underlying database is PostgreSQL. This ensures we do not see unexpected behavior with other database types where this sort of parameter casting may be unnecessary and/or erroneous. My original thought was to modify the method `ExtractValueFromIValueNode` in `ExecutionHelper.cs` to explicitly handle an `IValueNode` with a GraphQL scalar type of `SupportedHotChocolateTypes.DATETIME_TYPE` by parsing the `IValueNode` as a `DateTime` system type before being returned. For all practical purposes, this has the same desired effect as parsing the parameter in `MakeDbConnectionParam`, but the call to `ExtractValueFromIValueNode` happens upstream of the call to `MakeDbConnectionParam`. I believe that casting the parameters to their appropriate .NET type in `MakeDbConnectionParam` is appropriate because the method literally builds each parameter for the SQL query and the .NET type casting can be considered a part of this parameter building process. I do not think .NET type casting is as appropriate in `ExtractValueFromIValueNode` because this method serves the broader purpose of extracting a value from a GraphQL `IValueNode`, where the `IValueNode` could contain a scalar but it could also contain a variable that needs to be resolved recursively. For parameters that need to be cast to a .NET type, the control flow will go from `ExtractValueFromIValueNode` to eventually `MakeDbConnectionParam`, but again, `MakeDbConnectionParam` serves the sole purpose of building the parameter while `ExtractValueFromIValueNode` serves a broader purpose. ## How was this tested? - [ X] Integration Tests ## Sample Request(s) With query: ``` query { assignments(filter: { assignment_due_date: { gte: "2026-03-23T00:00:00.000Z" } }) { items { assignment_id assignment_name assignment_due_date } } } ``` Before: ``` SELECT * FROM public."Assignments" WHERE "Assignments"."assignment_due_date" >= '2026-03-23T00:00:00.000Z'::text; ``` After: ``` SELECT * FROM public."Assignments" WHERE "Assignments"."assignment_due_date" >= '2026-03-23T00:00:00.000Z'; ``` (cherry picked from commit eb3a58bfdf5685fd4d91a74b0777962c121658f1) --- .../BaseSqlQueryStructure.cs | 32 +++++++++++++++++-- src/Core/Services/GraphQLSchemaCreator.cs | 16 +++------- .../MetadataProviders/SqlMetadataProvider.cs | 10 +++--- .../GraphQLTypes/SupportedTypes.cs | 9 +++--- .../Sql/SchemaConverter.cs | 4 +-- .../Configuration/ConfigurationTests.cs | 4 +-- .../PostgreSqlGQLSupportedTypesTests.cs | 14 ++------ .../RestApiTests/Find/FindApiTestBase.cs | 16 +++++----- src/Service.Tests/SqlTests/SqlTestBase.cs | 6 ++-- 9 files changed, 61 insertions(+), 50 deletions(-) diff --git a/src/Core/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs index 99a5b1e72c..28c2699f62 100644 --- a/src/Core/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs @@ -84,6 +84,32 @@ public BaseSqlQueryStructure( } } + /// + public override string MakeDbConnectionParam(object? value, string? paramName = null, bool lengthOverride = false) + { + if (MetadataProvider.GetDatabaseType() is DatabaseType.PostgreSQL && + !string.IsNullOrEmpty(paramName) && + value is string stringValue && + GetUnderlyingSourceDefinition().Columns.TryGetValue(paramName, out ColumnDefinition? columnDefinition)) + { + Type columnSystemType = columnDefinition.SystemType; + if (columnSystemType != typeof(string)) + { + value = GetParamAsSystemType(stringValue, paramName, columnSystemType); + } + + // Npgsql requires DateTime with Kind=Unspecified for 'timestamp without time zone' columns. + // ParseParamAsSystemType returns Kind=Utc (via .UtcDateTime), which causes PostgreSQL to + // apply a UTC-to-local offset during comparison, producing incorrect filter results. + if (value is DateTime dtValue && dtValue.Kind == DateTimeKind.Utc && columnSystemType == typeof(DateTime)) + { + value = DateTime.SpecifyKind(dtValue, DateTimeKind.Unspecified); + } + } + + return base.MakeDbConnectionParam(value, paramName, lengthOverride); + } + /// /// For UPDATE (OVERWRITE) operation /// Adds result of (SourceDefinition.Columns minus MutationFields) to UpdateOperations with null values @@ -421,9 +447,9 @@ protected List GenerateOutputColumns() /// Tries to parse the string parameter to the given system type /// Useful for inferring parameter types for columns or procedure parameters /// - /// - /// - /// + /// The string value to parse. + /// The target system type for the parsed value. + /// The parameter parsed as the requested system type. /// protected static object ParseParamAsSystemType(string param, Type systemType) { diff --git a/src/Core/Services/GraphQLSchemaCreator.cs b/src/Core/Services/GraphQLSchemaCreator.cs index a7b51d8827..d449c396c0 100644 --- a/src/Core/Services/GraphQLSchemaCreator.cs +++ b/src/Core/Services/GraphQLSchemaCreator.cs @@ -295,9 +295,9 @@ private DocumentNode GenerateSqlGraphQLObjects(RuntimeEntities entities, Diction Dictionary> rolesAllowedForFields = new(); SourceDefinition sourceDefinition = sqlMetadataProvider.GetSourceDefinition(entityName); bool isStoredProcedure = entity.Source.Type is EntitySourceType.StoredProcedure; + EntityActionOperation operation = isStoredProcedure ? EntityActionOperation.Execute : EntityActionOperation.Read; foreach (string column in sourceDefinition.Columns.Keys) { - EntityActionOperation operation = isStoredProcedure ? EntityActionOperation.Execute : EntityActionOperation.Read; IEnumerable roles = _authorizationResolver.GetRolesForField(entityName, field: column, operation: operation); if (!rolesAllowedForFields.TryAdd(key: column, value: roles)) { @@ -309,7 +309,6 @@ private DocumentNode GenerateSqlGraphQLObjects(RuntimeEntities entities, Diction } } - // The roles allowed for Fields are the roles allowed to READ the fields, so any role that has a read definition for the field. // Only add objectTypeDefinition for GraphQL if it has a role definition defined for access. if (rolesAllowedForEntity.Any()) { @@ -397,23 +396,18 @@ private DocumentNode GenerateSqlGraphQLObjects(RuntimeEntities entities, Diction GenerateSourceTargetLinkingObjectDefinitions(objectTypes, linkingObjectTypes); } - // Return a list of all the object types to be exposed in the schema. - Dictionary fields = new(); - - // Add the DBOperationResult type to the schema NameNode nameNode = new(value: GraphQLUtils.DB_OPERATION_RESULT_TYPE); - FieldDefinitionNode field = GetDbOperationResultField(); - - fields.TryAdd(GraphQLUtils.DB_OPERATION_RESULT_FIELD_NAME, field); + // Add the DBOperationResult type to the schema objectTypes.Add(GraphQLUtils.DB_OPERATION_RESULT_TYPE, new ObjectTypeDefinitionNode( location: null, name: nameNode, description: null, new List(), new List(), - fields.Values.ToImmutableList())); + ImmutableList.Create(GetDbOperationResultField()))); + // Return a list of all the object types to be exposed in the schema. List nodes = new(objectTypes.Values); nodes.AddRange(enumTypes.Values); return new DocumentNode(nodes); @@ -748,7 +742,7 @@ private static FieldDefinitionNode GetDbOperationResultField() DocumentNode cosmosResult = GenerateCosmosGraphQLObjects(cosmosDataSourceNames, inputObjects); DocumentNode sqlResult = GenerateSqlGraphQLObjects(sql, inputObjects); // Create Root node with definitions from both cosmos and sql. - DocumentNode root = new(cosmosResult.Definitions.Concat(sqlResult.Definitions).ToImmutableList()); + DocumentNode root = cosmosResult.WithDefinitions(cosmosResult.Definitions.Concat(sqlResult.Definitions).ToImmutableList()); // Merge the inputobjectType definitions from cosmos and sql onto the root. return (root.WithDefinitions(root.Definitions.Concat(inputObjects.Values).ToImmutableList()), inputObjects); diff --git a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs index d93b4edbcf..af84260898 100644 --- a/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs +++ b/src/Core/Services/MetadataProviders/SqlMetadataProvider.cs @@ -169,7 +169,7 @@ public virtual string GetSchemaName(string entityName) { if (!EntityToDatabaseObject.TryGetValue(entityName, out DatabaseObject? databaseObject)) { - throw new DataApiBuilderException(message: $"Table Definition for {entityName} has not been inferred.", + throw new DataApiBuilderException(message: $"Database object for entity '{entityName}' has not been inferred.", statusCode: HttpStatusCode.InternalServerError, subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); } @@ -187,7 +187,7 @@ public string GetDatabaseObjectName(string entityName) { if (!EntityToDatabaseObject.TryGetValue(entityName, out DatabaseObject? databaseObject)) { - throw new DataApiBuilderException(message: $"Table Definition for {entityName} has not been inferred.", + throw new DataApiBuilderException(message: $"Database object for entity '{entityName}' has not been inferred.", statusCode: HttpStatusCode.InternalServerError, subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); } @@ -200,7 +200,7 @@ public SourceDefinition GetSourceDefinition(string entityName) { if (!EntityToDatabaseObject.TryGetValue(entityName, out DatabaseObject? databaseObject)) { - throw new DataApiBuilderException(message: $"Table Definition for {entityName} has not been inferred.", + throw new DataApiBuilderException(message: $"Database object for entity '{entityName}' has not been inferred.", statusCode: HttpStatusCode.InternalServerError, subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); } @@ -213,7 +213,7 @@ public StoredProcedureDefinition GetStoredProcedureDefinition(string entityName) { if (!EntityToDatabaseObject.TryGetValue(entityName, out DatabaseObject? databaseObject)) { - throw new DataApiBuilderException(message: $"Stored Procedure Definition for {entityName} has not been inferred.", + throw new DataApiBuilderException(message: $"Stored procedure definition for entity '{entityName}' has not been inferred.", statusCode: HttpStatusCode.InternalServerError, subStatusCode: DataApiBuilderException.SubStatusCodes.EntityNotFound); } @@ -1683,7 +1683,7 @@ private async Task ValidateDatabaseConnection() /// /// Using a data adapter, obtains the schema of the given table name - /// and adds the corresponding entity in the data set. + /// and adds the corresponding DataTable to the entities data set. /// private async Task FillSchemaForTableAsync( string schemaName, diff --git a/src/Service.GraphQLBuilder/GraphQLTypes/SupportedTypes.cs b/src/Service.GraphQLBuilder/GraphQLTypes/SupportedTypes.cs index c66c64a18d..e37609ae62 100644 --- a/src/Service.GraphQLBuilder/GraphQLTypes/SupportedTypes.cs +++ b/src/Service.GraphQLBuilder/GraphQLTypes/SupportedTypes.cs @@ -6,7 +6,7 @@ namespace Azure.DataApiBuilder.Service.GraphQLBuilder.GraphQLTypes /// /// Only used to group the supported type names under a class with a relevant name. /// The type names mentioned here are Hotchocolate scalar built in types. - /// The corresponding SQL type name may be different for e.g. UUID maps to Guid as the SQL type. + /// The corresponding SQL type name may be different for e.g. UUID maps to Guid as the .NET type. /// public static class SupportedHotChocolateTypes { @@ -32,20 +32,21 @@ public static class SupportedHotChocolateTypes // new name so the generated schema does not depend on a deprecated scalar. public const string BYTEARRAY_TYPE = "Base64String"; public const string DATETIME_TYPE = "DateTime"; - public const string DATETIMEOFFSET_TYPE = "DateTimeOffset"; public const string LOCALTIME_TYPE = "LocalTime"; public const string TIME_TYPE = "Time"; } /// - /// Class representing the sql datetime types supported by DAB which in addition to the sql datetime type, - /// all map to the same .NET type of DateTime and Hotchocolate scalar type of DateTime. + /// Class representing the sql datetime types supported by DAB. All types in this class + /// map to the Hotchocolate scalar type of DateTime. Most map to the .NET type of DateTime, + /// except DATETIMEOFFSET_TYPE which maps to the .NET type of DateTimeOffset. /// public static class SupportedDateTimeTypes { public const string DATE_TYPE = "date"; public const string SMALLDATETIME_TYPE = "smalldatetime"; public const string DATETIME2_TYPE = "datetime2"; + public const string DATETIMEOFFSET_TYPE = "datetimeoffset"; } /// diff --git a/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs b/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs index 9a65d3461d..25985a996d 100644 --- a/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs +++ b/src/Service.GraphQLBuilder/Sql/SchemaConverter.cs @@ -39,8 +39,8 @@ public enum AggregationType /// Runtime config information for the table. /// Key/Value Collection mapping entity name to the entity object, /// currently used to lookup relationship metadata. - /// Roles to add to authorize directive at the object level (applies to query/read ops). - /// Roles to add to authorize directive at the field level (applies to mutations). + /// Roles to add to authorize directive at the object level. + /// Roles to add to authorize directive at the field level. /// A GraphQL object type to be provided to a Hot Chocolate GraphQL document. public static ObjectTypeDefinitionNode GenerateObjectTypeDefinitionForDatabaseObject( string entityName, diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 71ef6ed6cf..5f7c4e2b36 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -1847,8 +1847,8 @@ public async Task TestSqlMetadataValidationForEntitiesWithInvalidSource() List exceptionMessagesList = configValidator.ConfigValidationExceptions.Select(x => x.Message).ToList(); Assert.IsTrue(exceptionMessagesList.Contains("The entity Book does not have a valid source object.")); Assert.IsTrue(exceptionMessagesList.Contains("The entity Publisher does not have a valid source object.")); - Assert.IsTrue(exceptionMessagesList.Contains("Table Definition for Book has not been inferred.")); - Assert.IsTrue(exceptionMessagesList.Contains("Table Definition for Publisher has not been inferred.")); + Assert.IsTrue(exceptionMessagesList.Contains("Database object for entity 'Book' has not been inferred.")); + Assert.IsTrue(exceptionMessagesList.Contains("Database object for entity 'Publisher' has not been inferred.")); Assert.IsTrue(exceptionMessagesList.Contains("Could not infer database object for source entity: Publisher in relationship: books. Check if the entity: Publisher is correctly defined in the config.")); Assert.IsTrue(exceptionMessagesList.Contains("Could not infer database object for target entity: Book in relationship: books. Check if the entity: Book is correctly defined in the config.")); } diff --git a/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/PostgreSqlGQLSupportedTypesTests.cs b/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/PostgreSqlGQLSupportedTypesTests.cs index b9459668d7..d06c5dea3c 100644 --- a/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/PostgreSqlGQLSupportedTypesTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLSupportedTypesTests/PostgreSqlGQLSupportedTypesTests.cs @@ -67,7 +67,7 @@ public async Task PG_real_graphql_single_filter_expectedValues( [DataRow(BOOLEAN_TYPE, "'false'", "false")] [DataRow(STRING_TYPE, "lksa;jdflasdf;alsdflksdfkldj", "\"lksa;jdflasdf;alsdflksdfkldj\"")] [DataTestMethod] - public async Task PGSQL_real_graphql_in_filter_expectedValues( + public async Task PGSQL_graphql_in_filter_expectedValues( string type, string sqlValue, string gqlValue) @@ -99,7 +99,7 @@ protected override string MakeQueryOnTypeTable( string orderBy = "id", string limit = "1") { - string formattedSelect = limit.Equals("1") ? "SELECT to_jsonb(subq3) AS DATA" : "SELECT json_agg(to_jsonb(subq3)) AS DATA"; + string formattedSelect = limit.Equals("1") ? "SELECT to_jsonb(subq3) AS DATA" : "SELECT COALESCE(json_agg(to_jsonb(subq3)), '[]'::json) AS DATA"; return @" " + formattedSelect + @" @@ -141,15 +141,5 @@ private static string ProperlyFormatTypeTableColumn(string columnName) return columnName; } } - - /// - /// Bypass DateTime GQL tests for PostreSql - /// - [DataTestMethod] - [Ignore] - public new void QueryTypeColumnFilterAndOrderByDateTime(string type, string filterOperator, string sqlValue, string gqlValue, string queryOperator) - { - Assert.Inconclusive("Test skipped for PostgreSql."); - } } } diff --git a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs index 289454e3dc..c216b4349c 100644 --- a/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs +++ b/src/Service.Tests/SqlTests/RestApiTests/Find/FindApiTestBase.cs @@ -39,7 +39,7 @@ await SetupAndRunRestApiTest( } /// - /// Tests the REST Api for FindByDateTimePk operation without a query string. + /// Tests the REST API for FindByDateTimePk operation without a query string. /// [TestMethod] public virtual async Task FindByDateTimePKTest() @@ -53,7 +53,7 @@ await SetupAndRunRestApiTest( } /// - /// Tests the REST Api for find many operation on stored procedure + /// Tests the REST API for find many operation on stored procedure /// Stored procedure result is not necessarily json. /// [TestMethod] @@ -71,7 +71,7 @@ await SetupAndRunRestApiTest( } /// - /// Tests the REST Api for find one operation using required parameter + /// Tests the REST API for find one operation using required parameter /// For Find operations, parameters must be passed in query string /// [TestMethod] @@ -89,7 +89,7 @@ await SetupAndRunRestApiTest( } /// - /// Tests the REST Api for Find operations on empty result sets + /// Tests the REST API for Find operations on empty result sets /// 1. GET an entity with no rows (empty table) /// 2. GET an entity with rows, filtered to none by query parameter /// Should be a 200 response with an empty array @@ -113,7 +113,7 @@ await SetupAndRunRestApiTest( } /// - /// Tests the Rest Api to validate that unique unicode + /// Tests the REST API to validate that unique unicode /// characters work in queries. /// /// @@ -128,7 +128,7 @@ await SetupAndRunRestApiTest( } /// - /// Tests the Rest Api to validate that queries work + /// Tests the REST API to validate that queries work /// when there is the same table name in two different /// schemas. In this test we have two tables both /// named magazines but with one in the schema "foo" and @@ -240,7 +240,7 @@ await SetupAndRunRestApiTest( /// /// Validates the repsonse when both $select and $orderby query strings are - /// used with Find API reqeusts. The response is expected to contain only the + /// used with Find API reqeusts. The response is expected to contain only the /// fields requested in $select clause. /// This test is executed against a table. /// @@ -260,7 +260,7 @@ await SetupAndRunRestApiTest( /// /// Validates the repsonse when both $select and $orderby query strings are - /// used with Find API reqeusts. The response is expected to contain only the + /// used with Find API reqeusts. The response is expected to contain only the /// fields requested in $select clause. /// This test is executed against a view. /// diff --git a/src/Service.Tests/SqlTests/SqlTestBase.cs b/src/Service.Tests/SqlTests/SqlTestBase.cs index 4e0fa5249c..94e6e8957c 100644 --- a/src/Service.Tests/SqlTests/SqlTestBase.cs +++ b/src/Service.Tests/SqlTests/SqlTestBase.cs @@ -423,9 +423,9 @@ await _queryExecutor.ExecuteQueryAsync( } /// - /// Does the setup required to perform a test of the REST Api for both - /// MsSql and Postgress. Shared setup logic eliminates some code duplication - /// between MsSql and Postgress. + /// Does the setup required to perform a test of the REST API for both + /// MsSql and Postgres. Shared setup logic eliminates some code duplication + /// between MsSql and Postgres. /// /// string represents the primary key route /// string represents the query string provided in URL From 7e0eebf57159d67431729edcb2a8f5820b6670fb Mon Sep 17 00:00:00 2001 From: Arpit Gupta <106474712+ar-guptaar@users.noreply.github.com> Date: Wed, 5 Aug 2026 12:59:45 +0530 Subject: [PATCH 4/6] =?UTF-8?q?Fix=20column=20mapping=20in=20GroupBy=20and?= =?UTF-8?q?=20aggregation=20queries=20to=20ensure=20corre=E2=80=A6=20(#375?= =?UTF-8?q?0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why make this change? There is a bug when running aggregation queries, wherein if a mapping backed column is present, the aggregation fails since it references the surfaced column and not the backed column. This pull request improves how group-by queries with mapped (aliased) columns are handled in SQL generation and adds a regression test to ensure correct behavior. The main focus is to ensure that SQL queries reference the backing (database) column names in the SELECT and GROUP BY clauses, while projecting the correct exposed (mapped) names in the results. ## What is this change? * Updated `ProcessGroupByField` and `ProcessGroupByFieldSelections` in `SqlQueryStructure.cs` to ensure that the backing (database) column name is used in the SQL query, while the exposed (mapped) field name is used as the label in the results. This resolves previous issues where the SELECT clause referenced the exposed name instead of the actual database column. [[1]](diffhunk://#diff-4b3df270873038f36ec2f627091120b7aafa271c0e2a2d257380f90602f5a298L907-R908) [[2]](diffhunk://#diff-4b3df270873038f36ec2f627091120b7aafa271c0e2a2d257380f90602f5a298L956-R956) ## How was this tested? - [x] Integration Tests - [X] Unit Tests * Added a new test method `TestSupportForGroupByAggregationWithMappedColumns` in `MsSqlGraphQLQueryTests.cs` to verify that group-by fields and aggregations both resolve to the correct backing columns, and that results are projected under the mapped names. This prevents regressions on mapped column handling in group-by queries. * Added missing import for `Azure.DataApiBuilder.Service.GraphQLBuilder.Queries` in `MsSqlGraphQLQueryTests.cs` to support the new test. --------- Co-authored-by: ARPIT GUPTA (cherry picked from commit 032eff937db95cefbdc5cd1e89317a1e7fb3745a) --- .../Sql Query Structures/SqlQueryStructure.cs | 4 +- src/Core/Resolvers/SqlPaginationUtil.cs | 15 +- .../MsSqlGraphQLQueryTests.cs | 331 +++++++++++++++++- 3 files changed, 339 insertions(+), 11 deletions(-) diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index 430b58c54f..f8e9336a31 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -905,7 +905,7 @@ private void ProcessGroupByField(FieldNode groupByField, IMiddlewareContext ctx, } GroupByMetadata.Fields[columnName] = new Column(DatabaseObject.SchemaName, DatabaseObject.Name, columnName, SourceAlias); - AddColumn(fieldName, backingColumn ?? fieldName); + AddColumn(columnName: columnName, labelName: fieldName); fieldsInArgument.Add(fieldName); } } @@ -953,7 +953,7 @@ private void ProcessGroupByFieldSelections(FieldNode groupByFieldSelection, Hash } string columnName = MetadataProvider.TryGetBackingColumn(EntityName, fieldName, out string? backingColumn) ? backingColumn : fieldName; - AddColumn(fieldName, columnName); + AddColumn(columnName: columnName, labelName: fieldName); } } diff --git a/src/Core/Resolvers/SqlPaginationUtil.cs b/src/Core/Resolvers/SqlPaginationUtil.cs index 852138ef09..3086e20d62 100644 --- a/src/Core/Resolvers/SqlPaginationUtil.cs +++ b/src/Core/Resolvers/SqlPaginationUtil.cs @@ -54,6 +54,10 @@ public static JsonDocument CreatePaginationConnectionFromJsonDocument(JsonDocume private static string GenerateGroupByObjectFromResult(GroupByMetadata groupByMetadata, IEnumerable rootEnumerated) { + HashSet aggregationAliases = groupByMetadata.Aggregations + .Select(aggregation => aggregation.Column.OperationAlias) + .ToHashSet(StringComparer.Ordinal); + JsonArray groupByArray = new(); foreach (JsonElement element in rootEnumerated) { @@ -62,16 +66,13 @@ private static string GenerateGroupByObjectFromResult(GroupByMetadata groupByMet JsonObject combinedObject = new(); foreach (JsonProperty property in element.EnumerateObject()) { - if (groupByMetadata.Fields.ContainsKey(property.Name)) + if (aggregationAliases.Contains(property.Name)) { - if (groupByMetadata.RequestedFields) - { - fieldObject.Add(property.Name, JsonNode.Parse(property.Value.GetRawText())); - } + aggregationObject.Add(property.Name, JsonNode.Parse(property.Value.GetRawText())); } - else + else if (groupByMetadata.RequestedFields) { - aggregationObject.Add(property.Name, JsonNode.Parse(property.Value.GetRawText())); + fieldObject.Add(property.Name, JsonNode.Parse(property.Value.GetRawText())); } } diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index 876424f0dd..3b31b05440 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -9,6 +9,7 @@ using System.Text.Json.Nodes; using System.Threading.Tasks; using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries; using Microsoft.VisualStudio.TestTools.UnitTesting; namespace Azure.DataApiBuilder.Service.Tests.SqlTests.GraphQLQueryTests @@ -757,8 +758,8 @@ public async Task TestStoredProcedureQueryWithNoDefaultInConfig() public async Task TestSupportForAggregationsWithAliases() { string msSqlQuery = @" - SELECT - MAX(categoryid) AS max, + SELECT + MAX(categoryid) AS max, MAX(price) AS max_price, MIN(price) AS min_price, AVG(price) AS avg_price, @@ -956,6 +957,332 @@ FROM stocks_price await TestSupportForGroupByNoAggregation(msSqlQuery); } + /// + /// Test to check GraphQL support for groupBy with aggregations on an entity whose columns are + /// mapped (aliased) in the runtime config (e.g. backing column '__column1' exposed as 'column1'). + /// Regression test: previously the SELECT clause referenced the exposed field name instead of the + /// backing column name, causing the query to fail because the exposed name is not a real database column. + /// This verifies that both the group-by field and the aggregation resolve to the backing column, + /// while the result is projected back under the exposed (mapped) names. + /// + [TestMethod] + public async Task TestSupportForGroupByAggregationWithMappedColumns() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + max_column1: max(field: column1) + count_column1: count(field: column1) + } + } + } + }"; + + string msSqlQuery = @" + SELECT + __column1 AS column1, + MAX(__column1) AS max_column1, + COUNT(__column1) AS count_column1 + FROM GQLmappings + GROUP BY __column1 + FOR JSON PATH, INCLUDE_NULL_VALUES"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + JsonElement groupByArray = actual.GetProperty(QueryBuilder.GROUP_BY_FIELD_NAME); + + string expected = await GetDatabaseResultAsync(msSqlQuery); + JsonDocument expectedDocument = JsonDocument.Parse(expected); + JsonElement expectedArray = expectedDocument.RootElement; + SqlTestHelper.AssertNumericAggregations(groupByArray, expectedArray); + } + + /// + /// End-to-end regression test for aggregations over a mapped (aliased) column. + /// Runs a real GraphQL request against the database and asserts the exact returned values + /// (rather than comparing against a re-generated SQL query). The 'gQLmappings' entity exposes + /// backing column '__column1' as 'column1', seeded with values {1, 3, 4, 5}. + /// This proves the aggregation functions resolve to the backing column while the result is + /// projected back under the exposed alias. + /// + [TestMethod] + public async Task TestSupportForAggregationWithMappedColumnReturnsExpectedValues() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings { + groupBy { + aggregations { + max_column1: max(field: column1) + min_column1: min(field: column1) + sum_column1: sum(field: column1) + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { + ""aggregations"": { + ""max_column1"": 5, + ""min_column1"": 1, + ""sum_column1"": 13, + ""count_column1"": 4 + } + } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// End-to-end regression test for a groupBy that selects only the mapped (aliased) field + /// (no aggregations). This specifically exercises ProcessGroupByFieldSelections in + /// SqlQueryStructure: the GROUP BY must reference the backing column '__column1' while the + /// projected 'fields' object must use the exposed alias 'column1'. Asserts the exact grouped + /// values {1, 3, 4, 5}. + /// + [TestMethod] + public async Task TestSupportForGroupByFieldsOnlyWithMappedColumnReturnsExpectedValues() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings { + groupBy(fields: [column1]) { + fields { + column1 + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""fields"": { ""column1"": 1 } }, + { ""fields"": { ""column1"": 3 } }, + { ""fields"": { ""column1"": 4 } }, + { ""fields"": { ""column1"": 5 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// End-to-end regression test combining a groupBy on the mapped (aliased) field with an + /// aggregation on the same mapped field. Because '__column1' (exposed as 'column1') is the + /// primary key, each value forms its own group with a count of 1. Asserts the exact shape and + /// values, verifying both the grouped 'fields' projection and the aggregation use the mapped + /// name in the response while targeting the backing column in SQL. + /// + [TestMethod] + public async Task TestSupportForGroupByFieldsAndAggregationWithMappedColumnReturnsExpectedValues() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""fields"": { ""column1"": 1 }, ""aggregations"": { ""count_column1"": 1 } }, + { ""fields"": { ""column1"": 3 }, ""aggregations"": { ""count_column1"": 1 } }, + { ""fields"": { ""column1"": 4 }, ""aggregations"": { ""count_column1"": 1 } }, + { ""fields"": { ""column1"": 5 }, ""aggregations"": { ""count_column1"": 1 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// Regression test for orderBy on a mapped (aliased) column within a groupBy query. + /// The orderBy validation resolves the exposed field to its backing column and requires it to be + /// present in GroupByMetadata.Fields (keyed by backing column). This verifies that ordering a + /// groupBy result by a mapped column is accepted (not incorrectly rejected with + /// "OrderBy field '...' must be present in the groupBy fields.") and that the DESC order is + /// actually applied to the returned groups. + /// + [TestMethod] + public async Task TestSupportForGroupByWithOrderByOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: DESC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""fields"": { ""column1"": 5 } }, + { ""fields"": { ""column1"": 4 } }, + { ""fields"": { ""column1"": 3 } }, + { ""fields"": { ""column1"": 1 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// End-to-end regression test for a field-level HAVING filter on an aggregation over a mapped + /// (aliased) column, without selecting the grouped fields. Groups by the mapped 'column1' + /// (backing '__column1', PK values {1, 3, 4, 5}); HAVING max(column1) > 3 keeps only groups 4 + /// and 5. Verifies the HAVING clause targets the backing column and the response projects only + /// the requested aggregation under its alias. + /// + [TestMethod] + public async Task TestSupportForGroupByHavingAggregationOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings { + groupBy(fields: [column1]) { + aggregations { + max_column1: max(field: column1, having: { gt: 3 }) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""aggregations"": { ""max_column1"": 4 } }, + { ""aggregations"": { ""max_column1"": 5 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// End-to-end regression test combining grouped fields, multiple aggregations, and a field-level + /// HAVING filter on a mapped (aliased) column (no ordering). Groups by the mapped 'column1' + /// (backing '__column1', PK values {1, 3, 4, 5}); HAVING max(column1) > 3 keeps groups 4 and 5. + /// Verifies the grouped 'fields' projection, the non-filtered aggregation, and the HAVING-filtered + /// aggregation all use the mapped name in the response while targeting the backing column in SQL. + /// + [TestMethod] + public async Task TestSupportForGroupByFieldsAggregationsAndHavingOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + max_column1: max(field: column1, having: { gt: 3 }) + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { + ""fields"": { ""column1"": 4 }, + ""aggregations"": { ""max_column1"": 4, ""count_column1"": 1 } + }, + { + ""fields"": { ""column1"": 5 }, + ""aggregations"": { ""max_column1"": 5, ""count_column1"": 1 } + } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// End-to-end regression test exercising every groupBy dimension together on a mapped (aliased) + /// column: grouping by the mapped field, selecting the mapped field, multiple aggregations on the + /// mapped field, a field-level HAVING filter on one aggregation, and an orderBy on the mapped field. + /// The 'gQLmappings' entity exposes backing '__column1' as 'column1' (PK, values {1, 3, 4, 5}), so + /// each value forms its own group. HAVING max(column1) > 3 keeps only groups 4 and 5, and + /// orderBy DESC returns them as 5 then 4. This proves the backing column is used consistently in + /// GROUP BY, HAVING and ORDER BY while the response projects the mapped names. + /// + [TestMethod] + public async Task TestSupportForGroupByWithFieldsAggregationsHavingAndOrderByOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: DESC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + max_column1: max(field: column1, having: { gt: 3 }) + min_column1: min(field: column1) + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { + ""fields"": { ""column1"": 5 }, + ""aggregations"": { ""max_column1"": 5, ""min_column1"": 5, ""count_column1"": 1 } + }, + { + ""fields"": { ""column1"": 4 }, + ""aggregations"": { ""max_column1"": 4, ""min_column1"": 4, ""count_column1"": 1 } + } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + /// /// Test to check that an exception is thrown when both items and groupBy are present in the same query. /// From 3699802a2580a2a3024cf327070065a90d18390a Mon Sep 17 00:00:00 2001 From: Arjun Narendra Date: Thu, 6 Aug 2026 14:38:12 -0700 Subject: [PATCH 5/6] Add groupby/aggregation support for PostgreSQL in GraphQL (#3741) ## 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? - [X] Integration Tests ## Sample Request(s) ``` query { stocks_prices { groupBy(fields: [categoryid, pieceid]) { fields { categoryid pieceid } aggregations { total: sum(field: price) count: count(field: pieceid, having: { gt: 5 }) } } } } ``` Previously, a query such as the one above would fail because the `groupBy` field would not have been attached to the `stocks_prices` object type. This PR adds the `groupBy` field to the relevant list query entity types. The `groupBy` field serves as a window to the rest of the groupBy/aggregation query. --------- Co-authored-by: Anusha Kolan (cherry picked from commit 7ad4f4979ce515ced5607b68451dde5613db0548) --- config-generators/postgresql-commands.txt | 1 + src/Core/Resolvers/BaseSqlQueryBuilder.cs | 92 +++- src/Core/Resolvers/BaseTSqlQueryBuilder.cs | 86 --- src/Core/Resolvers/PostgresQueryBuilder.cs | 8 +- .../Sql Query Structures/SqlQueryStructure.cs | 25 +- .../Queries/QueryBuilder.cs | 1 + src/Service.Tests/DatabaseSchema-MsSql.sql | 16 +- .../DatabaseSchema-PostgreSql.sql | 18 +- .../GraphQLBuilder/QueryBuilderTests.cs | 4 +- ...ingRuntimeConfigForPostgreSql.verified.txt | 29 + .../MsSqlGraphQLQueryTests.cs | 2 + .../PostgreSqlGraphQLQueryTests.cs | 494 ++++++++++++++---- src/Service.Tests/dab-config.PostgreSql.json | 29 + 13 files changed, 602 insertions(+), 203 deletions(-) diff --git a/config-generators/postgresql-commands.txt b/config-generators/postgresql-commands.txt index f249103253..7eb68c12f1 100644 --- a/config-generators/postgresql-commands.txt +++ b/config-generators/postgresql-commands.txt @@ -176,3 +176,4 @@ add dbo_DimAccount --config "dab-config.PostgreSql.json" --source "dimaccount" - update dbo_DimAccount --config "dab-config.PostgreSql.json" --map "parentaccountkey:ParentAccountKey,accountkey:AccountKey" update dbo_DimAccount --config "dab-config.PostgreSql.json" --relationship parent_account --target.entity dbo_DimAccount --cardinality one --relationship.fields "parentaccountkey:accountkey" update dbo_DimAccount --config "dab-config.PostgreSql.json" --relationship child_accounts --target.entity dbo_DimAccount --cardinality many --relationship.fields "accountkey:parentaccountkey" +add DateOnlyTable --config "dab-config.PostgreSql.json" --source "date_only_table" --permissions "anonymous:*" --rest true --graphql true --source.key-fields "event_date" diff --git a/src/Core/Resolvers/BaseSqlQueryBuilder.cs b/src/Core/Resolvers/BaseSqlQueryBuilder.cs index a509e9d842..ec4d799a57 100644 --- a/src/Core/Resolvers/BaseSqlQueryBuilder.cs +++ b/src/Core/Resolvers/BaseSqlQueryBuilder.cs @@ -193,6 +193,92 @@ protected virtual string Build(AggregationColumn column, bool useAlias = false) return $"{column.Type.ToString()}({columnName}) {appendAlias}"; } + /// + /// Build the Group By Clause needed to append to the main query + /// + /// Sql query structure to build query on + /// SQL query with group-by clause + protected virtual string BuildGroupBy(SqlQueryStructure structure) + { + // Add GROUP BY clause if there are any group by columns + if (structure.GroupByMetadata.Fields.Any()) + { + return $" GROUP BY {string.Join(", ", structure.GroupByMetadata.Fields.Values.Select(c => Build(c)))}"; + } + + return string.Empty; + } + + /// + /// Build the Having clause needed to append to the main query + /// + /// Sql query structure to build query on + /// SQL query with having clause + protected virtual string BuildHaving(SqlQueryStructure structure) + { + if (structure.GroupByMetadata.Aggregations.Count > 0) + { + List? havingPredicates = structure.GroupByMetadata.Aggregations + .SelectMany(aggregation => aggregation.HavingPredicates ?? new List()) + .ToList(); + + if (havingPredicates.Any()) + { + return $" HAVING {Build(havingPredicates)}"; + } + } + + return string.Empty; + } + + /// + /// Build the aggregation columns needed to append to the main query + /// + /// Sql query structure to build query on + /// SQL query with aggregation columns + protected virtual string BuildAggregationColumns(SqlQueryStructure structure) + { + string aggregations = string.Empty; + if (structure.GroupByMetadata.Aggregations.Count > 0) + { + if (structure.Columns.Any()) + { + aggregations = $",{BuildAggregationColumns(structure.GroupByMetadata)}"; + } + else + { + aggregations = $"{BuildAggregationColumns(structure.GroupByMetadata)}"; + } + } + + return aggregations; + } + + /// + /// Build the aggregation columns needed to append to the main query + /// + /// GroupByMetadata + /// SQL query with aggregation columns + protected virtual string BuildAggregationColumns(GroupByMetadata metadata) + { + return string.Join(", ", metadata.Aggregations.Select(aggregation => Build(aggregation.Column, useAlias: true))); + } + + /// + /// Build the Order By clause needed to append to the main query + /// + /// Sql query structure to build query on + /// SQL query with order-by clause + protected virtual string BuildOrderBy(SqlQueryStructure structure) + { + if (structure.OrderByColumns.Any()) + { + return $" ORDER BY {Build(structure.OrderByColumns)}"; + } + + return string.Empty; + } + /// /// Build orderby column as /// {SourceAlias}.{ColumnName} {direction} @@ -447,7 +533,7 @@ public virtual string BuildForeignKeyInfoQuery(int numberOfParameters) // constraint columns - one inner join for the columns from the 'Referencing table' // and the other join for the columns from the 'Referenced Table'. string foreignKeyQuery = $@" -SELECT +SELECT ReferentialConstraints.CONSTRAINT_NAME {QuoteIdentifier(nameof(ForeignKeyDefinition))}, ReferencingColumnUsage.TABLE_SCHEMA {QuoteIdentifier($"Referencing{nameof(DatabaseObject.SchemaName)}")}, @@ -457,9 +543,9 @@ public virtual string BuildForeignKeyInfoQuery(int numberOfParameters) {QuoteIdentifier($"Referenced{nameof(DatabaseObject.SchemaName)}")}, ReferencedColumnUsage.TABLE_NAME {QuoteIdentifier($"Referenced{nameof(SourceDefinition)}")}, ReferencedColumnUsage.COLUMN_NAME {QuoteIdentifier(nameof(ForeignKeyDefinition.ReferencedColumns))} -FROM +FROM INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS ReferentialConstraints - INNER JOIN + INNER JOIN INFORMATION_SCHEMA.KEY_COLUMN_USAGE ReferencingColumnUsage ON ReferentialConstraints.CONSTRAINT_CATALOG = ReferencingColumnUsage.CONSTRAINT_CATALOG AND ReferentialConstraints.CONSTRAINT_SCHEMA = ReferencingColumnUsage.CONSTRAINT_SCHEMA diff --git a/src/Core/Resolvers/BaseTSqlQueryBuilder.cs b/src/Core/Resolvers/BaseTSqlQueryBuilder.cs index 1bf7e73ef4..7d0d2ef4b7 100644 --- a/src/Core/Resolvers/BaseTSqlQueryBuilder.cs +++ b/src/Core/Resolvers/BaseTSqlQueryBuilder.cs @@ -1,5 +1,4 @@ using Azure.DataApiBuilder.Config.ObjectModel; -using Azure.DataApiBuilder.Core.Models; namespace Azure.DataApiBuilder.Core.Resolvers { @@ -43,90 +42,5 @@ protected virtual string BuildPredicates(SqlQueryStructure structure) Build(structure.PaginationMetadata.PaginationPredicate)); } - /// - /// Build the Group By Clause needed to append to the main query - /// - /// Sql query structure to build query on - /// SQL query with group-by clause - protected virtual string BuildGroupBy(SqlQueryStructure structure) - { - // Add GROUP BY clause if there are any group by columns - if (structure.GroupByMetadata.Fields.Any()) - { - return $" GROUP BY {string.Join(", ", structure.GroupByMetadata.Fields.Values.Select(c => Build(c)))}"; - } - - return string.Empty; - } - - /// - /// Build the Having clause needed to append to the main query - /// - /// Sql query structure to build query on - /// SQL query with having clause - protected virtual string BuildHaving(SqlQueryStructure structure) - { - if (structure.GroupByMetadata.Aggregations.Count > 0) - { - List? havingPredicates = structure.GroupByMetadata.Aggregations - .SelectMany(aggregation => aggregation.HavingPredicates ?? new List()) - .ToList(); - - if (havingPredicates.Any()) - { - return $" HAVING {Build(havingPredicates)}"; - } - } - - return string.Empty; - } - - /// - /// Build the Order By clause needed to append to the main query - /// - /// Sql query structure to build query on - /// SQL query with order-by clause - protected virtual string BuildOrderBy(SqlQueryStructure structure) - { - if (structure.OrderByColumns.Any()) - { - return $" ORDER BY {Build(structure.OrderByColumns)}"; - } - - return string.Empty; - } - - /// - /// Build the aggregation columns needed to append to the main query - /// - /// Sql query structure to build query on - /// SQL query with aggregation columns - protected virtual string BuildAggregationColumns(SqlQueryStructure structure) - { - string aggregations = string.Empty; - if (structure.GroupByMetadata.Aggregations.Count > 0) - { - if (structure.Columns.Any()) - { - aggregations = $",{BuildAggregationColumns(structure.GroupByMetadata)}"; - } - else - { - aggregations = $"{BuildAggregationColumns(structure.GroupByMetadata)}"; - } - } - - return aggregations; - } - - /// - /// Build the aggregation columns needed to append to the main query - /// - /// GroupByMetadata - /// SQL query with aggregation columns - protected virtual string BuildAggregationColumns(GroupByMetadata metadata) - { - return string.Join(", ", metadata.Aggregations.Select(aggregation => Build(aggregation.Column, useAlias: true))); - } } } diff --git a/src/Core/Resolvers/PostgresQueryBuilder.cs b/src/Core/Resolvers/PostgresQueryBuilder.cs index 00ac571149..9a768f82a0 100644 --- a/src/Core/Resolvers/PostgresQueryBuilder.cs +++ b/src/Core/Resolvers/PostgresQueryBuilder.cs @@ -41,10 +41,14 @@ public string Build(SqlQueryStructure structure) Build(structure.Predicates), Build(structure.PaginationMetadata.PaginationPredicate)); - string query = $"SELECT {MakeSelectColumns(structure)}" + string aggregations = BuildAggregationColumns(structure); + + string query = $"SELECT {MakeSelectColumns(structure)}{aggregations}" + $" FROM {fromSql}" + $" WHERE {predicates}" - + $" ORDER BY {Build(structure.OrderByColumns)}" + + BuildGroupBy(structure) + + BuildHaving(structure) + + BuildOrderBy(structure) + $" LIMIT {structure.Limit()}"; string subqueryName = QuoteIdentifier($"subq{structure.Counter.Next()}"); diff --git a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs index f8e9336a31..26e7a020d5 100644 --- a/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs +++ b/src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs @@ -378,6 +378,26 @@ private List PrimaryKeyAsOrderByColumns() return _primaryKeyAsOrderByColumns; } + /// + /// Exposes the groupBy fields of this structure as a list of OrderByColumn, + /// giving groupBy queries a deterministic row order (in the absence of an explicit + /// client-provided orderBy) since Postgres/MSSQL do not guarantee GROUP BY row order otherwise. + /// + private List GroupByColumnsAsOrderByColumns() + { + List groupByColumnsAsOrderByColumns = new(); + + foreach (Column column in GroupByMetadata.Fields.Values) + { + groupByColumnsAsOrderByColumns.Add(new OrderByColumn(tableSchema: column.TableSchema, + tableName: column.TableName, + columnName: column.ColumnName, + tableAlias: column.TableAlias)); + } + + return groupByColumnsAsOrderByColumns; + } + /// /// Private constructor that is used for recursive query generation, /// for each subquery that's necessary to resolve a nested GraphQL @@ -511,8 +531,9 @@ private SqlQueryStructure( } } - // primary key should only be added to order by for non groupby queries. - OrderByColumns = isGroupByQuery ? [] : PrimaryKeyAsOrderByColumns(); + // groupBy queries default to ordering by the grouped columns (since Postgres/MSSQL + // don't guarantee GROUP BY row order otherwise); non-groupBy queries default to the primary key. + OrderByColumns = isGroupByQuery ? GroupByColumnsAsOrderByColumns() : PrimaryKeyAsOrderByColumns(); if (IsListQuery && queryParams.ContainsKey(QueryBuilder.ORDER_BY_FIELD_NAME)) { object? orderByObject = queryParams[QueryBuilder.ORDER_BY_FIELD_NAME]; diff --git a/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs b/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs index a2cc63b2c2..d744dc549e 100644 --- a/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs +++ b/src/Service.GraphQLBuilder/Queries/QueryBuilder.cs @@ -36,6 +36,7 @@ public static class QueryBuilder { DatabaseType.MSSQL, DatabaseType.DWSQL, + DatabaseType.PostgreSQL, }; /// diff --git a/src/Service.Tests/DatabaseSchema-MsSql.sql b/src/Service.Tests/DatabaseSchema-MsSql.sql index 4e87394aee..4ebeb36317 100644 --- a/src/Service.Tests/DatabaseSchema-MsSql.sql +++ b/src/Service.Tests/DatabaseSchema-MsSql.sql @@ -1,6 +1,8 @@ -- Copyright (c) Microsoft Corporation. -- Licensed under the MIT License. +SET QUOTED_IDENTIFIER ON; + BEGIN TRANSACTION DROP SECURITY POLICY IF EXISTS revenuesSecPolicy; DROP FUNCTION IF EXISTS revenuesPredicate; @@ -489,8 +491,8 @@ WITH cteN(Number) AS CROSS JOIN sys.all_columns AS s2 ) INSERT INTO bookmarks ([id], [bkname]) -SELECT -[Number], +SELECT +[Number], 'Test Item #' + format([Number], '00000') FROM cteN WHERE [Number] <= @UpperBound; SET IDENTITY_INSERT bookmarks OFF @@ -503,8 +505,8 @@ WITH cteN(Number) AS CROSS JOIN sys.all_columns AS s2 ) INSERT INTO mappedbookmarks ([id], [bkname]) -SELECT -[Number], +SELECT +[Number], 'Test Item #' + format([Number], '00000') FROM cteN WHERE [Number] <= @UpperBound; @@ -822,7 +824,7 @@ CREATE TABLE date_only_table ( event_timestamp datetime NOT NULL ); -INSERT INTO date_only_table( event_date, event_time, event_timestamp) -VALUES ('2023-01-01', '08:30:00', '2023-01-01 08:30:00'), - ('2023-02-15', '12:45:00', '2023-02-15 12:45:00'), +INSERT INTO date_only_table( event_date, event_time, event_timestamp) +VALUES ('2023-01-01', '08:30:00', '2023-01-01 08:30:00'), + ('2023-02-15', '12:45:00', '2023-02-15 12:45:00'), ('2023-03-30', '17:15:00', '2023-03-30 17:15:00'); diff --git a/src/Service.Tests/DatabaseSchema-PostgreSql.sql b/src/Service.Tests/DatabaseSchema-PostgreSql.sql index 523e96c22f..21f36eac37 100644 --- a/src/Service.Tests/DatabaseSchema-PostgreSql.sql +++ b/src/Service.Tests/DatabaseSchema-PostgreSql.sql @@ -40,6 +40,7 @@ DROP TABLE IF EXISTS default_with_function_table; DROP TABLE IF EXISTS users; DROP TABLE IF EXISTS user_profiles; DROP TABLE IF EXISTS dimaccount; +DROP TABLE IF EXISTS date_only_table; DROP FUNCTION IF EXISTS insertCompositeView; DROP SCHEMA IF EXISTS foo; @@ -349,14 +350,14 @@ INSERT INTO bookmarks (id, bkname) SELECT value, CONCAT('Test Item #' , value) -FROM +FROM GENERATE_SERIES(1, 10000, 1) as value; INSERT INTO mappedbookmarks (id, bkname) SELECT value, CONCAT('Test Item #' , value) -FROM +FROM GENERATE_SERIES(1, 10000, 1) as value; INSERT INTO GQLmappings(__column1, __column2, column3) VALUES (1, 'Incompatible GraphQL Name', 'Compatible GraphQL Name'); @@ -366,7 +367,7 @@ INSERT INTO GQLmappings(__column1, __column2, column3) VALUES (5, 'Filtered Reco INSERT INTO publishers(id, name) VALUES (1234, 'Big Company'), (2345, 'Small Town Publisher'), (2323, 'TBD Publishing One'), (2324, 'TBD Publishing Two Ltd'), (1940, 'Policy Publisher 01'), (1941, 'Policy Publisher 02'), (1156, 'The First Publisher'); INSERT INTO clubs(id, name) VALUES (1111, 'Manchester United'), (1112, 'FC Barcelona'), (1113, 'Real Madrid'); INSERT INTO players(id, name, current_club_id, new_club_id) - VALUES + VALUES (1, 'Cristiano Ronaldo', 1113, 1111), (2, 'Leonel Messi', 1112, 1113); INSERT INTO authors(id, name, birthdate) VALUES (123, 'Jelte', '2001-01-01'), (124, 'Aniruddh', '2002-02-02'), (125, 'Aniruddh', '2001-01-01'), (126, 'Aaron', '2001-01-01'); @@ -470,3 +471,14 @@ $$ LANGUAGE plpgsql; CREATE TRIGGER insertCompositeViewTrigger INSTEAD OF INSERT ON books_publishers_view_composite_insertable FOR EACH ROW EXECUTE PROCEDURE insertCompositeView(); + +CREATE TABLE date_only_table ( + event_date date NOT NULL, + event_time time NOT NULL, + event_timestamp timestamp NOT NULL +); + +INSERT INTO date_only_table(event_date, event_time, event_timestamp) +VALUES ('2023-01-01', '08:30:00', '2023-01-01 08:30:00'), + ('2023-02-15', '12:45:00', '2023-02-15 12:45:00'), + ('2023-03-30', '17:15:00', '2023-03-30 17:15:00'); diff --git a/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs b/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs index 93a3df9778..54fb55d909 100644 --- a/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs +++ b/src/Service.Tests/GraphQLBuilder/QueryBuilderTests.cs @@ -578,12 +578,12 @@ public void GenerateReturnType_ExcludesGroupByField_WhenAggregationDisabled() /// Tests that QueryBuilder.Build correctly adds or omits the groupBy field on the /// connection type based on whether the database type is in /// . - /// MSSQL and DWSQL are supported; other types (e.g. PostgreSQL) are not. + /// MSSQL, DWSQL, and PostgreSQL are supported; other types (e.g. MySQL) are not. /// [DataTestMethod] [DataRow(DatabaseType.MSSQL, true, DisplayName = "MSSQL: groupBy field present when aggregation enabled")] [DataRow(DatabaseType.DWSQL, true, DisplayName = "DWSQL: groupBy field present when aggregation enabled")] - [DataRow(DatabaseType.PostgreSQL, false, DisplayName = "PostgreSQL: groupBy field absent (not in AggregationEnabledDatabaseTypes)")] + [DataRow(DatabaseType.PostgreSQL, true, DisplayName = "PostgreSQL: groupBy field present when aggregation enabled")] [DataRow(DatabaseType.MySQL, false, DisplayName = "MySQL: groupBy field absent (not in AggregationEnabledDatabaseTypes)")] [TestCategory("Query Builder - Aggregation")] public void Build_WithAggregationEnabled_GroupByPresenceMatchesDatabaseSupport( diff --git a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt index 05f042b619..8c849f8f8f 100644 --- a/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt +++ b/src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForPostgreSql.verified.txt @@ -2726,6 +2726,35 @@ } } } + }, + { + DateOnlyTable: { + Source: { + Object: date_only_table, + Type: Table, + KeyFields: [ + event_date + ] + }, + GraphQL: { + Singular: DateOnlyTable, + Plural: DateOnlyTables, + Enabled: true + }, + Rest: { + Enabled: true + }, + Permissions: [ + { + Role: anonymous, + Actions: [ + { + Action: * + } + ] + } + ] + } } ] } \ No newline at end of file diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index 3b31b05440..c6bb28fcff 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -945,12 +945,14 @@ HAVING SUM(price) > 50 AND COUNT(pieceid) <= 100 [TestMethod] public async Task TestSupportForGroupByNoAggregation() { + // ORDER BY makes explicit the row order that groupBy defaults to (declared groupBy field order) absent an orderBy argument. string msSqlQuery = @" SELECT categoryid, pieceid FROM stocks_price GROUP BY categoryid, pieceid + ORDER BY categoryid, pieceid FOR JSON PATH, INCLUDE_NULL_VALUES"; // Execute the test for the SQL query diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/PostgreSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/PostgreSqlGraphQLQueryTests.cs index 9136d2f5a6..5b9fd11ab8 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/PostgreSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/PostgreSqlGraphQLQueryTests.cs @@ -1,8 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Text.Json; using System.Threading.Tasks; using Azure.DataApiBuilder.Config.ObjectModel; +using Azure.DataApiBuilder.Service.GraphQLBuilder.Queries; using Microsoft.VisualStudio.TestTools.UnitTesting; namespace Azure.DataApiBuilder.Service.Tests.SqlTests.GraphQLQueryTests @@ -438,45 +440,49 @@ await TestConfigTakesPrecedenceForRelationshipFieldsOverDB( /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForAggregationsWithAliases() { - string msSqlQuery = @" - SELECT - MAX(categoryid) AS max, - MAX(price) AS max_price, - MIN(price) AS min_price, - AVG(price) AS avg_price, - SUM(price) AS sum_price - FROM stocks_price - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + MAX(categoryid) AS max, + MAX(price) AS max_price, + MIN(price) AS min_price, + AVG(price) AS avg_price, + SUM(price) AS sum_price + FROM stocks_price + ) AS table0"; // Execute the test for the SQL query - await TestSupportForAggregationsWithAliases(msSqlQuery); + await TestSupportForAggregationsWithAliases(postgresQuery); } /// /// Test to check GraphQL support for aggregations with aliases and groupby. /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. + /// Note: an explicit ORDER BY categoryid is required here because DAB's actual generated query selects + /// categoryid as an extra column, which changes Postgres's query plan (and therefore GROUP BY row order) + /// compared to a query that only selects the aggregates. /// [TestMethod] - [Ignore] public async Task TestSupportForGroupByAggregationsWithAliases() { - string msSqlQuery = @" - SELECT - MAX(categoryid) AS max, - MAX(price) AS max_price, - MIN(price) AS min_price, - AVG(price) AS avg_price, - SUM(price) AS sum_price, - COUNT(categoryid) AS count - FROM stocks_price - GROUP BY categoryid - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + MAX(categoryid) AS max, + MAX(price) AS max_price, + MIN(price) AS min_price, + AVG(price) AS avg_price, + SUM(price) AS sum_price, + COUNT(categoryid) AS count + FROM stocks_price + GROUP BY categoryid + ORDER BY categoryid + ) AS table0"; // Execute the test for the SQL query - await TestSupportForGroupByAggregationsWithAliases(msSqlQuery); + await TestSupportForGroupByAggregationsWithAliases(postgresQuery); } /// @@ -484,17 +490,17 @@ GROUP BY categoryid /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForMinAggregation() { - string msSqlQuery = @" - SELECT - MIN(price) AS min_price - FROM stocks_price - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + MIN(price) AS min_price + FROM stocks_price + ) AS table0"; // Execute the test for the SQL query - await TestSupportForMinAggregation(msSqlQuery); + await TestSupportForMinAggregation(postgresQuery); } /// @@ -502,17 +508,17 @@ FROM stocks_price /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForMaxAggregation() { - string msSqlQuery = @" - SELECT - MAX(price) AS max_price - FROM stocks_price - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + MAX(price) AS max_price + FROM stocks_price + ) AS table0"; // Execute the test for the SQL query - await TestSupportForMaxAggregation(msSqlQuery); + await TestSupportForMaxAggregation(postgresQuery); } /// @@ -520,17 +526,17 @@ FROM stocks_price /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForAvgAggregation() { - string msSqlQuery = @" - SELECT - AVG(price) AS avg_price - FROM stocks_price - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + AVG(price) AS avg_price + FROM stocks_price + ) AS table0"; // Execute the test for the SQL query - await TestSupportForAvgAggregation(msSqlQuery); + await TestSupportForAvgAggregation(postgresQuery); } /// @@ -538,17 +544,17 @@ FROM stocks_price /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForSumAggregation() { - string msSqlQuery = @" - SELECT - SUM(price) AS sum_price - FROM stocks_price - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + SUM(price) AS sum_price + FROM stocks_price + ) AS table0"; // Execute the test for the SQL query - await TestSupportForSumAggregation(msSqlQuery); + await TestSupportForSumAggregation(postgresQuery); } /// @@ -556,17 +562,17 @@ FROM stocks_price /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForCountAggregation() { - string msSqlQuery = @" - SELECT - COUNT(categoryid) AS count_categoryid - FROM stocks_price - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + COUNT(categoryid) AS count_categoryid + FROM stocks_price + ) AS table0"; // Execute the test for the SQL query - await TestSupportForCountAggregation(msSqlQuery); + await TestSupportForCountAggregation(postgresQuery); } /// @@ -574,18 +580,19 @@ FROM stocks_price /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForHavingAggregation() { - string msSqlQuery = @" - SELECT - SUM(price) AS sum_price - FROM stocks_price - HAVING SUM(price) > 50 - FOR JSON PATH, INCLUDE_NULL_VALUES"; + // HAVING may exclude the only row; json_agg over zero rows returns NULL, not []. + string postgresQuery = @" + SELECT COALESCE(json_agg(to_jsonb(table0)), '[]') FROM ( + SELECT + MAX(id) AS max + FROM publishers + HAVING MAX(id) > 2346 + ) AS table0"; // Execute the test for the SQL query - await TestSupportForHavingAggregation(msSqlQuery); + await TestSupportForHavingAggregation(postgresQuery); } /// @@ -593,19 +600,19 @@ HAVING SUM(price) > 50 /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForGroupByHavingAggregation() { - string msSqlQuery = @" - SELECT - SUM(price) AS sum_price - FROM stocks_price - GROUP BY categoryid, pieceid - HAVING SUM(price) > 50 - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + SUM(price) AS sum_price + FROM stocks_price + GROUP BY categoryid, pieceid + HAVING SUM(price) > 50 + ) AS table0"; // Execute the test for the SQL query - await TestSupportForGroupByHavingAggregation(msSqlQuery); + await TestSupportForGroupByHavingAggregation(postgresQuery); } /// @@ -613,22 +620,22 @@ HAVING SUM(price) > 50 /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForGroupByHavingFieldsAggregation() { - string msSqlQuery = @" - SELECT - categoryid, - pieceid, - SUM(price) AS sum_price, - COUNT(pieceid) AS count_piece - FROM stocks_price - GROUP BY categoryid, pieceid - HAVING SUM(price) > 50 AND COUNT(pieceid) <= 100 - FOR JSON PATH, INCLUDE_NULL_VALUES"; + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + categoryid, + pieceid, + SUM(price) AS sum_price, + COUNT(pieceid) AS count_piece + FROM stocks_price + GROUP BY categoryid, pieceid + HAVING SUM(price) > 50 AND COUNT(pieceid) <= 100 + ) AS table0"; // Execute the test for the SQL query - await TestSupportForGroupByHavingFieldsAggregation(msSqlQuery); + await TestSupportForGroupByHavingFieldsAggregation(postgresQuery); } /// @@ -636,23 +643,314 @@ HAVING SUM(price) > 50 AND COUNT(pieceid) <= 100 /// This test verifies that the SQL query results are correctly mapped to the expected GraphQL format. /// [TestMethod] - [Ignore] public async Task TestSupportForGroupByNoAggregation() { - string msSqlQuery = @" - SELECT - categoryid, - pieceid - FROM stocks_price - GROUP BY categoryid, pieceid - FOR JSON PATH, INCLUDE_NULL_VALUES"; + // ORDER BY makes explicit the row order that groupBy defaults to (declared groupBy field order) absent an orderBy argument. + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + categoryid, + pieceid + FROM stocks_price + GROUP BY categoryid, pieceid + ORDER BY categoryid, pieceid + ) AS table0"; // Execute the test for the SQL query - await TestSupportForGroupByNoAggregation(msSqlQuery); + await TestSupportForGroupByNoAggregation(postgresQuery); + } + + /// + /// Tests groupBy with aggregations on a mapped column. + /// + [TestMethod] + public async Task TestSupportForGroupByAggregationWithMappedColumns() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: ASC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + max_column1: max(field: column1) + count_column1: count(field: column1) + } + } + } + }"; + + string postgresQuery = @" + SELECT json_agg(to_jsonb(table0)) FROM ( + SELECT + __column1 AS column1, + MAX(__column1) AS max_column1, + COUNT(__column1) AS count_column1 + FROM GQLmappings + GROUP BY __column1 + ORDER BY __column1 + ) AS table0"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + JsonElement groupByArray = actual.GetProperty(QueryBuilder.GROUP_BY_FIELD_NAME); + + string expected = await GetDatabaseResultAsync(postgresQuery); + using JsonDocument expectedDocument = JsonDocument.Parse(expected); + SqlTestHelper.AssertNumericAggregations(groupByArray, expectedDocument.RootElement); + } + + /// + /// Tests aggregations over a mapped column without grouped fields. + /// + [TestMethod] + public async Task TestSupportForAggregationWithMappedColumnReturnsExpectedValues() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings { + groupBy { + aggregations { + max_column1: max(field: column1) + min_column1: min(field: column1) + sum_column1: sum(field: column1) + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { + ""aggregations"": { + ""max_column1"": 5, + ""min_column1"": 1, + ""sum_column1"": 13, + ""count_column1"": 4 + } + } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// Tests selecting only a mapped field from groupBy. + /// + [TestMethod] + public async Task TestSupportForGroupByFieldsOnlyWithMappedColumnReturnsExpectedValues() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: ASC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""fields"": { ""column1"": 1 } }, + { ""fields"": { ""column1"": 3 } }, + { ""fields"": { ""column1"": 4 } }, + { ""fields"": { ""column1"": 5 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// Tests grouped fields and aggregations over the same mapped column. + /// + [TestMethod] + public async Task TestSupportForGroupByFieldsAndAggregationWithMappedColumnReturnsExpectedValues() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: ASC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""fields"": { ""column1"": 1 }, ""aggregations"": { ""count_column1"": 1 } }, + { ""fields"": { ""column1"": 3 }, ""aggregations"": { ""count_column1"": 1 } }, + { ""fields"": { ""column1"": 4 }, ""aggregations"": { ""count_column1"": 1 } }, + { ""fields"": { ""column1"": 5 }, ""aggregations"": { ""count_column1"": 1 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// Tests orderBy on a mapped column in a groupBy query. + /// + [TestMethod] + public async Task TestSupportForGroupByWithOrderByOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: DESC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""fields"": { ""column1"": 5 } }, + { ""fields"": { ""column1"": 4 } }, + { ""fields"": { ""column1"": 3 } }, + { ""fields"": { ""column1"": 1 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// Tests a HAVING filter on an aggregation over a mapped column. + /// + [TestMethod] + public async Task TestSupportForGroupByHavingAggregationOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: ASC }) { + groupBy(fields: [column1]) { + aggregations { + max_column1: max(field: column1, having: { gt: 3 }) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { ""aggregations"": { ""max_column1"": 4 } }, + { ""aggregations"": { ""max_column1"": 5 } } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// Tests grouped fields, aggregations, and HAVING over a mapped column. + /// + [TestMethod] + public async Task TestSupportForGroupByFieldsAggregationsAndHavingOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: ASC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + max_column1: max(field: column1, having: { gt: 3 }) + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { + ""fields"": { ""column1"": 4 }, + ""aggregations"": { ""max_column1"": 4, ""count_column1"": 1 } + }, + { + ""fields"": { ""column1"": 5 }, + ""aggregations"": { ""max_column1"": 5, ""count_column1"": 1 } + } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); + } + + /// + /// Tests fields, aggregations, HAVING, and orderBy together over a mapped column. + /// + [TestMethod] + public async Task TestSupportForGroupByWithFieldsAggregationsHavingAndOrderByOnMappedColumn() + { + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: DESC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + max_column1: max(field: column1, having: { gt: 3 }) + min_column1: min(field: column1) + count_column1: count(field: column1) + } + } + } + }"; + + string expected = @" + { + ""groupBy"": [ + { + ""fields"": { ""column1"": 5 }, + ""aggregations"": { ""max_column1"": 5, ""min_column1"": 5, ""count_column1"": 1 } + }, + { + ""fields"": { ""column1"": 4 }, + ""aggregations"": { ""max_column1"": 4, ""min_column1"": 4, ""count_column1"": 1 } + } + ] + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + SqlTestHelper.PerformTestEqualJsonStrings(expected, actual.ToString()); } [TestMethod] - [Ignore] public override async Task TestNoAggregationOptionsForTableWithoutNumericFields() { await base.TestNoAggregationOptionsForTableWithoutNumericFields(); diff --git a/src/Service.Tests/dab-config.PostgreSql.json b/src/Service.Tests/dab-config.PostgreSql.json index b9bfae0ec3..136a02291e 100644 --- a/src/Service.Tests/dab-config.PostgreSql.json +++ b/src/Service.Tests/dab-config.PostgreSql.json @@ -2220,6 +2220,35 @@ } ] }, + "DateOnlyTable": { + "source": { + "object": "date_only_table", + "type": "table", + "key-fields": [ + "event_date" + ] + }, + "graphql": { + "enabled": true, + "type": { + "singular": "DateOnlyTable", + "plural": "DateOnlyTables" + } + }, + "rest": { + "enabled": true + }, + "permissions": [ + { + "role": "anonymous", + "actions": [ + { + "action": "*" + } + ] + } + ] + }, "GQLmappings": { "source": { "object": "gqlmappings", From fa2765d477b05b1617c2e1a124d224ac02d932ae Mon Sep 17 00:00:00 2001 From: Arpit Gupta <106474712+ar-guptaar@users.noreply.github.com> Date: Fri, 7 Aug 2026 01:40:42 +0530 Subject: [PATCH 6/6] =?UTF-8?q?Enhance=20test=20coverage=20for=20GraphQL?= =?UTF-8?q?=20queries=20by=20adding=20orderBy=20clause=20to=E2=80=A6=20(#3?= =?UTF-8?q?753)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why make this change? In a previously merged PR, there were comments about some gaps in testing scenarios, which is addressed as part of this change. ## What is this change? This pull request adds comprehensive regression tests for groupBy queries involving mapped (aliased) columns in both MS SQL and DWSQL environments, and ensures deterministic query results by explicitly specifying `orderBy` in all relevant test GraphQL queries. **Test Coverage Improvements:** * Added two new end-to-end regression tests to `DwSqlGraphQLQueryTests.cs`: * One test verifies that grouping and projecting only a mapped column works as expected, ensuring the backing column is used for grouping and the alias is used in the response. * Another test covers grouping, selecting, aggregating, applying HAVING filters, and ordering on a mapped column, confirming consistent use of the backing column in all SQL clauses while projecting the mapped names in the response. ## How was this tested? - [X] Integration Tests - [X] Unit Tests Co-authored-by: ARPIT GUPTA (cherry picked from commit 3cfb270c6cd869894803a9ea29f61ea696277794) --- .../DwSqlGraphQLQueryTests.cs | 337 +++++++++++------- .../MsSqlGraphQLQueryTests.cs | 8 +- 2 files changed, 221 insertions(+), 124 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs index 8cf55c247d..899ee40fa3 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/DwSqlGraphQLQueryTests.cs @@ -158,7 +158,7 @@ SELECT TOP 100 [table0].[title] AS [title], ([table1_subq].[data]) AS [websiteplacement] FROM [dbo].[books] AS [table0] - + OUTER APPLY ( SELECT STRING_AGG( '{' + @@ -177,7 +177,7 @@ FROM [dbo].[book_website_placements] AS [table1] ORDER BY [table1].[id] ASC ) AS [table1] ) AS [table1_subq]([data]) - + WHERE ( [table0].[title] IN ('Awesome book', 'Also Awesome book') AND EXISTS ( @@ -202,9 +202,9 @@ ORDER BY [table0].[id] DESC public async Task OneToOneJoinQuery() { string dwSqlQuery = @" - SELECT COALESCE('[' + STRING_AGG('{' + N'""id"":' + ISNULL(STRING_ESCAPE(CAST([id] AS NVARCHAR(MAX)), 'json'), - 'null') + ',' + N'""title"":' + ISNULL('""' + STRING_ESCAPE([title], 'json') + '""', 'null') + ',' + - N'""websiteplacement"":' + ISNULL([websiteplacement], 'null') + + SELECT COALESCE('[' + STRING_AGG('{' + N'""id"":' + ISNULL(STRING_ESCAPE(CAST([id] AS NVARCHAR(MAX)), 'json'), + 'null') + ',' + N'""title"":' + ISNULL('""' + STRING_ESCAPE([title], 'json') + '""', 'null') + ',' + + N'""websiteplacement"":' + ISNULL([websiteplacement], 'null') + '}', ', ') + ']', '[]') FROM ( SELECT TOP 100 [table0].[id] AS [id], @@ -212,7 +212,7 @@ SELECT COALESCE('[' + STRING_AGG('{' + N'""id"":' + ISNULL(STRING_ESCAPE(CAST([i ([table1_subq].[data]) AS [websiteplacement] FROM [dbo].[books] AS [table0] OUTER APPLY ( - SELECT STRING_AGG('{' + N'""price"":' + ISNULL(STRING_ESCAPE(CAST([price] AS NVARCHAR(MAX)), 'json'), + SELECT STRING_AGG('{' + N'""price"":' + ISNULL(STRING_ESCAPE(CAST([price] AS NVARCHAR(MAX)), 'json'), 'null') + '}', ', ') FROM ( SELECT TOP 1 [table1].[price] AS [price] @@ -241,76 +241,76 @@ public async override Task DeeplyNestedManyToOneJoinQuery() SELECT COALESCE( '[' + STRING_AGG( '{' + N'""title"":' + ISNULL('""' + STRING_ESCAPE([title], 'json') + '""', 'null') + ',' + - N'""publishers"":' + ISNULL('""' + STRING_ESCAPE([publishers], 'json') + '""', 'null') + '}', + N'""publishers"":' + ISNULL('""' + STRING_ESCAPE([publishers], 'json') + '""', 'null') + '}', ', ' - ) + ']', + ) + ']', '[]' ) FROM ( - SELECT TOP 5 - [table0].[title] AS [title], + SELECT TOP 5 + [table0].[title] AS [title], ([table1_subq].[data]) AS [publishers] FROM [dbo].[books] AS [table0] OUTER APPLY ( SELECT STRING_AGG( '{' + N'""name"":' + ISNULL('""' + STRING_ESCAPE([name], 'json') + '""', 'null') + ',' + - N'""books"":' + ISNULL('""' + STRING_ESCAPE([books], 'json') + '""', 'null') + '}', + N'""books"":' + ISNULL('""' + STRING_ESCAPE([books], 'json') + '""', 'null') + '}', ', ' ) FROM ( - SELECT TOP 1 - [table1].[name] AS [name], + SELECT TOP 1 + [table1].[name] AS [name], (COALESCE([table2_subq].[data], '[]')) AS [books] FROM [dbo].[publishers] AS [table1] OUTER APPLY ( SELECT COALESCE( '[' + STRING_AGG( '{' + N'""title"":' + ISNULL('""' + STRING_ESCAPE([title], 'json') + '""', 'null') + ',' + - N'""publishers"":' + ISNULL('""' + STRING_ESCAPE([publishers], 'json') + '""', 'null') + '}', + N'""publishers"":' + ISNULL('""' + STRING_ESCAPE([publishers], 'json') + '""', 'null') + '}', ', ' - ) + ']', + ) + ']', '[]' ) FROM ( - SELECT TOP 4 - [table2].[title] AS [title], + SELECT TOP 4 + [table2].[title] AS [title], ([table3_subq].[data]) AS [publishers] FROM [dbo].[books] AS [table2] OUTER APPLY ( SELECT STRING_AGG( '{' + N'""name"":' + ISNULL('""' + STRING_ESCAPE([name], 'json') + '""', 'null') + ',' + - N'""books"":' + ISNULL('""' + STRING_ESCAPE([books], 'json') + '""', 'null') + '}', + N'""books"":' + ISNULL('""' + STRING_ESCAPE([books], 'json') + '""', 'null') + '}', ', ' ) FROM ( - SELECT TOP 1 - [table3].[name] AS [name], + SELECT TOP 1 + [table3].[name] AS [name], (COALESCE([table4_subq].[data], '[]')) AS [books] FROM [dbo].[publishers] AS [table3] OUTER APPLY ( SELECT COALESCE( '[' + STRING_AGG( '{' + N'""title"":' + ISNULL('""' + STRING_ESCAPE([title], 'json') + '""', 'null') + ',' + - N'""publishers"":' + ISNULL('""' + STRING_ESCAPE([publishers], 'json') + '""', 'null') + '}', + N'""publishers"":' + ISNULL('""' + STRING_ESCAPE([publishers], 'json') + '""', 'null') + '}', ', ' - ) + ']', + ) + ']', '[]' ) FROM ( - SELECT TOP 3 - [table4].[title] AS [title], + SELECT TOP 3 + [table4].[title] AS [title], ([table5_subq].[data]) AS [publishers] FROM [dbo].[books] AS [table4] OUTER APPLY ( SELECT STRING_AGG( - '{' + N'""name"":' + ISNULL('""' + STRING_ESCAPE([name], 'json') + '""', 'null') + '}', + '{' + N'""name"":' + ISNULL('""' + STRING_ESCAPE([name], 'json') + '""', 'null') + '}', ', ' ) FROM ( - SELECT TOP 1 + SELECT TOP 1 [table5].[name] AS [name] FROM [dbo].[publishers] AS [table5] - WHERE [table4].[publisher_id] = [table5].[id] + WHERE [table4].[publisher_id] = [table5].[id] AND [table5].[id] = [table4].[publisher_id] ORDER BY [table5].[id] ASC ) AS [table5] @@ -319,7 +319,7 @@ ORDER BY [table5].[id] ASC ORDER BY [table4].[id] ASC ) AS [table4] ) AS [table4_subq]([data]) - WHERE [table2].[publisher_id] = [table3].[id] + WHERE [table2].[publisher_id] = [table3].[id] AND [table3].[id] = [table2].[publisher_id] ORDER BY [table3].[id] ASC ) AS [table3] @@ -328,7 +328,7 @@ ORDER BY [table3].[id] ASC ORDER BY [table2].[id] ASC ) AS [table2] ) AS [table2_subq]([data]) - WHERE [table0].[publisher_id] = [table1].[id] + WHERE [table0].[publisher_id] = [table1].[id] AND [table1].[id] = [table0].[publisher_id] ORDER BY [table1].[id] ASC ) AS [table1] @@ -384,52 +384,52 @@ ORDER BY [table0].[id] ASC public async Task OneToManyJoinQuery() { string dwSqlQuery = @" - SELECT + SELECT COALESCE( '[' + STRING_AGG( - '{' + + '{' + N'""id"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [id]), 'json'), 'null') + ',' + - N'""reviews"":' + ISNULL('""' + STRING_ESCAPE([reviews], 'json') + '""', 'null') + - '}', + N'""reviews"":' + ISNULL('""' + STRING_ESCAPE([reviews], 'json') + '""', 'null') + + '}', ', ' - ) + ']', + ) + ']', '[]' - ) - FROM + ) + FROM ( - SELECT TOP 2 - [table0].[id] AS [id], + SELECT TOP 2 + [table0].[id] AS [id], COALESCE([table1_subq].[data], '[]') AS [reviews] - FROM + FROM [dbo].[books] AS [table0] - OUTER APPLY + OUTER APPLY ( - SELECT + SELECT COALESCE( '[' + STRING_AGG( - '{' + - N'""content"":' + ISNULL('""' + STRING_ESCAPE([content], 'json') + '""', 'null') + - '}', + '{' + + N'""content"":' + ISNULL('""' + STRING_ESCAPE([content], 'json') + '""', 'null') + + '}', ', ' - ) + ']', + ) + ']', '[]' - ) - FROM + ) + FROM ( - SELECT TOP 100 + SELECT TOP 100 [table1].[content] AS [content] - FROM + FROM [dbo].[reviews] AS [table1] - WHERE + WHERE [table1].[book_id] = [table0].[id] - ORDER BY - [table1].[book_id] ASC, + ORDER BY + [table1].[book_id] ASC, [table1].[id] ASC ) AS [table1] ) AS [table1_subq]([data]) - WHERE + WHERE 1 = 1 - ORDER BY + ORDER BY [table0].[id] ASC ) AS [table0]"; @@ -457,7 +457,7 @@ [table0].[id] ASC } /// - /// Added more complicated cases when queries are deeply nested and compare the results. + /// Added more complicated cases when queries are deeply nested and compare the results. /// /// [TestMethod] @@ -471,8 +471,8 @@ SELECT COALESCE( ) + ']', '[]' ) FROM ( - SELECT TOP 5 - [table0].[title] AS [title], + SELECT TOP 5 + [table0].[title] AS [title], COALESCE([table1_subq].[data], '[]') AS [authors] FROM [dbo].[books] AS [table0] OUTER APPLY ( @@ -483,11 +483,11 @@ SELECT COALESCE( ) + ']', '[]' ) FROM ( - SELECT TOP 4 - [table1].[name] AS [name], + SELECT TOP 4 + [table1].[name] AS [name], COALESCE([table2_subq].[data], '[]') AS [books] FROM [dbo].[authors] AS [table1] - INNER JOIN [dbo].[book_author_link] AS [table11] + INNER JOIN [dbo].[book_author_link] AS [table11] ON [table11].[author_id] = [table1].[id] OUTER APPLY ( SELECT COALESCE( @@ -497,11 +497,11 @@ SELECT COALESCE( ) + ']', '[]' ) FROM ( - SELECT TOP 3 - [table2].[title] AS [title], + SELECT TOP 3 + [table2].[title] AS [title], COALESCE([table3_subq].[data], '[]') AS [authors] FROM [dbo].[books] AS [table2] - INNER JOIN [dbo].[book_author_link] AS [table8] + INNER JOIN [dbo].[book_author_link] AS [table8] ON [table8].[book_id] = [table2].[id] OUTER APPLY ( SELECT COALESCE( @@ -510,10 +510,10 @@ SELECT COALESCE( ) + ']', '[]' ) FROM ( - SELECT TOP 2 + SELECT TOP 2 [table3].[name] AS [name] FROM [dbo].[authors] AS [table3] - INNER JOIN [dbo].[book_author_link] AS [table5] + INNER JOIN [dbo].[book_author_link] AS [table5] ON [table5].[author_id] = [table3].[id] WHERE [table5].[book_id] = [table2].[id] ORDER BY [table3].[id] ASC @@ -576,7 +576,7 @@ public async Task OneToOneJoinQueryWithMappedFieldNamesInRelationship() string dwSqlQuery = @" SELECT COALESCE('['+STRING_AGG('{'+N'""fancyName"":' + ISNULL('""' + STRING_ESCAPE([fancyName],'json') + '""','null')+','+N'""fungus"":' + ISNULL([fungus],'null')+'}',', ')+']','[]') FROM ( - SELECT TOP 100 [table0].[species] AS [fancyName], + SELECT TOP 100 [table0].[species] AS [fancyName], (SELECT TOP 1 '{""habitat"":""' + STRING_ESCAPE([table1].[habitat], 'json') + '""}' FROM [dbo].[fungi] AS [table1] WHERE [table0].[species] = [table1].[habitat] AND [table1].[habitat] = [table0].[species] @@ -902,15 +902,15 @@ SELECT COALESCE( N'""avg_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [avg_price]), 'json'), 'null') + ',' + N'""sum_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [sum_price]), 'json'), 'null') + '}', ', ' ) + ']', '[]' -) +) FROM ( - SELECT TOP 100 - max([table0].[categoryid]) AS [max], - max([table0].[price]) AS [max_price], - min([table0].[price]) AS [min_price], - avg([table0].[price]) AS [avg_price], - sum([table0].[price]) AS [sum_price] - FROM [dbo].[stocks_price] AS [table0] + SELECT TOP 100 + max([table0].[categoryid]) AS [max], + max([table0].[price]) AS [max_price], + min([table0].[price]) AS [min_price], + avg([table0].[price]) AS [avg_price], + sum([table0].[price]) AS [sum_price] + FROM [dbo].[stocks_price] AS [table0] WHERE 1 = 1 ) AS [table0];"; @@ -935,17 +935,17 @@ SELECT COALESCE( N'""sum_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [sum_price]), 'json'), 'null') + ',' + N'""count"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [count]), 'json'), 'null') + '}', ', ' ) + ']', '[]' -) +) FROM ( SELECT TOP 100 - max([table0].[categoryid]) AS [max], - max([table0].[price]) AS [max_price], - min([table0].[price]) AS [min_price], - avg([table0].[price]) AS [avg_price], - sum([table0].[price]) AS [sum_price], - count([table0].[categoryid]) AS [count] - FROM [dbo].[stocks_price] AS [table0] - WHERE 1 = 1 + max([table0].[categoryid]) AS [max], + max([table0].[price]) AS [max_price], + min([table0].[price]) AS [min_price], + avg([table0].[price]) AS [avg_price], + sum([table0].[price]) AS [sum_price], + count([table0].[categoryid]) AS [count] + FROM [dbo].[stocks_price] AS [table0] + WHERE 1 = 1 GROUP BY [table0].[categoryid] ) AS [table0];"; @@ -965,10 +965,10 @@ SELECT COALESCE( '[' + STRING_AGG( '{' + N'""min_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [min_price]), 'json'), 'null') + '}', ', ' ) + ']', '[]' -) +) FROM ( - SELECT TOP 100 min([table0].[price]) AS [min_price] - FROM [dbo].[stocks_price] AS [table0] + SELECT TOP 100 min([table0].[price]) AS [min_price] + FROM [dbo].[stocks_price] AS [table0] WHERE 1 = 1 ) AS [table0];"; @@ -983,10 +983,10 @@ FROM [dbo].[stocks_price] AS [table0] public async Task TestSupportForMaxAggregation() { string msSqlQuery = @" -SELECT COALESCE('['+STRING_AGG('{'+N'""max_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [max_price]),'json'),'null')+'}',', ')+']','[]') +SELECT COALESCE('['+STRING_AGG('{'+N'""max_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [max_price]),'json'),'null')+'}',', ')+']','[]') FROM ( - SELECT TOP 100 max([table0].[price]) AS [max_price] - FROM [dbo].[stocks_price] AS [table0] + SELECT TOP 100 max([table0].[price]) AS [max_price] + FROM [dbo].[stocks_price] AS [table0] WHERE 1 = 1 ) AS [table0];"; @@ -1002,10 +1002,10 @@ FROM [dbo].[stocks_price] AS [table0] public async Task TestSupportForAvgAggregation() { string msSqlQuery = @" -SELECT COALESCE('['+STRING_AGG('{'+N'""avg_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [avg_price]),'json'),'null')+'}',', ')+']','[]') +SELECT COALESCE('['+STRING_AGG('{'+N'""avg_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [avg_price]),'json'),'null')+'}',', ')+']','[]') FROM ( - SELECT TOP 100 avg([table0].[price]) AS [avg_price] - FROM [dbo].[stocks_price] AS [table0] + SELECT TOP 100 avg([table0].[price]) AS [avg_price] + FROM [dbo].[stocks_price] AS [table0] WHERE 1 = 1 ) AS [table0];"; @@ -1021,10 +1021,10 @@ FROM [dbo].[stocks_price] AS [table0] public async Task TestSupportForSumAggregation() { string msSqlQuery = @" -SELECT COALESCE('['+STRING_AGG('{'+N'""sum_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [sum_price]),'json'),'null')+'}',', ')+']','[]') +SELECT COALESCE('['+STRING_AGG('{'+N'""sum_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [sum_price]),'json'),'null')+'}',', ')+']','[]') FROM ( - SELECT TOP 100 sum([table0].[price]) AS [sum_price] - FROM [dbo].[stocks_price] AS [table0] + SELECT TOP 100 sum([table0].[price]) AS [sum_price] + FROM [dbo].[stocks_price] AS [table0] WHERE 1 = 1 ) AS [table0];"; @@ -1059,11 +1059,11 @@ FROM [dbo].[stocks_price] AS [table0] public async Task TestSupportForHavingAggregation() { string msSqlQuery = @" -SELECT COALESCE('[' + STRING_AGG('{' + N'""max"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [max]), 'json'), 'null') + '}', ', ') + ']', '[]') +SELECT COALESCE('[' + STRING_AGG('{' + N'""max"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [max]), 'json'), 'null') + '}', ', ') + ']', '[]') FROM ( - SELECT TOP 100 max([table0].[id]) AS [max] - FROM [dbo].[publishers] AS [table0] - WHERE 1 = 1 + SELECT TOP 100 max([table0].[id]) AS [max] + FROM [dbo].[publishers] AS [table0] + WHERE 1 = 1 HAVING max([table0].[id]) > 2346 ) AS [table0];"; @@ -1083,13 +1083,13 @@ SELECT COALESCE( '[' + STRING_AGG( '{' + N'""sum_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [sum_price]), 'json'), 'null') + '}', ', ' ) + ']', '[]' -) +) FROM ( - SELECT TOP 100 - SUM([table0].[price]) AS [sum_price] - FROM [dbo].[stocks_price] AS [table0] - WHERE 1 = 1 - GROUP BY [table0].[categoryid], [table0].[pieceid] + SELECT TOP 100 + SUM([table0].[price]) AS [sum_price] + FROM [dbo].[stocks_price] AS [table0] + WHERE 1 = 1 + GROUP BY [table0].[categoryid], [table0].[pieceid] HAVING SUM([table0].[price]) > 50 ) AS [table0];"; @@ -1112,16 +1112,16 @@ SELECT COALESCE( N'""sum_price"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [sum_price]), 'json'), 'null') + ',' + N'""count_piece"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [count_piece]), 'json'), 'null') + '}', ', ' ) + ']', '[]' -) +) FROM ( - SELECT TOP 100 - [table0].[categoryid] AS [categoryid], - [table0].[pieceid] AS [pieceid], - SUM([table0].[price]) AS [sum_price], - COUNT([table0].[pieceid]) AS [count_piece] - FROM [dbo].[stocks_price] AS [table0] - WHERE 1 = 1 - GROUP BY [table0].[categoryid], [table0].[pieceid] + SELECT TOP 100 + [table0].[categoryid] AS [categoryid], + [table0].[pieceid] AS [pieceid], + SUM([table0].[price]) AS [sum_price], + COUNT([table0].[pieceid]) AS [count_piece] + FROM [dbo].[stocks_price] AS [table0] + WHERE 1 = 1 + GROUP BY [table0].[categoryid], [table0].[pieceid] HAVING SUM([table0].[price]) > 50 AND COUNT([table0].[pieceid]) <= 100 ) AS [table0];"; @@ -1142,13 +1142,13 @@ SELECT COALESCE( '{' + N'""categoryid"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [categoryid]), 'json'), 'null') + ',' + N'""pieceid"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [pieceid]), 'json'), 'null') + '}', ', ' ) + ']', '[]' -) +) FROM ( - SELECT TOP 100 - [table0].[categoryid] AS [categoryid], - [table0].[pieceid] AS [pieceid] - FROM [dbo].[stocks_price] AS [table0] - WHERE 1 = 1 + SELECT TOP 100 + [table0].[categoryid] AS [categoryid], + [table0].[pieceid] AS [pieceid] + FROM [dbo].[stocks_price] AS [table0] + WHERE 1 = 1 GROUP BY [table0].[categoryid], [table0].[pieceid] ) AS [table0]"; @@ -1156,6 +1156,103 @@ FROM [dbo].[stocks_price] AS [table0] await TestSupportForGroupByNoAggregation(msSqlQuery); } + /// + /// Regression test for groupBy that selects only a mapped (aliased) field on DWSQL. + /// The 'GQLmappings' entity exposes backing column '__column1' as 'column1'. This verifies the + /// GROUP BY references the backing column while the projected 'fields' object uses the exposed + /// alias. An explicit orderBy makes the returned row order deterministic. Seeded values {1, 3, 4, 5}. + /// + [TestMethod] + public async Task TestSupportForGroupByFieldsOnlyWithMappedColumnReturnsExpectedValues() + { + string dwSqlQuery = @" +SELECT COALESCE( + '[' + STRING_AGG( + '{' + N'""column1"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [column1]), 'json'), 'null') + '}', ', ' + ) WITHIN GROUP (ORDER BY [column1] ASC) + ']', '[]' +) +FROM ( + SELECT + [table0].[__column1] AS [column1] + FROM [dbo].[GQLmappings] AS [table0] + WHERE 1 = 1 + GROUP BY [table0].[__column1] +) AS [table0]"; + + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: ASC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + } + } + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + string expected = await GetDatabaseResultAsync(dwSqlQuery); + + SqlTestHelper.PerformTestEqualJsonStringsForAggreagtionQueries(expected, actual.ToString()); + } + + /// + /// End-to-end regression test on DWSQL exercising every groupBy dimension together on a mapped + /// (aliased) column: grouping by the mapped field, selecting the mapped field, multiple + /// aggregations on the mapped field, a field-level HAVING filter on one aggregation, and an + /// orderBy on the mapped field. The 'GQLmappings' entity exposes backing '__column1' as 'column1' + /// (values {1, 3, 4, 5}), each forming its own group. HAVING max(column1) > 3 keeps groups 4 + /// and 5, and orderBy DESC returns them as 5 then 4. Proves the backing column is used + /// consistently in GROUP BY, HAVING and ORDER BY while the response projects the mapped names. + /// + [TestMethod] + public async Task TestSupportForGroupByWithFieldsAggregationsHavingAndOrderByOnMappedColumn() + { + string dwSqlQuery = @" +SELECT COALESCE( + '[' + STRING_AGG( + '{' + + N'""column1"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [column1]), 'json'), 'null') + ', ' + + N'""max_column1"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [max_column1]), 'json'), 'null') + ', ' + + N'""min_column1"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [min_column1]), 'json'), 'null') + ', ' + + N'""count_column1"":' + ISNULL(STRING_ESCAPE(CONVERT(NVARCHAR(MAX), [count_column1]), 'json'), 'null') + '}', ', ' + ) WITHIN GROUP (ORDER BY [column1] DESC) + ']', '[]' +) +FROM ( + SELECT + [table0].[__column1] AS [column1], + MAX([table0].[__column1]) AS [max_column1], + MIN([table0].[__column1]) AS [min_column1], + COUNT([table0].[__column1]) AS [count_column1] + FROM [dbo].[GQLmappings] AS [table0] + GROUP BY [table0].[__column1] + HAVING MAX([table0].[__column1]) > 3 +) AS [table0]"; + + string graphQLQueryName = "gQLmappings"; + string graphQLQuery = @" + { + gQLmappings(orderBy: { column1: DESC }) { + groupBy(fields: [column1]) { + fields { + column1 + } + aggregations { + max_column1: max(field: column1, having: { gt: 3 }) + min_column1: min(field: column1) + count_column1: count(field: column1) + } + } + } + }"; + + JsonElement actual = await ExecuteGraphQLRequestAsync(graphQLQuery, graphQLQueryName, isAuthenticated: false); + string expected = await GetDatabaseResultAsync(dwSqlQuery); + + SqlTestHelper.PerformTestEqualJsonStringsForAggreagtionQueries(expected, actual.ToString()); + } + /// /// Test to check that an exception is thrown when both items and groupBy are present in the same query. /// diff --git a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs index c6bb28fcff..249c84c5d8 100644 --- a/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs +++ b/src/Service.Tests/SqlTests/GraphQLQueryTests/MsSqlGraphQLQueryTests.cs @@ -1061,7 +1061,7 @@ public async Task TestSupportForGroupByFieldsOnlyWithMappedColumnReturnsExpected string graphQLQueryName = "gQLmappings"; string graphQLQuery = @" { - gQLmappings { + gQLmappings(orderBy: { column1: ASC }) { groupBy(fields: [column1]) { fields { column1 @@ -1097,7 +1097,7 @@ public async Task TestSupportForGroupByFieldsAndAggregationWithMappedColumnRetur string graphQLQueryName = "gQLmappings"; string graphQLQuery = @" { - gQLmappings { + gQLmappings(orderBy: { column1: ASC }) { groupBy(fields: [column1]) { fields { column1 @@ -1173,7 +1173,7 @@ public async Task TestSupportForGroupByHavingAggregationOnMappedColumn() string graphQLQueryName = "gQLmappings"; string graphQLQuery = @" { - gQLmappings { + gQLmappings(orderBy: { column1: ASC }) { groupBy(fields: [column1]) { aggregations { max_column1: max(field: column1, having: { gt: 3 }) @@ -1207,7 +1207,7 @@ public async Task TestSupportForGroupByFieldsAggregationsAndHavingOnMappedColumn string graphQLQueryName = "gQLmappings"; string graphQLQuery = @" { - gQLmappings { + gQLmappings(orderBy: { column1: ASC }) { groupBy(fields: [column1]) { fields { column1