Database policy support for PUT/PATCH operations - PostgreSQL#3694
Database policy support for PUT/PATCH operations - PostgreSQL#3694ArjunNarendra wants to merge 12 commits into
Conversation
…ate error/success message is returned
…te/update policies
There was a problem hiding this comment.
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
GetMultipleResultSetsIfAnyAsyncimplementation 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. |
|
|
||
| // 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}"; |
There was a problem hiding this comment.
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_cteupdates it and RS2 is non-empty -> thecount==0branch returns the row WITHOUTIS_UPDATE_RESULT_SET-> an update is reported as 201 Created instead of 200 OK. - count=1 at RS1, row deleted concurrently ->
insert_cteruns and RS2 is non-empty ->IS_UPDATE_RESULT_SET=trueis 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.
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.GetMultipleResultSetsIfAnyAsynchas been provided another implementation specific to PostgreSql inPostgreSqlExecutor. TheDbDataReaderinstance for the query being executed for the PUT/PATCH operation will always contain two result sets.Different scenarios are added to the method
PostgreSqlExecutor.GetMultipleResultSetsIfAnyAsyncto 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