Database policy support for PUT/PATCH operations - PostgreSQL by ArjunNarendra · Pull Request #3694 · Azure/data-api-builder

ArjunNarendra · 2026-07-05T03:15:39Z

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?

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.
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).
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.
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

…ate error/success message is returned

…t processing

…te/update policies

Copilot

Pull request overview

This PR extends PostgreSQL PUT/PATCH (upsert) behavior to correctly apply database policies for both the update and insert branches, aligning Postgres behavior with the expected policy semantics for REST upserts.

Changes:

Updated PostgreSQL upsert SQL generation to evaluate update-policy vs create-policy depending on which branch executes, while preserving “try update then insert” semantics.
Added a PostgreSQL-specific GetMultipleResultSetsIfAnyAsync implementation to interpret multi-result-set upsert outcomes and return 403/404 appropriately.
Updated PostgreSQL REST integration tests and test config to exercise create-policy behavior for PUT/PATCH insert cases.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Service.Tests/SqlTests/RestApiTests/Put/PostgreSqlPutApiTests.cs	Adds expected SQL for PUT insert-with-policy and removes ignored overrides so tests run.
src/Service.Tests/SqlTests/RestApiTests/Patch/PostgreSqlPatchApiTests.cs	Adds expected SQL for PATCH insert-with-policy and removes ignored overrides so tests run.
src/Service.Tests/dab-config.PostgreSql.json	Adds a PostgreSQL create-action database policy used by the new PUT/PATCH scenarios.
src/Core/Resolvers/SqlMutationEngine.cs	Passes additional context into the result-set handler for upsert result interpretation.
src/Core/Resolvers/PostgresQueryBuilder.cs	Reworks PostgreSQL upsert SQL to support separate create/update policies and emit a PK-existence count result set.
src/Core/Resolvers/PostgreSqlExecutor.cs	Implements PostgreSQL-specific multi-result-set handling to map policy failures vs not-found.
src/Core/Configurations/RuntimeConfigValidator.cs	Allows PostgreSQL to define create-action database policies in config validation.
config-generators/postgresql-commands.txt	Updates generator commands to set separate create/read permissions and the new create policy.

…er a PostgreSQL upsert operation performed an update or not. Reliance on this flag could potentially lead to incorrect HTTP response code being sent in certain race scenarios.

… whether an upsert operation performed an update or not because it is required by other database providers and does not apply for PostgreSQL

ArjunNarendra · 2026-07-14T18:10:27Z

So it looks like all the automated tests in Azure pipelines are passing now but at one point one of the Cosmos tests was failing. Specifically, one of the tests in the TestTimePartitionedSampler test method in the SamplerTests.cs test class was failing with the following error: Assert.IsTrue failed. Expected result count is 10 and Actual result count is 8. The specific data row that was causing this failure was [DataRow(null, 1, 0, 10, DisplayName = "Retrieve 10 records, if 1 item is allowed to fetch from each group and number of groups is 10 (i.e default)")]. So it was passing before until I merged the main upstream branch into my feature branch which is odd because it would suggest that the changes pushed to main were breaking the test or some weird interaction between the changes in the feature branch and the new changes in main were breaking the test. I tried to rerun the failing test suites but that particular test was still failing. Then, I converted this PR to a draft and then reconverted it back to a 'ready for review' state. That seemed to have done the trick and now all the test suites are passing. Therefore, I am not even sure what caused that one Cosmos test to fail in the first place; I think it can be chalked up to some flakiness in the environment or inconsistencies in the Azure Pipelines environment.

ArjunNarendra · 2026-07-15T19:47:13Z

/azp run

azure-pipelines · 2026-07-15T19:47:39Z

Azure Pipelines: Successfully started running 6 pipeline(s).

RubenCerna2079

I don't think the use of the isFallbackToUpdate is correct. Please remove it if possible from the PostgreSqlExecutor.cs file.

RubenCerna2079 · 2026-07-16T22:00:46Z

