Skip to content

Eliminate code duplication requests between Upsert and Insert #3287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aaronburtle wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Eliminate code duplication requests between Upsert and Insert #3287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d623359
let keyless upsert fall through to insert
aaronburtle Mar 16, 2026
e05f279
Factor out dupe code, add test for keys in body update
aaronburtle Mar 16, 2026
94151e7
add foo. to pgsql
aaronburtle Mar 17, 2026
9c017fc
Potential fix for pull request finding
aaronburtle Mar 17, 2026
c57a28f
Potential fix for pull request finding
aaronburtle Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 54 additions & 72 deletions src/Core/Resolvers/SqlMutationEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Transactions;
using System.Collections.Generic;
using Azure.DataApiBuilder.Auth;
using Azure.DataApiBuilder.Config.DatabasePrimitives;
using Azure.DataApiBuilder.Config.ObjectModel;
Expand Down Expand Up @@ -542,86 +543,59 @@ await queryExecutor.ExecuteQueryAsync(

try
{
if (context.OperationType is EntityActionOperation.Upsert || context.OperationType is EntityActionOperation.UpsertIncremental)
// When the URL path has no primary key route but the request body contains
// ALL PK columns, promote those values into PrimaryKeyValuePairs so the upsert
// path can build a proper UPDATE ... WHERE pk = value (with INSERT fallback)
// instead of blindly inserting and failing on a PK violation.
// Every PK column must be present — including auto-generated ones — because
// a partial composite key cannot uniquely identify a row for UPDATE.
if ((context.OperationType is EntityActionOperation.Upsert || context.OperationType is EntityActionOperation.UpsertIncremental)
&& context.PrimaryKeyValuePairs.Count == 0)
{
// When no primary key values are provided (empty PrimaryKeyValuePairs),
// there is no row to look up for update. The upsert degenerates to a
// pure INSERT - execute it via the insert path so the mutation engine
// generates a correct INSERT statement instead of an UPDATE with an
// empty WHERE clause (WHERE 1 = 1) that would match every row.
if (context.PrimaryKeyValuePairs.Count == 0)
{
DbResultSetRow? insertResultRow = null;
SourceDefinition sourceDefinition = sqlMetadataProvider.GetSourceDefinition(context.EntityName);
bool allPKsInBody = true;
List<string> pkExposedNames = new();

try
foreach (string pk in sourceDefinition.PrimaryKey)
{
if (!sqlMetadataProvider.TryGetExposedColumnName(context.EntityName, pk, out string? exposedName))
{
using (TransactionScope transactionScope = ConstructTransactionScopeBasedOnDbType(sqlMetadataProvider))
{
insertResultRow =
await PerformMutationOperation(
entityName: context.EntityName,
operationType: EntityActionOperation.Insert,
parameters: parameters,
sqlMetadataProvider: sqlMetadataProvider);

if (insertResultRow is null)
{
throw new DataApiBuilderException(
message: "An unexpected error occurred while trying to execute the query.",
statusCode: HttpStatusCode.InternalServerError,
subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError);
}

if (insertResultRow.Columns.Count == 0)
{
throw new DataApiBuilderException(
message: "Could not insert row with given values.",
statusCode: HttpStatusCode.Forbidden,
subStatusCode: DataApiBuilderException.SubStatusCodes.DatabasePolicyFailure);
}

if (isDatabasePolicyDefinedForReadAction)
{
FindRequestContext findRequestContext = ConstructFindRequestContext(context, insertResultRow, roleName, sqlMetadataProvider);
IQueryEngine queryEngine = _queryEngineFactory.GetQueryEngine(sqlMetadataProvider.GetDatabaseType());
selectOperationResponse = await queryEngine.ExecuteAsync(findRequestContext);
}

transactionScope.Complete();
}
allPKsInBody = false;
break;
}
catch (TransactionException)

if (!context.FieldValuePairsInBody.ContainsKey(exposedName))
{
throw _dabExceptionWithTransactionErrorMessage;
allPKsInBody = false;
break;
}

if (isReadPermissionConfiguredForRole && !isDatabasePolicyDefinedForReadAction)
pkExposedNames.Add(exposedName);
}

if (allPKsInBody)
{
// Populate PrimaryKeyValuePairs from the body so the upsert path
// generates an UPDATE with the correct WHERE clause.
foreach (string exposedName in pkExposedNames)
{
IEnumerable<string> allowedExposedColumns = _authorizationResolver.GetAllowedExposedColumns(context.EntityName, roleName, EntityActionOperation.Read);
foreach (string columnInResponse in insertResultRow.Columns.Keys)
if (context.FieldValuePairsInBody.TryGetValue(exposedName, out object? value))
{
if (!allowedExposedColumns.Contains(columnInResponse))
{
insertResultRow.Columns.Remove(columnInResponse);
}
context.PrimaryKeyValuePairs[exposedName] = value!;
}
}

string pkRouteForLocationHeader = isReadPermissionConfiguredForRole
? SqlResponseHelpers.ConstructPrimaryKeyRoute(context, insertResultRow.Columns, sqlMetadataProvider)
: string.Empty;

return SqlResponseHelpers.ConstructCreatedResultResponse(
insertResultRow.Columns,
selectOperationResponse,
pkRouteForLocationHeader,
isReadPermissionConfiguredForRole,
isDatabasePolicyDefinedForReadAction,
context.OperationType,
GetBaseRouteFromConfig(_runtimeConfigProvider.GetConfig()),
GetHttpContext());
}
}

// When an upsert still has no primary key values after checking the body,
// it degenerates to a pure INSERT. Fall through to the shared insert/update
// handling so the mutation engine generates a correct INSERT statement instead
// of an UPDATE with an empty WHERE clause (WHERE 1 = 1) that would match every row.
bool isKeylessUpsert = (context.OperationType is EntityActionOperation.Upsert || context.OperationType is EntityActionOperation.UpsertIncremental)
&& context.PrimaryKeyValuePairs.Count == 0;

if (!isKeylessUpsert && (context.OperationType is EntityActionOperation.Upsert || context.OperationType is EntityActionOperation.UpsertIncremental))
{
DbResultSet? upsertOperationResult;
DbResultSetRow upsertOperationResultSetRow;

Expand Down Expand Up @@ -723,7 +697,12 @@ await PerformMutationOperation(
}
else
{
// This code block gets executed when the operation type is one among Insert, Update or UpdateIncremental.
// This code block handles Insert, Update, UpdateIncremental,
// and keyless upsert (which degenerates to Insert).
EntityActionOperation effectiveOperationType = isKeylessUpsert
? EntityActionOperation.Insert
: context.OperationType;

DbResultSetRow? mutationResultRow = null;

try
Expand All @@ -734,13 +713,13 @@ await PerformMutationOperation(
mutationResultRow =
await PerformMutationOperation(
entityName: context.EntityName,
operationType: context.OperationType,
operationType: effectiveOperationType,
parameters: parameters,
sqlMetadataProvider: sqlMetadataProvider);

if (mutationResultRow is null || mutationResultRow.Columns.Count == 0)
{
if (context.OperationType is EntityActionOperation.Insert)
if (effectiveOperationType is EntityActionOperation.Insert)
{
if (mutationResultRow is null)
{
Expand Down Expand Up @@ -827,15 +806,18 @@ await PerformMutationOperation(
string primaryKeyRouteForLocationHeader = isReadPermissionConfiguredForRole ? SqlResponseHelpers.ConstructPrimaryKeyRoute(context, mutationResultRow!.Columns, sqlMetadataProvider)
: string.Empty;

if (context.OperationType is EntityActionOperation.Insert)
if (effectiveOperationType is EntityActionOperation.Insert)
{
// Location Header is made up of the Base URL of the request and the primary key of the item created.
// For POST requests, the primary key info would not be available in the URL and needs to be appended. So, the primary key of the newly created item
// which is stored in the primaryKeyRoute is used to construct the Location Header.
// Note: context.OperationType (not effectiveOperationType) is passed intentionally so that
// ConstructCreatedResultResponse can distinguish a keyless upsert from a true Insert when
// deciding whether to populate the Location header.
return SqlResponseHelpers.ConstructCreatedResultResponse(mutationResultRow!.Columns, selectOperationResponse, primaryKeyRouteForLocationHeader, isReadPermissionConfiguredForRole, isDatabasePolicyDefinedForReadAction, context.OperationType, GetBaseRouteFromConfig(_runtimeConfigProvider.GetConfig()), GetHttpContext());
}

if (context.OperationType is EntityActionOperation.Update || context.OperationType is EntityActionOperation.UpdateIncremental)
if (effectiveOperationType is EntityActionOperation.Update || effectiveOperationType is EntityActionOperation.UpdateIncremental)
{
return SqlResponseHelpers.ConstructOkMutationResponse(mutationResultRow!.Columns, selectOperationResponse, isReadPermissionConfiguredForRole, isDatabasePolicyDefinedForReadAction);
}
Expand Down
13 changes: 13 additions & 0 deletions src/Service.Tests/SqlTests/RestApiTests/Patch/MsSqlPatchApiTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ public class MsSqlPatchApiTests : PatchApiTestBase
$"AND [publisher_id] = 1234 " +
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
},
{
"PatchOne_Update_KeylessWithPKInBody_ExistingRow_Test",
$"SELECT [id], [title], [issue_number] FROM [foo].{ _integration_NonAutoGenPK_TableName } " +
$"WHERE [id] = 1 AND [title] = 'Updated Vogue' " +
$"AND [issue_number] = 1234 " +
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
},
{
"PatchOne_Insert_KeylessWithPKInBody_NewRow_Test",
$"SELECT [id], [title], [issue_number] FROM [foo].{ _integration_NonAutoGenPK_TableName } " +
$"WHERE [id] = { STARTING_ID_FOR_TEST_INSERTS } AND [title] = 'Brand New Magazine' " +
$"FOR JSON PATH, INCLUDE_NULL_VALUES, WITHOUT_ARRAY_WRAPPER"
},
{
"PatchOne_Insert_NonAutoGenPK_Test",
$"SELECT [id], [title], [issue_number] FROM [foo].{ _integration_NonAutoGenPK_TableName } " +
Expand Down
22 changes: 22 additions & 0 deletions src/Service.Tests/SqlTests/RestApiTests/Patch/MySqlPatchApiTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,28 @@ public class MySqlPatchApiTests : PatchApiTestBase
) AS subq
"
},
{
"PatchOne_Update_KeylessWithPKInBody_ExistingRow_Test",
@"SELECT JSON_OBJECT('id', id, 'title', title, 'issue_number', issue_number) AS data
FROM (
SELECT id, title, issue_number
FROM " + _integration_NonAutoGenPK_TableName + @"
WHERE id = 1
AND title = 'Updated Vogue' AND issue_number = 1234
) AS subq
"
},
{
"PatchOne_Insert_KeylessWithPKInBody_NewRow_Test",
@"SELECT JSON_OBJECT('id', id, 'title', title, 'issue_number', issue_number) AS data
FROM (
SELECT id, title, issue_number
FROM " + _integration_NonAutoGenPK_TableName + @"
WHERE id = " + STARTING_ID_FOR_TEST_INSERTS + @"
AND title = 'Brand New Magazine'
) AS subq
"
},
{
"PatchOne_Insert_NonAutoGenPK_Test",
@"SELECT JSON_OBJECT('id', id, 'title', title, 'issue_number', issue_number ) AS data
Expand Down
59 changes: 59 additions & 0 deletions src/Service.Tests/SqlTests/RestApiTests/Patch/PatchApiTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,65 @@ await SetupAndRunRestApiTest(
);
}

/// <summary>
/// Tests the PatchOne functionality with a REST PATCH request
/// without a primary key route, where the request body contains
/// all PK columns that match an existing row.
/// The engine should detect the PK in the body, route through the
/// upsert path, find the existing row, and perform an UPDATE (200 OK).
/// This is a regression test: previously, a keyless upsert with body PKs
/// always executed an INSERT, which would fail on a PK violation.
/// </summary>
[TestMethod]
public virtual async Task PatchOne_Update_KeylessWithPKInBody_ExistingRow_Test()
{
// id=1 exists in the magazines table with title='Vogue'.
// Sending a PATCH with the PK in the body should UPDATE the existing row.
string requestBody = @"
{
""id"": 1,
""title"": ""Updated Vogue""
}";

await SetupAndRunRestApiTest(
primaryKeyRoute: string.Empty,
queryString: null,
entityNameOrPath: _integration_NonAutoGenPK_EntityName,
sqlQuery: GetQuery(nameof(PatchOne_Update_KeylessWithPKInBody_ExistingRow_Test)),
operationType: EntityActionOperation.UpsertIncremental,
requestBody: requestBody,
expectedStatusCode: HttpStatusCode.OK
);
}

/// <summary>
/// Tests the PatchOne functionality with a REST PATCH request
/// without a primary key route, where the request body contains
/// all PK columns that do NOT match any existing row.
/// The engine should detect the PK in the body, route through the
/// upsert path, find no existing row, and perform an INSERT (201 Created).
/// </summary>
[TestMethod]
public virtual async Task PatchOne_Insert_KeylessWithPKInBody_NewRow_Test()
{
string requestBody = @"
{
""id"": " + STARTING_ID_FOR_TEST_INSERTS + @",
""title"": ""Brand New Magazine""
}";

await SetupAndRunRestApiTest(
primaryKeyRoute: string.Empty,
queryString: null,
entityNameOrPath: _integration_NonAutoGenPK_EntityName,
sqlQuery: GetQuery(nameof(PatchOne_Insert_KeylessWithPKInBody_NewRow_Test)),
operationType: EntityActionOperation.UpsertIncremental,
requestBody: requestBody,
expectedStatusCode: HttpStatusCode.Created,
expectedLocationHeader: string.Empty
);
}

/// <summary>
/// Tests successful execution of PATCH update requests on views
/// when requests try to modify fields belonging to one base table
Expand Down
24 changes: 24 additions & 0 deletions src/Service.Tests/SqlTests/RestApiTests/Patch/PostgreSqlPatchApiTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,30 @@ SELECT to_jsonb(subq) AS data
) AS subq
"
},
{
"PatchOne_Update_KeylessWithPKInBody_ExistingRow_Test",
@"
SELECT to_jsonb(subq) AS data
FROM (
SELECT id, title, issue_number
FROM " + "foo." + _integration_NonAutoGenPK_TableName + @"
WHERE id = 1
AND title = 'Updated Vogue' AND issue_number = 1234
) AS subq
"
},
{
"PatchOne_Insert_KeylessWithPKInBody_NewRow_Test",
@"
SELECT to_jsonb(subq) AS data
FROM (
SELECT id, title, issue_number
FROM " + "foo." + _integration_NonAutoGenPK_TableName + @"
WHERE id = " + STARTING_ID_FOR_TEST_INSERTS + @"
AND title = 'Brand New Magazine'
) AS subq
"
},
{
"PatchOne_Insert_Mapping_Test",
@"
Expand Down
Loading
Loading