Re: Fix RLS checks for UPDATE/DELETE FOR PORTION OF leftover rows

On Tue, Jun 30, 2026 at 1:08 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
> > On Jun 30, 2026, at 16:00, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >
> > Yes, that’s the same issue. I saw Paul wrote this there:
> > ```
> > Skipping the RLS checks to insert the leftovers seems like the correct
> > behavior to me, since we are skipping the ACL checks (per the
> > standard). Shouldn't it be consistent?
> > I think the reason we skip the checks is that semantically, the
> > leftovers aren't changing anything: they are preserving the history
> > that is already there.
> > ```
> >
> > That explains why the ACL checks are skipped as stated in the doc, but I don’t think the same reasoning should apply to RLS checks. As I explained in my patch email, for example, directly inserting [70,100) is blocked by policy t_ins, but a user can work around that by inserting [1,100) and then updating [30,70), which seems like a security hole.
> >
>
> I just noticed that I forgot the attached again.

Thank you for working on this!

I still think it is a weird inconsistency to check RLS policies when
we're skipping ACLs, but there are a couple things I like about it:

- It gives the user more choice. They can use the policy to make
whatever decision they want. So enforcing the policy offers a superset
of the functionality of not enforcing it (but see below).

- RLS is maybe more general than just permissions. Your examples
aren't really about permissions. You can write policies that are more
like CHECK constraints, but with selective enforcement. (OTOH is that
really a good idea?)

One thing I'm concerned about is this: if we apply INSERT policies for
UPDATE FOR PORTION OF, how do we support users who want to forbid
*adding new entities* but allow *changing the history of existing
entities*? By applying INSERT policies, we are blocking them from
both. So maybe this doesn't really offer a superset of functionality
after all. And I think my use case is fairly realistic. More realistic
than wanting to allow updates but not if they update only part of the
history.

If a policy had a way of discerning when an INSERT is for a temporal
leftover, then there would be no problem: the user could decide to let
those through. But I don't think we have a way to do that, do we? If
there might be a way to do it in the future, that would also alleviate
my concern here. I already have patches to expose FOR PORTION OF
parameters in TG_* variables. Maybe a policy could use something
similar.

@@ -393,6 +393,51 @@ get_row_security_policies(Query *root,
RangeTblEntry *rte, int rt_index,
}
}

+ /*
+ * UPDATE/DELETE FOR PORTION OF may insert leftover rows to preserve the
+ * portions of the old row not covered by the target range. Those hidden
+ * inserts go through ExecInsert(), so they need the same INSERT RLS WITH
+ * CHECK options as ordinary INSERTs.
+ */
+ if (root->forPortionOf != NULL && rt_index == root->resultRelation &&
+ (commandType == CMD_UPDATE || commandType == CMD_DELETE))
+ {
+ List *insert_permissive_policies;
+ List *insert_restrictive_policies;
+
+ get_policies_for_relation(rel, CMD_INSERT, user_id,
+ &insert_permissive_policies,
+ &insert_restrictive_policies);
+ add_with_check_options(rel, rt_index,
+ WCO_RLS_INSERT_CHECK,
+ insert_permissive_policies,
+ insert_restrictive_policies,
+ withCheckOptions,
+ hasSubLinks,
+ false);

Is this the right way to do these checks? I'll try to look into it a
bit more today, but does this cause problems when an UPDATE *doesn't*
create leftovers?

We should update the documentation as well. I think it should point
out the contrast between ACLs and RLS here.

Yours,

--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com