+
+                if (isFallbackToUpdate)
+                {
+                    if (args is not null && args.Count > 1)


Isn't this if statement redundant? If isFallbackToUpdate true then it means args is is not null and its count is > 2.

Yes, you are right. I will make this change.

I removed the check and I am getting the below compiler/build error:

In externally visible method 'Task<DbResultSet> PostgreSqlQueryExecutor.GetMultipleResultSetsIfAnyAsync(DbDataReader dbDataReader, List<string>? args = null)', validate parameter 'args' is non-null before using it. If appropriate, throw an 'ArgumentNullException' when the argument is 'null'.[CA1062](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1062)

For some reason, even though we are guaranteed that both args is not null and count > 2 because isFallbackToUpdate is true, the compiler is not able to make this logical leap. I played around with it and, of the two predicates, only args is not null is required for the code to compile successfully.

I am going to leave the code as is to make the compiler happy, but let me know if you still have any concerns with this.

I think you should be able to avoid this issue by stating the args value is not null by giving it the ! sign. So I think it would look something like this args![0]. But if it doesn't work it is fine to leave it as it is.

Yep, it works! Thanks. I made the change.

Wait...I think the check is necessary now. Since I took isFallbackToUpdate out of args, we have no idea what args contains anymore (since we don't extract isFallbackToUpdate from args[2] anymore before this if check). I am not going to make the change. Let me know if my logic makes sense.

…via the query result instead

…or insert, update, and update incremental operations

souvikghosh04

LGTM

souvikghosh04 · 2026-07-22T15:05:17Z

+                           ? new List<string> { prettyPrintPk, entityName }
+                           : new List<string> { prettyPrintPk, entityName });


the ternary conditions are same and this doesn't make any difference.

Since the isFallbackToUpdate was removed from the PostgresQueryBuilder then we can just remove this shorthand if else statement.

Oh my bad...must have missed it. Thanks for catching it.

souvikghosh04 · 2026-07-22T15:31:06Z

+                    $"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}";


question- this won't have a race conditions right?

I don't think so since it is within a SQL statement. However, Mihir did bring up a race condition that I fixed here: #3694 (comment)

ArjunNarendra added 6 commits July 4, 2026 19:37

Build upsert queries appropriately with insert/create policies

626d6f0

Change logic to determine how to process result sets so that appropri…

3c9b908

…ate error/success message is returned

Provide another parameter when executing the query to help with resul…

73d536c

…t processing

Add (unignore) relevant tests

b5f3734

Add create policy config and permissions for PostgreSQL tests

000ec1b

Modify policy updates when generating the config file to reflect crea…

4e06b23

…te/update policies

Copilot AI review requested due to automatic review settings July 5, 2026 03:15

ArjunNarendra requested review from Alekhya-Polavarapu, Aniruddh25, JerryNixon, RubenCerna2079, aaronburtle, anushakolan, rusamant, sourabh1007, souvikghosh04, stuartpa and vadeveka as code owners July 5, 2026 03:15

Copilot started reviewing on behalf of ArjunNarendra July 5, 2026 03:16 View session

Copilot AI reviewed Jul 5, 2026

View reviewed changes

Comment thread src/Core/Resolvers/SqlMutationEngine.cs Outdated

Comment thread src/Core/Configurations/RuntimeConfigValidator.cs

ArjunNarendra added 6 commits July 5, 2026 12:00

Remove unnecessary using statements

2333d8e

Update PostgreSQL verified snapshot policies

e744cae

Only pass in isFallbackToUpdate parameter for PostgreSQL

849b32f

Update PostgreSQL insert query builder to account for create policy

fa64d55

Update unit test to reflect support for create policy for PostgreSQL

ddde125

Merge branch 'main' into user/an/db-policy-for-put-patch-postgresql

b96b0c6

unattendedfaxmachine reviewed Jul 7, 2026

View reviewed changes

Comment thread src/Core/Resolvers/PostgresQueryBuilder.cs Outdated

ArjunNarendra added 3 commits July 13, 2026 17:05

Remove dependency on IS_UPDATE_RESULT_SET flag when determining wheth…

af1f7b2

…er a PostgreSQL upsert operation performed an update or not. Reliance on this flag could potentially lead to incorrect HTTP response code being sent in certain race scenarios.

Add back the dependency on IS_UPDATE_RESULT_SET flag when determining…

a2a1de1

… whether an upsert operation performed an update or not because it is required by other database providers and does not apply for PostgreSQL

Merge branch 'main' into user/an/db-policy-for-put-patch-postgresql

fa30fcd

ArjunNarendra marked this pull request as draft July 14, 2026 05:11

ArjunNarendra marked this pull request as ready for review July 14, 2026 05:11

Merge branch 'main' into user/an/db-policy-for-put-patch-postgresql

56723d6

unattendedfaxmachine approved these changes Jul 15, 2026

View reviewed changes

ArjunNarendra added the assign-for-review label Jul 15, 2026

github-actions Bot assigned RubenCerna2079 and souvikghosh04 Jul 15, 2026

github-actions Bot removed the assign-for-review label Jul 15, 2026

RubenCerna2079 requested changes Jul 16, 2026

View reviewed changes

RubenCerna2079 reviewed Jul 16, 2026

View reviewed changes

Comment thread src/Core/Resolvers/PostgreSqlExecutor.cs

ArjunNarendra linked an issue Jul 20, 2026 that may be closed by this pull request

[Known Issue] Database policy support for Create action in PostgreSQL #1334

Open

Remove isFallbackToUpdate flag from argument list and pass this flag …

02b8411

…via the query result instead

unattendedfaxmachine requested a review from RubenCerna2079 July 20, 2026 23:27

Simplify if-else structure when determining what exception to throw f…

3fddd8f

…or insert, update, and update incremental operations

souvikghosh04 approved these changes Jul 22, 2026

View reviewed changes

Remove seperate argument list for PostgreSQL when performing an upsert

74854a8

		? new List<string> { prettyPrintPk, entityName }
		: new List<string> { prettyPrintPk, entityName });

Uh oh!

Conversation

ArjunNarendra commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why make this change?

What is this change?

How was this tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ArjunNarendra commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArjunNarendra commented Jul 15, 2026

Uh oh!

azure-pipelines Bot commented Jul 15, 2026

Uh oh!

RubenCerna2079 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

souvikghosh04 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArjunNarendra Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ArjunNarendra commented Jul 5, 2026 •

edited

Loading

ArjunNarendra commented Jul 14, 2026 •

edited

Loading

ArjunNarendra Jul 22, 2026 •

edited

Loading