Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. doc: Improve description of RLS policies applied by command type.

  2. Add new RLS tests to test policies applied by command type.

  1. Docs and tests for RLS policies applied by command type

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-03-27T16:03:21Z

    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.

Regards,
Dean

  2. Re: Docs and tests for RLS policies applied by command type

    Viktor Holmberg <v@viktorh.net> — 2025-10-20T14:02:01Z

    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.
>
> Regards,
> Dean

  3. Re: Docs and tests for RLS policies applied by command type

    Viktor Holmberg <v@viktorh.net> — 2025-10-20T16:00:43Z

    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.
> >
> > Regards,
> > Dean

  4. Re: Docs and tests for RLS policies applied by command type

    jian he <jian.universality@gmail.com> — 2025-10-23T08:22:27Z

    On Tue, Oct 21, 2025 at 12:01 AM Viktor Holmberg <v@viktorh.net> wrote:
>
> 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.
>
hi.
I only applied the 0001.

it would be better to add some comments to the regress tests, IMHO.
for example, for below:
+SELECT * FROM rls_test_src FOR UPDATE;
+SELECT * FROM rls_test_src FOR NO KEY UPDATE;
+SELECT * FROM rls_test_src FOR SHARE;
+SELECT * FROM rls_test_src FOR KEY SHARE;

we could add a comment such as:
"Expect both UPDATE and the SELECT command policies to be invoked for
these four below query".

seems missing tests for INSERT ... ON CONFLICT DO NOTHING
which only INSERT policy to be invoked.

The 0001 regess tests define several functions: sel_using_fn,
ins_check_fn, upd_using_fn,
upd_check_fn, and del_using_fn.
IMHO, these could be simplified (we probably only need two functions).

see the attached version for my attempt to reduce them.

  5. Re: Docs and tests for RLS policies applied by command type

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-10-23T15:14:56Z

    On Thu, 23 Oct 2025 at 09:23, jian he <jian.universality@gmail.com> wrote:
>
> On Tue, Oct 21, 2025 at 12:01 AM Viktor Holmberg <v@viktorh.net> wrote:
> >
> > 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.
> >
> hi.
> I only applied the 0001.
>
> it would be better to add some comments to the regress tests, IMHO.
> for example, for below:
> +SELECT * FROM rls_test_src FOR UPDATE;
> +SELECT * FROM rls_test_src FOR NO KEY UPDATE;
> +SELECT * FROM rls_test_src FOR SHARE;
> +SELECT * FROM rls_test_src FOR KEY SHARE;
>
> we could add a comment such as:
> "Expect both UPDATE and the SELECT command policies to be invoked for
> these four below query".

Thank you both for the reviews.

Attached is a new version with more comments in the tests, focusing on
what is expected from each test.

> The 0001 regess tests define several functions: sel_using_fn,
> ins_check_fn, upd_using_fn,
> upd_check_fn, and del_using_fn.
> IMHO, these could be simplified (we probably only need two functions).

Good point. Actually it can be done with just one function, further
reducing the amount of test code.

A recent commit reminded me that COPY ... TO also applies RLS SELECT
policies (and so does TABLE, though I doubt many people use that), so
I think it's worth testing and documenting those too. Updated patches
attached.

Regards,
Dean

  6. Re: Docs and tests for RLS policies applied by command type

    Viktor Holmberg <v@viktorh.net> — 2025-10-24T19:23:56Z

    Looks great. The test is easy to understand now. I’ve set the commitfest entry to “ready for committer”.

/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.
>
> Regards,
> Dean

  7. Re: Docs and tests for RLS policies applied by command type

    jian he <jian.universality@gmail.com> — 2025-10-27T05:02:24Z

    On Thu, Oct 23, 2025 at 11:15 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> Attached is a new version with more comments in the tests, focusing on
