From 98fdeda54e5d49c0dc2144718a80960ce978b3d6 Mon Sep 17 00:00:00 2001 From: Daniel Harvey Date: Fri, 16 Jun 2023 09:47:15 +0100 Subject: [PATCH] fix(server): skip insert when no values provided for SQLServer PR-URL: https://github.com/hasura/graphql-engine-mono/pull/9564 GitOrigin-RevId: 7d95ee428b8da7e246d9f1040f04d8abe359b667 --- scripts/make/tests.mk | 8 ++--- .../SQLServer/DefaultValues/OnConflictSpec.hs | 33 +++++++++++++++++++ .../src/Test/Mutations/Insert/MultipleSpec.hs | 8 +++-- .../Hasura/Backends/MSSQL/Execute/Insert.hs | 25 ++++++++------ 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/scripts/make/tests.mk b/scripts/make/tests.mk index 472818755c5..0e9619b19c2 100644 --- a/scripts/make/tests.mk +++ b/scripts/make/tests.mk @@ -16,7 +16,7 @@ test-bigquery: build remove-tix-file start-api-test-postgres .PHONY: test-sqlserver ## test-sqlserver: run tests for MS SQL Server backend -test-sqlserver: build remove-tix-file start-api-test-backends +test-sqlserver: build remove-tix-file start-api-tests-backends HASURA_TEST_BACKEND_TYPE=SQLServer \ GRAPHQL_ENGINE=$(GRAPHQL_ENGINE_PATH) \ POSTGRES_AGENT=$(POSTGRES_AGENT_PATH) \ @@ -24,7 +24,7 @@ test-sqlserver: build remove-tix-file start-api-test-backends .PHONY: test-citus ## test-citus: run tests for Citus backend -test-citus: build remove-tix-file start-api-test-backends +test-citus: build remove-tix-file start-api-tests-backends HASURA_TEST_BACKEND_TYPE=Citus \ GRAPHQL_ENGINE=$(GRAPHQL_ENGINE_PATH) \ POSTGRES_AGENT=$(POSTGRES_AGENT_PATH) \ @@ -32,7 +32,7 @@ test-citus: build remove-tix-file start-api-test-backends .PHONY: test-data-connectors ## test-data-connectors: run tests for Data Connectors -test-data-connectors: build remove-tix-file start-api-test-backends +test-data-connectors: build remove-tix-file start-api-tests-backends HASURA_TEST_BACKEND_TYPE=DataConnector \ GRAPHQL_ENGINE=$(GRAPHQL_ENGINE_PATH) \ POSTGRES_AGENT=$(POSTGRES_AGENT_PATH) \ @@ -40,7 +40,7 @@ test-data-connectors: build remove-tix-file start-api-test-backends .PHONY: test-cockroach ## test-cockroach: run tests for Cockroach backend -test-cockroach: build remove-tix-file start-api-test-backends +test-cockroach: build remove-tix-file start-api-tests-backends HASURA_TEST_BACKEND_TYPE=Cockroach \ GRAPHQL_ENGINE=$(GRAPHQL_ENGINE_PATH) \ POSTGRES_AGENT=$(POSTGRES_AGENT_PATH) \ diff --git a/server/lib/api-tests/src/Test/Databases/SQLServer/DefaultValues/OnConflictSpec.hs b/server/lib/api-tests/src/Test/Databases/SQLServer/DefaultValues/OnConflictSpec.hs index 26cfcf4aff3..579ab79f500 100644 --- a/server/lib/api-tests/src/Test/Databases/SQLServer/DefaultValues/OnConflictSpec.hs +++ b/server/lib/api-tests/src/Test/Databases/SQLServer/DefaultValues/OnConflictSpec.hs @@ -150,3 +150,36 @@ tests = do |] shouldReturnYaml testEnvironment actual expected + + it "Upsert with no objects does not break" \testEnvironment -> do + let expected :: Value + expected = + [yaml| + data: + insert_hasura_somedefaults: + affected_rows: 0 + returning: [] + |] + + actual :: IO Value + actual = + postGraphql + testEnvironment + [graphql| + mutation { + insert_hasura_somedefaults( + objects: [] + if_matched: { + match_columns: name, + update_columns: [] + } + ) { + affected_rows + returning { + id + } + } + } + |] + + shouldReturnYaml testEnvironment actual expected diff --git a/server/lib/api-tests/src/Test/Mutations/Insert/MultipleSpec.hs b/server/lib/api-tests/src/Test/Mutations/Insert/MultipleSpec.hs index 5b129ac6a9e..0973086e81a 100644 --- a/server/lib/api-tests/src/Test/Mutations/Insert/MultipleSpec.hs +++ b/server/lib/api-tests/src/Test/Mutations/Insert/MultipleSpec.hs @@ -12,6 +12,7 @@ import Data.List.NonEmpty qualified as NE import Harness.Backend.Citus qualified as Citus import Harness.Backend.Cockroach qualified as Cockroach import Harness.Backend.Postgres qualified as Postgres +import Harness.Backend.Sqlserver qualified as Sqlserver import Harness.GraphqlEngine (postGraphql) import Harness.Quoter.Graphql (graphql) import Harness.Quoter.Yaml (interpolateYaml) @@ -41,6 +42,11 @@ spec = do { Fixture.setupTeardown = \(testEnv, _) -> [ Cockroach.setupTablesAction schema testEnv ] + }, + (Fixture.fixture $ Fixture.Backend Sqlserver.backendTypeMetadata) + { Fixture.setupTeardown = \(testEnv, _) -> + [ Sqlserver.setupTablesAction schema testEnv + ] } ] ) @@ -104,13 +110,11 @@ tests = do insert_#{schemaName}_article( objects: [ { - id: 1, title: "Article 1", content: "Sample article content", author_id: 1 }, { - id: 2, title: "Article 2", content: "Sample article content", author_id: 2 diff --git a/server/src-lib/Hasura/Backends/MSSQL/Execute/Insert.hs b/server/src-lib/Hasura/Backends/MSSQL/Execute/Insert.hs index 97af4620a2a..752fedd0051 100644 --- a/server/src-lib/Hasura/Backends/MSSQL/Execute/Insert.hs +++ b/server/src-lib/Hasura/Backends/MSSQL/Execute/Insert.hs @@ -162,16 +162,20 @@ buildInsertTx tableName withAlias stringifyNum insert queryTags = do Tx.unitQueryE defaultMSSQLTxErrorHandler (createInsertedTempTableQuery `withQueryTags` queryTags) - -- Choose between running a regular @INSERT INTO@ statement or a @MERGE@ statement - -- depending on the @if_matched@ field. - -- - -- Affected rows will be inserted into the #inserted temporary table regardless. - case ifMatchedField of - Nothing -> do - -- Insert values into the table using INSERT query - let insertQuery = toQueryFlat $ TQ.fromInsert $ TSQL.fromInsert insert - Tx.unitQueryE mutationMSSQLTxErrorHandler (insertQuery `withQueryTags` queryTags) - Just ifMatched -> buildUpsertTx tableName insert ifMatched queryTags + -- check we have any values to insert, SQLServer doesn't appear to have a + -- nice syntax for "insert no rows please" + unless (null $ _aiInsertObject $ _aiData insert) + $ + -- Choose between running a regular @INSERT INTO@ statement or a @MERGE@ statement + -- depending on the @if_matched@ field. + -- + -- Affected rows will be inserted into the #inserted temporary table regardless. + case ifMatchedField of + Nothing -> do + -- Insert values into the table using INSERT query + let insertQuery = toQueryFlat $ TQ.fromInsert $ TSQL.fromInsert insert + Tx.unitQueryE mutationMSSQLTxErrorHandler (insertQuery `withQueryTags` queryTags) + Just ifMatched -> buildUpsertTx tableName insert ifMatched queryTags -- Build a response to the user using the values in the temporary table named #inserted (responseText, checkConditionInt) <- buildInsertResponseTx stringifyNum withAlias insert queryTags @@ -225,6 +229,7 @@ buildUpsertTx tableName insert ifMatched queryTags = do toQueryFlat $ TQ.fromInsertValuesIntoTempTable $ TSQL.toInsertValuesIntoTempTable tempTableNameValues insert + Tx.unitQueryE mutationMSSQLTxErrorHandler (insertValuesIntoTempTableQuery `withQueryTags` queryTags) -- Run the MERGE query and store the mutated rows in #inserted temporary table