Skip to content

Commit 4fbe9f8

Browse files
authored
Mutations on table with triggers - MsSql (#1630)
## Why make this change? When there is a DML trigger configured on a table, we cannot use OUTPUT clause to return data. When an insert DML trigger is configured, OUTPUT clause cannot be used to return data because of the insert operation. When an update DML trigger is configured, OUTPUT clause cannot be used to return data because of the update operation. In such cases, we need to use a subsequent select statement to get the data after the mutation. ## Details **What would handling of different kind of mutations look like when a trigger is enabled on a table?** 1. **DELETE:** We don't need to return any data for deletions, so nothing will change for deletions. 2. **UPDATE:** For updates, we will cease to use the output clause. Instead, we will do a subsequent select query on the table to get the data. Since PK would already be provided to us we can easily do a select query. Not to mention, we can do this only when we determine that there is indeed a trigger enabled on the table. If not, we can continue with the same query that we generate in present day. 3. **INSERT:** When it comes to insertions, we can perform insertions on two kind of tables: 1. **Tables with non-auto generated primary keys:** When we have tables with non-autogen PK, we can insert values of non-autogen PK fields into a temporary table (we might not know value of every PK before hand because PK can have a default value as well) and then perform an inner join for the subsequent select on the actual table and the temporary table based on the values of these PK fields. 2. **Tables with auto-gen primary keys:** Tables with an autogenerated PK column fall into this category.. For tables with autogen PK, we would not be aware of the PK beforehand and so we cannot directly perform the subsequent select query. To get the autogen primary key, we can use `SCOPE_IDENTITY()` method provided by Sql Server.. Once we get the autogen PK, in addition to having an inner join for non-autogen PKs as discussed in the previous approach, we can add an additional `WHERE` predicate `where autogen_PK_field = SCOPE_IDENTITY()`. Let us discuss more about this approach and the concerns: **Delving deeper into [Approach#3](#452 (comment) We can use `OUTPUT` clause coupled with `INTO` clause to store the values for the primary keys in a temporary table. And then do a subsequent select query on the table to get the data using those primary keys. In the temporary table, we insert only those columns from the PK which are not autogenerated. Why? Because we cannot insert a value for an autogenerated column even in a temporary table. Hence in the select query, we will add an additional WHERE predicate (alongwith the join) for the autogenerated column in the PK. It is to be noted that **MsSql supports only one IDENTITY/autogenerated column per table**. We can choose to go with this approach only if we determine that there are any triggers enabled on the table. Two assumptions which I am making here that I want to put ahead (btw, I don't think those are valid use cases) are: 1. **What if primary key column has a datatype of timestamp?** This concern arise because we cannot insert value for `timestamp` column (which is what the approach includes - inserting value via `INTO` clause). Well, `rowversion`/`timestamp` is intended for versioning and tracking changes within a table, rather than serving as a primary key. Also, by design, primary key values should not be modified after they have been assigned to a record. So having a PK with timestamp datatype does not seem to be a valid use case to me. 2. **What if the trigger updates the values of the primary keys** and the subsequent select query we are going to perform returns no data because the values of the PK has been changed in the table? This concern arises because technically, it is possible for triggers to update the value of PK columns. However, this is generally not recommended and can lead to undesirable consequences. Changing the value of a primary key can result in data integrity issues, as it may cause conflicts or break referential integrity constraints if other tables are referencing the primary key. It can also lead to confusion and difficulty in identifying records uniquely. ## What is this change? 1. Added properties `IsUpdateDMLTriggerEnabled` and `IsInsertDMLTriggerEnabled` to `SourceDefinition` class which if true will indicate that there is a corresponding DML trigger enabled on the table. 2. Added method `SqlMetadataProvider.PopulateTriggerMetadataForTable` whose overridden implementation in `MsSqlMetadataProvider` will populate the properties mentioned in (1) with appropriate metadata values. 3. Refactored how queries are built in the `SqlInsertStructure`, `SqlUpsertQueryStructure`, `SqlUpdateStructure` so that when trigger is configured for the mutation operation, a subsequent select query is used instead of the `OUTPUT` clause to return data. 4. In case of insertions in table with auto-gen PK, we get the latest value of the auto-gen column using `SCOPE_IDENTITY() ` function provided by sql server. Reference: https://learn.microsoft.com/en-us/sql/t-sql/functions/scope-identity-transact-sql?view=sql-server-ver16 ## How was this tested? - [x] Integration Tests - Added mutations tests to Rest/GQL performing inserts/updates/upserts on tables with insert/update DML trigger enabled. Tests are added for tables with autogen/non-autogen PK as the queries differ in the two cases.
1 parent a5f9db0 commit 4fbe9f8

File tree

15 files changed

+963
-25
lines changed

15 files changed

+963
-25
lines changed

config-generators/mssql-commands.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ add InsertAndDisplayAllBooksUnderGivenPublisher --config "dab-config.MsSql.json"
3939
add GQLmappings --config "dab-config.MsSql.json" --source "GQLmappings" --permissions "anonymous:*" --rest true --graphql true
4040
add Bookmarks --config "dab-config.MsSql.json" --source "bookmarks" --permissions "anonymous:*" --rest true --graphql true
4141
add MappedBookmarks --config "dab-config.MsSql.json" --source "mappedbookmarks" --permissions "anonymous:*" --rest true --graphql true
42+
add FteData --config "dab-config.MsSql.json" --source fte_data --permissions "anonymous:*" --graphql "FteData:FteData"
43+
add InternData --config "dab-config.MsSql.json" --source intern_data --permissions "anonymous:*" --graphql "InternData:InternData"
4244
add BooksSold --config "dab-config.MsSql.json" --source "books_sold" --rest true --graphql "books_sold:books_sold" --permissions "anonymous:*"
4345
update GQLmappings --config "dab-config.MsSql.json" --map "__column1:column1,__column2:column2" --permissions "authenticated:*"
4446
update Publisher --config "dab-config.MsSql.json" --permissions "authenticated:create,read,update,delete" --rest true --graphql true --relationship books --target.entity Book --cardinality many

src/Config/DatabasePrimitives/DatabaseObject.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,21 @@ public class SourceDefinition
154154
public Dictionary<string, RelationshipMetadata> SourceEntityRelationshipMap { get; private set; } =
155155
new(StringComparer.InvariantCultureIgnoreCase);
156156

157+
/// <summary>
158+
/// Indicates whether an update trigger enabled on the table.
159+
/// The default value must be kept as false, meaning by default we assume no trigger is enabled.
160+
/// Based on whether trigger is enabled, we use either OUTPUT (when no trigger is enabled) / or SELECT (when a trigger is enabled),
161+
/// to return the data after a mutation operation.
162+
/// </summary>
163+
public bool IsUpdateDMLTriggerEnabled;
164+
165+
/// <summary>
166+
/// Indicates whether an insert trigger enabled on the table.
167+
/// The default value must be kept as false, meaning by default we assume no trigger is enabled.
168+
/// Based on whether trigger is enabled, we use either OUTPUT (when no trigger is enabled) / or SELECT (when a trigger is enabled),
169+
/// to return the data after a mutation operation.
170+
public bool IsInsertDMLTriggerEnabled;
171+
157172
/// <summary>
158173
/// Given the list of column names to check, evaluates
159174
/// if any of them is a nullable column when matched with the columns in this source definition.

src/Core/Resolvers/IQueryBuilder.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ public interface IQueryBuilder
7373
/// an overridden implementation of the method.</exception>
7474
string BuildQueryToGetReadOnlyColumns(string schemaOrDatabaseParamName, string tableParamName);
7575

76+
/// <summary>
77+
/// Builds the query to determine the number of enabled triggers on a database table.
78+
/// Needed only for MsSql.
79+
/// </summary>
80+
string BuildFetchEnabledTriggersQuery() => throw new NotImplementedException();
81+
7682
/// <summary>
7783
/// Adds database specific quotes to string identifier
7884
/// </summary>

src/Core/Resolvers/MsSqlQueryBuilder.cs

Lines changed: 256 additions & 23 deletions
Large diffs are not rendered by default.

src/Core/Services/MetadataProviders/MsSqlMetadataProvider.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System.Data;
5+
using System.Text.Json;
6+
using System.Text.Json.Nodes;
7+
using Azure.DataApiBuilder.Config.DatabasePrimitives;
48
using Azure.DataApiBuilder.Core.Configurations;
9+
using Azure.DataApiBuilder.Core.Models;
510
using Azure.DataApiBuilder.Core.Resolvers;
611
using Microsoft.Data.SqlClient;
712
using Microsoft.Extensions.Logging;
@@ -40,5 +45,38 @@ public override Type SqlToCLRType(string sqlType)
4045
{
4146
return TypeHelper.GetSystemTypeFromSqlDbType(sqlType);
4247
}
48+
49+
/// <inheritdoc/>
50+
public override async Task PopulateTriggerMetadataForTable(string entityName, string schemaName, string tableName, SourceDefinition sourceDefinition)
51+
{
52+
string enumerateEnabledTriggers = SqlQueryBuilder.BuildFetchEnabledTriggersQuery();
53+
Dictionary<string, DbConnectionParam> parameters = new()
54+
{
55+
{ $"{BaseQueryStructure.PARAM_NAME_PREFIX}param0", new(schemaName, DbType.String) },
56+
{ $"{BaseQueryStructure.PARAM_NAME_PREFIX}param1", new(tableName, DbType.String) }
57+
};
58+
59+
JsonArray? resultArray = await QueryExecutor.ExecuteQueryAsync(
60+
sqltext: enumerateEnabledTriggers,
61+
parameters: parameters,
62+
dataReaderHandler: QueryExecutor.GetJsonArrayAsync);
63+
using JsonDocument sqlResult = JsonDocument.Parse(resultArray!.ToJsonString());
64+
65+
foreach (JsonElement element in sqlResult.RootElement.EnumerateArray())
66+
{
67+
string type_desc = element.GetProperty("type_desc").ToString();
68+
if ("UPDATE".Equals(type_desc))
69+
{
70+
sourceDefinition.IsUpdateDMLTriggerEnabled = true;
71+
_logger.LogInformation($"An update trigger is enabled for the entity: {entityName}");
72+
}
73+
74+
if ("INSERT".Equals(type_desc))
75+
{
76+
sourceDefinition.IsInsertDMLTriggerEnabled = true;
77+
_logger.LogInformation($"An insert trigger is enabled for the entity: {entityName}");
78+
}
79+
}
80+
}
4381
}
4482
}