> what is expected from each test.
>
> > The 0001 regess tests define several functions: sel_using_fn,
> > ins_check_fn, upd_using_fn,
> > upd_check_fn, and del_using_fn.
> > IMHO, these could be simplified (we probably only need two functions).
>
> Good point. Actually it can be done with just one function, further
> reducing the amount of test code.
>

hi.

v2-0001 looks good to me.

> A recent commit reminded me that COPY ... TO also applies RLS SELECT
> policies (and so does TABLE, though I doubt many people use that), so
> I think it's worth testing and documenting those too. Updated patches
> attached.
>

other Utility commands will also invoke the SELECT/UPDATE policy.
The below several commands will invoke SELECT or UPDATE policy,
if rls_test_src have SELECT or UPDATE policy on it.

create table sss as SELECT * FROM rls_test_src FOR UPDATE;
explain analyze SELECT * FROM rls_test_src FOR UPDATE;
PREPARE q1 AS SELECT * FROM rls_test_src FOR UPDATE;
EXECUTE q1;
create MATERIALIZED view mv as SELECT * FROM rls_test_src FOR UPDATE
with no data;
REFRESH MATERIALIZED VIEW mv;
create MATERIALIZED view mv1 as SELECT * FROM rls_test_src FOR UPDATE;
DECLARE curs1 CURSOR WITH HOLD FOR SELECT * FROM rls_test_src;

While at it, I found out that
table "Policies Applied by Command Type" was missing SELECT FOR NO KEY UPDATE
and SELECT FOR KEY SHARE.


While at it create_policy.sgml, I am not sure the below sentence is
not fully accurate.
""
If an INSERT or UPDATE command attempts to add rows to the table that do not
pass the ALL policy's WITH CHECK expression, the entire command will be aborted.
""
The above sentence fails to mention the case when the WITH CHECK
expression does not exist.
for example:

create table tts(x int);
CREATE POLICY p1 ON tts FOR all using (x = 1);
grant select, insert on tts to alice;
alter table tts ENABLE ROW LEVEL SECURITY;
set role alice;
insert into tts values (2);

  8. Re: Docs and tests for RLS policies applied by command type

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-10-27T11:26:31Z

    On Mon, 27 Oct 2025 at 05:03, jian he <jian.universality@gmail.com> wrote:
>
> v2-0001 looks good to me.

Thanks. I've pushed that one.

> > A recent commit reminded me that COPY ... TO also applies RLS SELECT
> > policies (and so does TABLE, though I doubt many people use that), so
> > I think it's worth testing and documenting those too. Updated patches
> > attached.
>
> other Utility commands will also invoke the SELECT/UPDATE policy.
> The below several commands will invoke SELECT or UPDATE policy,
> if rls_test_src have SELECT or UPDATE policy on it.

I don't think it's worth documenting every single command that
includes a SELECT somewhere in it. Adding too much to the docs makes
them harder to read, not easier, and I think it's pretty clear that
these examples end up executing a SELECT, and so it should be clear
that they apply the RLS SELECT/UPDATE policies.

In fact, on reflection, I don't think it's worth mentioning TABLE here
either, since it's not really a separate command. It doesn't have its
own doc page, but instead is only mentioned on the SELECT doc page,
which says that it's equivalent to SELECT * FROM table, so it should
be clear that it applies SELECT policies.

So I think I'll stick to just mentioning COPY .. TO, since it might
not otherwise be obvious that it does apply RLS SELECT policies.

> While at it, I found out that
> table "Policies Applied by Command Type" was missing SELECT FOR NO KEY UPDATE
> and SELECT FOR KEY SHARE.

I don't think that's necessary. We could try to say something like
"SELECT ... FOR [NO KEY] UPDATE / [KEY] SHARE", but I think that would
make it harder to read, given the lack of space in that table. Several
other places in the documentation already use the text "FOR
UPDATE/SHARE" to include all 4 variants of SELECT row locking,
including the SELECT doc page itself, so I think it should be
sufficient here too.

