Skip to content

Database policy support for PUT/PATCH operations - PostgreSQL#3694

Open
ArjunNarendra wants to merge 12 commits into
Azure:mainfrom
ArjunNarendra:user/an/db-policy-for-put-patch-postgresql
Open

Database policy support for PUT/PATCH operations - PostgreSQL#3694
ArjunNarendra wants to merge 12 commits into
Azure:mainfrom
ArjunNarendra:user/an/db-policy-for-put-patch-postgresql

Conversation

@ArjunNarendra

@ArjunNarendra ArjunNarendra commented Jul 5, 2026

Copy link
Copy Markdown

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

// 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} FROM {tableName} WHERE {pkPredicates}";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concurrency concern: Build(SqlUpsertQueryStructure) emits two separate statements — RS1 (SELECT COUNT(*) ... AS cnt_rows_to_update) and RS2 (the upsert CTE). Under Postgres default READ COMMITTED isolation these run on independent snapshots, so a concurrent insert/delete of the PK row between the two statements can make PostgreSqlExecutor.GetMultipleResultSetsIfAnyAsync misclassify insert vs update:

  • count=0 at RS1, row inserted concurrently before the CTE runs -> update_cte updates it and RS2 is non-empty -> the count==0 branch returns the row WITHOUT IS_UPDATE_RESULT_SET -> an update is reported as 201 Created instead of 200 OK.
  • count=1 at RS1, row deleted concurrently -> insert_cte runs and RS2 is non-empty -> IS_UPDATE_RESULT_SET=true is set on an insert -> reported as 200 instead of 201 Created.

The CTE already returns the ___upsert_op___ identifier (inserted/updated), which is atomic with the write. Consider deriving insert-vs-update from that label rather than a separate COUNT statement (the COUNT is only strictly needed to disambiguate 403 vs 404 when RS2 is empty), which eliminates the race.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Known Issue] Support for database policy for PUT/PATCH operations in PostgreSQL

3 participants