src/Core/Services/MetadataProviders/SqlMetadataProvider.cs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public abstract class SqlMetadataProvider<ConnectionT, DataAdapterT, CommandT> :
6969
public Dictionary<string, DatabaseObject> EntityToDatabaseObject { get; set; } =
7070
new(StringComparer.InvariantCulture);
7171

72-
private readonly ILogger<ISqlMetadataProvider> _logger;
72+
protected readonly ILogger<ISqlMetadataProvider> _logger;
7373

7474
public SqlMetadataProvider(
7575
RuntimeConfigProvider runtimeConfigProvider,
@@ -361,6 +361,19 @@ private async Task FillSchemaForStoredProcedureAsync(
361361
/// </summary>
362362
public abstract Type SqlToCLRType(string sqlType);
363363

364+
/// <summary>
365+
/// Updates a table's SourceDefinition object's metadata with whether any enabled insert/update DML triggers exist for the table.
366+
/// This method is only called for tables in MsSql.
367+
/// </summary>
368+
/// <param name="entityName">Name of the entity.</param>
369+
/// <param name="schemaName">Name of the schema in which the table is present.</param>
370+
/// <param name="tableName">Name of the table.</param>
371+
/// <param name="sourceDefinition">Table definition to update.</param>
372+
public virtual Task PopulateTriggerMetadataForTable(string entityName, string schemaName, string tableName, SourceDefinition sourceDefinition)
373+
{
374+
throw new NotImplementedException();
375+
}
376+
364377
/// <summary>
365378
/// Generates the map used to find a given entity based
366379
/// on the path that will be used for that entity.
@@ -975,10 +988,15 @@ private async Task PopulateSourceDefinitionAsync(
975988
subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization);
976989
}
977990

991+
_entities.TryGetValue(entityName, out Entity? entity);
992+
if (GetDatabaseType() is DatabaseType.MSSQL && entity is not null && entity.Source.Type is EntitySourceType.Table)
993+
{
994+
await PopulateTriggerMetadataForTable(entityName, schemaName, tableName, sourceDefinition);
995+
}
996+
978997
using DataTableReader reader = new(dataTable);
979998
DataTable schemaTable = reader.GetSchemaTable();
980999
RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig();
981-
_entities.TryGetValue(entityName, out Entity? entity);
9821000
foreach (DataRow columnInfoFromAdapter in schemaTable.Rows)
9831001
{
9841002
string columnName = columnInfoFromAdapter["ColumnName"].ToString()!;

src/Service.Tests/DatabaseSchema-MsSql.sql

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ DROP TABLE IF EXISTS graphql_incompatible;
4747
DROP TABLE IF EXISTS GQLmappings;
4848
DROP TABLE IF EXISTS bookmarks;
4949
DROP TABLE IF EXISTS mappedbookmarks;
50+
DROP TABLE IF EXISTS fte_data;
51+
DROP TABLE IF EXISTS intern_data;
5052
DROP TABLE IF EXISTS books_sold;
5153
DROP SCHEMA IF EXISTS [foo];
5254
COMMIT;
@@ -256,6 +258,23 @@ CREATE TABLE mappedbookmarks
256258
bkname nvarchar(50) NOT NULL
257259
)
258260

261+
create Table fte_data(
262+
id int IDENTITY(5001,1),
263+
u_id int DEFAULT 2,
264+
name varchar(50),
265+
position varchar(20),
266+
salary int default 20,
267+
PRIMARY KEY(id, u_id)
268+
);
269+
270+
create Table intern_data(
271+
id int,
272+
months int default 2 NOT NULL,
273+
name varchar(50),
274+
salary int default 15,
275+
PRIMARY KEY(id, months)
276+
);
277+
259278
create table books_sold
260279
(
261280
id int PRIMARY KEY not null,
@@ -468,6 +487,13 @@ VALUES
468487
(10, 'Aaron', 'F.', 'Burtle', null, null)
469488
SET IDENTITY_INSERT authors_history OFF
470489

490+
SET IDENTITY_INSERT fte_data ON
491+
insert into fte_data(id, name, position, salary) values(1, 'Ellie', 'Junior Dev', 20), (2, 'Chris', 'Senior Dev', 40);
492+
SET IDENTITY_INSERT fte_data OFF
493+
494+
insert into intern_data(id, months, name) values(1, 3, 'Tess'), (2, 4, 'Frank');
495+
496+
471497
INSERT INTO revenues(id, category, revenue, accessible_role) VALUES (1, 'Book', 5000, 'Anonymous'), (2, 'Comics', 10000, 'Anonymous'),
472498
(3, 'Journals', 20000, 'Authenticated'), (4, 'Series', 40000, 'Authenticated');
473499

@@ -545,3 +571,63 @@ EXEC('CREATE FUNCTION dbo.revenuesPredicate(@accessible_role varchar(20))
545571
-- Adding a security policy which would restrict access to the rows in revenues table for
546572
-- SELECT,UPDATE,DELETE operations using the filter predicate dbo.revenuesPredicate.
547573
EXEC('CREATE SECURITY POLICY dbo.revenuesSecPolicy ADD FILTER PREDICATE dbo.revenuesPredicate(accessible_role) ON dbo.revenues;');
574+
575+
-- Create an after insert trigger for the fte_data table.
576+
-- This trigger will ensure that no employee record can be inserted with a salary of more than 100 and less than 0.
577+
EXEC('CREATE OR ALTER TRIGGER fte_data_after_insert_trigger ON fte_data AFTER INSERT AS
578+
BEGIN
579+
UPDATE fte_data
580+
SET fte_data.salary =
581+
case
582+
when fte_data.salary > 100 then 100
583+
when fte_data.salary < 0 then 0
584+
else fte_data.salary
585+
end
586+
FROM fte_data
587+
INNER JOIN inserted ON fte_data.id = inserted.id AND fte_data.u_id = inserted.u_id;
588+
END;');
589+
590+
-- Create an after update trigger for the fte_data table.
591+
-- This trigger will ensure that no employee record can be updated to have a salary of more than 150 and less than 0.
592+
EXEC('CREATE OR ALTER TRIGGER fte_data_after_update_trigger ON fte_data AFTER UPDATE AS
593+
BEGIN
594+
UPDATE fte_data
595+
SET fte_data.salary =
596+
case
597+
when fte_data.salary > 150 then 150
598+
when fte_data.salary < 0 then 0
599+
else fte_data.salary
600+
end
601+
FROM fte_data
602+
INNER JOIN inserted ON fte_data.id = inserted.id AND fte_data.u_id = inserted.u_id;
603+
END;');
604+
605+
-- Create an after insert trigger for the intern_data table.
606+
-- This trigger will ensure that no intern record can be inserted with a salary of more than 30 and less than 0.
607+
EXEC('CREATE OR ALTER TRIGGER intern_data_after_insert_trigger ON intern_data AFTER INSERT AS
608+
BEGIN
609+
UPDATE intern_data
610+
SET intern_data.salary =
611+
case
612+
when intern_data.salary > 30 then 30
613+
when intern_data.salary < 0 then 0
614+
else intern_data.salary
615+
end
616+
FROM intern_data
617+
INNER JOIN inserted ON intern_data.id = inserted.id AND intern_data.months = inserted.months;
618+
END;');
619+
620+
-- Create an after update trigger for the intern_data table.
621+
-- This trigger will ensure that no intern record can be updated to have a salary of more than 50 and less than 0.
622+
EXEC('CREATE OR ALTER TRIGGER intern_data_after_update_trigger ON intern_data AFTER UPDATE AS
623+
BEGIN
624+
UPDATE intern_data
625+
SET intern_data.salary =
626+
case
627+
when intern_data.salary > 50 then 50
628+
when intern_data.salary < 0 then 0
629+
else intern_data.salary
630+
end
631+
FROM intern_data
632+
INNER JOIN inserted ON intern_data.id = inserted.id AND intern_data.months = inserted.months;
633+
END;');

src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2359,6 +2359,58 @@
23592359
}
23602360
}
23612361
},
2362+
{
2363+
FteData: {
2364+
Source: {
2365+
Object: fte_data,
2366+
Type: Table
2367+
},
2368+
GraphQL: {
2369+
Singular: FteData,
2370+
Plural: FteData,
2371+
Enabled: true
2372+
},
2373+
Rest: {
2374+
Enabled: true
2375+
},
2376+
Permissions: [
2377+
{
2378+
Role: anonymous,
2379+
Actions: [
2380+
{
2381+
Action: *
2382+
}
2383+
]
2384+
}
2385+
]
2386+
}
2387+
},
2388+
{
2389+
InternData: {
2390+
Source: {
2391+
Object: intern_data,
2392+
Type: Table
2393+
},
2394+
GraphQL: {
2395+
Singular: InternData,
2396+
Plural: InternData,
2397+
Enabled: true
2398+
},
2399+
Rest: {
2400+
Enabled: true
2401+
},
2402+
Permissions: [
2403+
{
2404+
Role: anonymous,
2405+
Actions: [
2406+
{
2407+
Action: *
2408+
}
2409+
]
2410+
}
2411+
]
2412+
}
2413+
},
23622414
{
23632415
BooksSold: {
23642416
Source: {

0 commit comments

Comments
 (0)