equalPolicy() doesn't compare the permissive flag

On Thu, 2026-07-09 at 16:28 +0200, Andreas Lind wrote:

While working on an unrelated RLS patch, I noticed that equalPolicy() in
relcache.c doesn't compare the permissive field of RowSecurityPolicy. It
compares polcmd, hassublinks, policy_name, roles, qual, and
with_check_qual, but not permissive, so two policies that are identical
in every other respect except their PERMISSIVE/RESTRICTIVE designation
are reported as equal.

equalPolicy() is used (via equalRSDesc()) by RelationRebuildRelation()
to decide whether an open relation's existing row-security descriptor
can be kept as-is across a relcache rebuild, or must be treated as
changed. ALTER POLICY has no way to flip a policy's
PERMISSIVE/RESTRICTIVE designation, but DROP POLICY followed by CREATE
POLICY of the same name, roles, command, and quals, differing only in AS
PERMISSIVE/RESTRICTIVE, hits this exactly: the stale descriptor would be
kept, leaving the relcache out of sync with how the policy should now
combine with others (PERMISSIVE policies are ORed together; RESTRICTIVE
policies are ANDed with the rest).

I wasn't able to construct a simple SQL reproduction:
RelationRebuildRelation() (and thus this comparison) is only reached
while the relation is still open (refcount > 0) at the moment its
invalidation is processed. Closing and reopening the relation between
statements -- the usual behavior -- sidesteps the bug, and a second
session can't hold the relation open across the DROP/CREATE either,
since both need AccessExclusiveLock. This looks analogous to
ab6d1cd26eb, which fixed the same kind of omission in this function for
the USING qual and likewise shipped without a test.

I don't know if the omission to check for identical PERMISSIVE or
RESTRICTIVE settings can lead to real problems, but I feel that it
violates the promise of the function. So I am +1 on this change.

Yours,
Laurenz Albe

On Fri, Jul 10, 2026 at 08:58:22AM +0200, Laurenz Albe wrote:

I don't know if the omission to check for identical PERMISSIVE or
RESTRICTIVE settings can lead to real problems, but I feel that it
violates the promise of the function. So I am +1 on this change.

That does not make sense here, two policies with a different
polpermissive are not the same, or am I missing something?

That just looks like a hole coming from 093129c9d9fc where the field
has been added. Here is the thread related to this commit, without
the relcache part being mentioned:
/messages/by-id/20160901063404.GY4028@tamriel.snowman.net
--
Michael

On Mon, 2026-07-13 at 11:06 +0900, Michael Paquier wrote:

On Fri, Jul 10, 2026 at 08:58:22AM +0200, Laurenz Albe wrote:

I don't know if the omission to check for identical PERMISSIVE or
RESTRICTIVE settings can lead to real problems, but I feel that it
violates the promise of the function.  So I am +1 on this change.

That does not make sense here, two policies with a different
polpermissive are not the same, or am I missing something?

I think we agree. I probably did not express myself well.
I am for fixing the function the way Andreas suggested.

I just wasn't sure if the omission was intentional or not;
your research says it was a mistake.

Yours,
Laurenz Albe

On Mon, Jul 13, 2026 at 09:49:09AM +0200, Laurenz Albe wrote:

I just wasn't sure if the omission was intentional or not;
your research says it was a mistake.

The CREATE/DROP requirement makes this issue less of a problem as it
means that a relcache invalidation would happen between the two
queries and we lock the objects, still missing the check was feeling
contrary to the API promise, so applied and backpatched down.
--
Michael