pgsql: Prevent self-join elimination when RTEs' checkAsUser fields diff - Mailing list pgsql-committers

From Tom Lane
Subject pgsql: Prevent self-join elimination when RTEs' checkAsUser fields diff
Date
Msg-id E1xBZUN-00000001lNz-2tU2@gemulon.postgresql.org
Whole thread
List pgsql-committers
Prevent self-join elimination when RTEs' checkAsUser fields differ.

SJE didn't consider the possibility that two RTEs referencing the
same table have different securityQuals, and might choose to merge
them anyway, thereby possibly losing quals that need to be enforced.
This is a regression introduced by 2ebf25e7d, since before that we
didn't re-generate baserestrictinfo lists from the RTEs' securityQuals
after performing SJE.

To defend against this, only consider SJE between RTEs with the same
checkAsUser values.  That solves the problem because the set of
applicable RLS policies depends only on the role that is considered
to be accessing the table, so that the securityQuals must be equal
if the checkAsUser values are.  It might also keep us from creating
similar bugs if we ever invent other features that depend on the
accessing role.  And it's a lot cheaper than comparing securityQuals
trees themselves would be, not least because we can't sort them so
the preliminary sort step wouldn't help.

(The first proposed solution was to not perform SJE at all on RTEs
with nonempty securityQuals, but that seems rather sad, especially
since such cases worked before 2ebf25e7d.  Making the restriction
depend on checkAsUser seems much less likely to interfere with SJE
unnecessarily, since in most cases that'll be the same for all
potentially-mergeable RTEs.)

The added test cases show that SJE is rejected when necessary, and
also demonstrate two quirks of this implementation.  One is that if
a removed security qual includes an InitPlan, we'll still attach the
now-unused InitPlan to the plan.  That's because SS_process_sublinks
runs before self-join elimination, so the extra InitPlan has already
been made.  The other quirk is that we won't merge RTEs having zero
and nonzero checkAsUser fields, even if the calling user matches the
nonzero checkAsUser value so that there is no difference for this
user.  That's intentional so that SJE doesn't require having to mark
the plan as caller-dependent.

Reported-by: Yonghwa Lee <underdog@theori.io>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/20260926174457.13.noahmisch@microsoft.com
Backpatch-through: 18

Branch
------
REL_19_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/78d07020bd0593e4916cbf7e535061e63f82b767

Modified Files
--------------
src/backend/optimizer/plan/analyzejoins.c | 47 ++++++++++++++++++++++---------
src/test/regress/expected/rowsecurity.out | 47 ++++++++++++++++++++++++++++++-
src/test/regress/sql/rowsecurity.sql      | 17 ++++++++++-
3 files changed, 95 insertions(+), 16 deletions(-)


pgsql-committers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: pgsql: pg_resetwal: Fix handling of commit timestamp XIDs