Oops, looks like CI got mad as I didn’t include all patch files - trying again.
/Viktor
On 20 Oct 2025 at 16:02 +0200, Viktor Holmberg <v@viktorh.net>, wrote:
I’ve had a look at this.
The doc updates make it much clearer how things work.
For the new test, I’ve verified that they pass. I also think that having them is very good, considering the complexity of the RLS system. I found the added test quite hectic to follow, but this could just be a me problem (not very used to reading the postgres test code). I’ve attached a patch with AI-generated comments, which helped me understand the test, if that is of any help. If you could add some yourself, that would of course be even better.
Regardless of if those comments are included or not, which I leave to you, I think this is good to commit.
/Viktor
On 20 Oct 2025 at 15:29 +0200, Dean Rasheed <dean.a.rasheed@gmail.com>, wrote:
While looking at the INSERT ... ON CONFLICT DO SELECT patch, I noticed that the "Policies Applied by Command Type" table on the CREATE POLICY page doesn't fully or accurately describe all the policies that are actually checked in all cases:
* INSERT ON CONFLICT checks the new row from the INSERT against SELECT policy expressions, regardless of what ON CONFLICT action is performed.
* If an ON CONFLICT DO UPDATE is executed, the new row from the auxiliary UPDATE command is also checked against SELECT policy expressions.
* MERGE always checks all candidate source and target rows against SELECT policy expressions, even if no action is performed.
* MERGE ... THEN INSERT checks the new row against SELECT policy expressions, if there is a RETURNING clause.
* MERGE ... THEN UPDATE always checks the new and existing rows against SELECT policy expressions, even if there is no RETURNING clause.
* MERGE ... THEN DELETE isn't mentioned at all. It always checks the existing row against SELECT policy expressions.
I think having MERGE use the same row in the doc table as other commands makes it harder to read, and it would be better to just list each of the MERGE cases separately, even if that does involve some repetition.
In addition, a paragraph above the table for INSERT policies says:
""" Note that INSERT with ON CONFLICT DO UPDATE checks INSERT policies' WITH CHECK expressions only for rows appended to the relation by the INSERT path. """
Maybe that was once true, but it isn't true now, in any supported PG version. The WITH CHECK expressions from INSERT policies are always checked, regardless of which path it ends up taking.
I think it would be good to have regression tests specifically covering all these cases. Yes, there are a lot of existing RLS regression tests, but they tend to cover more complex scenarios, and focus on whether the result of the command was what was expected, rather than precisely which policies were checked in the process. Thus, it's not obvious whether they provide complete coverage.
So patch 0001, attached, adds a new set of regression tests, near the start of rowsecurity.sql, which specifically tests which policies are applied for each command variant.
Patch 0002 updates the doc table to try to be clearer and more accurate, and consistent with the test results from 0001, and fixes the paragraph mentioned above.