> While at it create_policy.sgml, I am not sure the below sentence is
> not fully accurate.
> ""
> If an INSERT or UPDATE command attempts to add rows to the table that do not
> pass the ALL policy's WITH CHECK expression, the entire command will be aborted.
> ""
> The above sentence fails to mention the case when the WITH CHECK
> expression does not exist.

Hmm, the sentence immediately before that explains that the USING
expression will be used to check new rows, if there is no WITH CHECK
expression, but that's using UPDATE as an example, so I guess it's
worth being clear that the same applies to an INSERT.

Updated patch attached.

Regards,
Dean

  9. Re: Docs and tests for RLS policies applied by command type

    jian he <jian.universality@gmail.com> — 2025-10-28T03:33:51Z

    On Mon, Oct 27, 2025 at 7:26 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> Updated patch attached.
>

hi.

TABLE, Policies Applied by Command Type
MERGE related change looks very similar to standalone INSERT/UPDATE/DELETE.
overall v3-0001 looks good to me.

while reading the doc, this description in UPDATE section:
"""
Note, however, that unlike a standalone UPDATE command, if the existing row does
not pass the USING expressions, an error will be thrown (the UPDATE path will
never be silently avoided).
""

I think the above statement also applies to MERGE ... THEN UPDATE.
Perhaps the table “Policies Applied by Command Type” already conveys this,
but I’m not sure.

below shows that MERGE ... THEN UPDATE USING expression does not pass will
result error

begin;
reset role;
drop table if exists tts;
create table tts(a int , b int);
insert into tts values (4, 5), (2,5), (3, 5);
CREATE POLICY p1 ON tts FOR SELECT USING (a < 3);
CREATE POLICY p3 ON tts FOR UPDATE USING (a > 3) WITH CHECK (b = 5);
grant all on tts to alice;
ALTER TABLE tts ENABLE ROW LEVEL SECURITY;
commit;

BEGIN;
SET ROLE alice;
MERGE INTO tts d USING (SELECT 2 as sdid) s ON a = s.sdid
WHEN MATCHED THEN
    UPDATE SET b = 5;
ROLLBACK;

  10. Re: Docs and tests for RLS policies applied by command type

    Dean Rasheed <dean.a.rasheed@gmail.com> — 2025-11-03T11:21:49Z

    On Tue, 28 Oct 2025 at 03:34, jian he <jian.universality@gmail.com> wrote:
>
> while reading the doc, this description in UPDATE section:
> """
> Note, however, that unlike a standalone UPDATE command, if the existing row does
> not pass the USING expressions, an error will be thrown (the UPDATE path will
> never be silently avoided).
> ""
>
> I think the above statement also applies to MERGE ... THEN UPDATE.
> Perhaps the table “Policies Applied by Command Type” already conveys this,
> but I’m not sure.

Yeah, reading through the text on that page in more detail, there are
a number of other omissions, or places that aren't quite fully
correct, so I've gone through those and attempted to improve things.

Also, I think it would be better if the table made the distinction
between policy checks that just filter out rows, without throwing an
error, and checks that do cause an error to be thrown.

v4 attached.

Regards,
Dean

  11. Re: Docs and tests for RLS policies applied by command type

    jian he <jian.universality@gmail.com> — 2025-11-06T02:45:58Z

    On Mon, Nov 3, 2025 at 7:22 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> Yeah, reading through the text on that page in more detail, there are
> a number of other omissions, or places that aren't quite fully
> correct, so I've gone through those and attempted to improve things.
>
> Also, I think it would be better if the table made the distinction
> between policy checks that just filter out rows, without throwing an
> error, and checks that do cause an error to be thrown.
>
> v4 attached.
>

some of the <literal> can be replaced by <command>, for example:
+         A <literal>MERGE</literal> command requires <literal>SELECT</literal>
+         permissions on both the source and target relations, and so each

currently the visual appearance is the same, I guess it's not a big deal.

(Table 300. Policies Applied by Command Type) is way more intuitive.

overall looks good to me.