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

src/backend/optimizer/plan/analyzejoins.c

@@ -36,6 +36,7 @@
 #include "optimizer/planmain.h"
 #include "optimizer/prep.h"
 #include "optimizer/restrictinfo.h"
+#include "parser/parse_relation.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/lsyscache.h"
 
@@ -55,18 +56,21 @@ typedef struct DistinctColInfo
 
 /*
  * Utility structure.  A sorting procedure is needed to simplify the search
- * of SJE-candidate baserels referencing the same database relation.  Having
- * collected all baserels from the query jointree, the planner sorts them
- * according to the reloid value, groups them with the next pass and attempts
- * to remove self-joins.
- *
- * Preliminary sorting prevents quadratic behavior that can be harmful in the
- * case of numerous joins.
+ * for SJE-candidate baserels, which must reference the same database relation
+ * with the same reader permissions (checkAsUser value).  We require the
+ * checkAsUser fields to match to ensure that merged RTEs carry the same
+ * securityQuals; in future this rule might keep us out of trouble with other
+ * role-based features, too.  Having collected all baserels from the jointree,
+ * remove_self_joins_recurse sorts them according to their reloid and useroid
+ * values, groups them in another pass and attempts to remove self-joins
+ * within each group.  This preliminary sorting prevents quadratic behavior
+ * in the case of numerous joins.
  */
 typedef struct
 {
 	int			relid;
 	Oid			reloid;
+	Oid			useroid;
 } SelfJoinCandidate;
 
 bool		enable_self_join_elimination;
@@ -2050,17 +2054,27 @@ remove_self_joins_recurse(PlannerInfo *root, List *joinlist)
 		return removed;			/* ... but don't fail to report sub-removals */
 
 	/*
-	 * In order to find relations with the same oid we first build an array of
-	 * candidates and then sort it by oid.
+	 * In order to find relations with the same reloid/useroid we first build
+	 * an array of candidates and then sort it by those oids.
 	 */
-	candidates = (SelfJoinCandidate *) palloc(sizeof(SelfJoinCandidate) *
-											  numRels);
+	candidates = palloc_array(SelfJoinCandidate, numRels);
 	i = -1;
 	j = 0;
 	while ((i = bms_next_member(relids, i)) >= 0)
 	{
+		RangeTblEntry *rte = root->simple_rte_array[i];
+
 		candidates[j].relid = i;
-		candidates[j].reloid = root->simple_rte_array[i]->relid;
+		candidates[j].reloid = rte->relid;
+		if (rte->perminfoindex != 0)
+		{
+			RTEPermissionInfo *perminfo;
+
+			perminfo = getRTEPermissionInfo(root->parse->rteperminfos, rte);
+			candidates[j].useroid = perminfo->checkAsUser;
+		}
+		else
+			candidates[j].useroid = InvalidOid;
 		j++;
 	}
 
@@ -2068,7 +2082,7 @@ remove_self_joins_recurse(PlannerInfo *root, List *joinlist)
 		  self_join_candidates_cmp);
 
 	/*
-	 * Iteratively form a group of relation indexes with the same oid and
+	 * Iteratively form a group of relation indexes with the same oids and
 	 * launch the routine that detects self-joins in this group.
 	 *
 	 * We remove considered relations from relids as we scan, so that that set
@@ -2077,11 +2091,13 @@ remove_self_joins_recurse(PlannerInfo *root, List *joinlist)
 	i = 0;
 	for (j = 1; j <= numRels; j++)
 	{
-		if (j == numRels || candidates[j].reloid != candidates[i].reloid)
+		if (j == numRels ||
+			candidates[j].reloid != candidates[i].reloid ||
+			candidates[j].useroid != candidates[i].useroid)
 		{
 			if (j - i >= 2)
 			{
-				/* Create a group of relation indexes with the same oid */
+				/* Create a group of relation indexes with the same oids */
 				Relids		group = NULL;
 
 				while (i < j)
@@ -2113,7 +2129,7 @@ remove_self_joins_recurse(PlannerInfo *root, List *joinlist)
 }
 
 /*
- * Compare self-join candidates by their oids.
+ * Compare self-join candidates by their reloid and then useroid.
  */
 static int
 self_join_candidates_cmp(const void *a, const void *b)
@@ -2123,6 +2139,8 @@ self_join_candidates_cmp(const void *a, const void *b)
 
 	if (ca->reloid != cb->reloid)
 		return (ca->reloid < cb->reloid ? -1 : 1);
+	else if (ca->useroid != cb->useroid)
+		return (ca->useroid < cb->useroid ? -1 : 1);
 	else
 		return 0;
 }

src/test/regress/expected/rowsecurity.out

@@ -4493,7 +4493,7 @@ DROP USER regress_rls_dob_role2;
 -- Bug #15708: view + table with RLS should check policies as view owner
 CREATE TABLE ref_tbl (a int);
 INSERT INTO ref_tbl VALUES (1);
-CREATE TABLE rls_tbl (a int);
+CREATE TABLE rls_tbl (a int PRIMARY KEY);
 INSERT INTO rls_tbl VALUES (10);
 ALTER TABLE rls_tbl ENABLE ROW LEVEL SECURITY;
 CREATE POLICY p1 ON rls_tbl USING (EXISTS (SELECT 1 FROM ref_tbl));
@@ -4513,6 +4513,51 @@ SELECT * FROM rls_view; -- OK
  10
 (1 row)
 
+EXPLAIN (COSTS OFF) -- check that RLS enforcement is actually happening
+SELECT * FROM rls_view;
+         QUERY PLAN          
+-----------------------------
+ Seq Scan on rls_tbl
+   Filter: (InitPlan 1).col1
+   InitPlan 1
+     ->  Seq Scan on ref_tbl
+(4 rows)
+
+-- Use the same view+table to test interaction of SJE with RLS
+SET enable_self_join_elimination = on;
+-- We can do SJE here, although presently an unused InitPlan survives
+EXPLAIN (COSTS OFF)
+SELECT * FROM rls_view r1, rls_view r2 WHERE r1.a = r2.a;
+              QUERY PLAN               
+---------------------------------------
+ Seq Scan on rls_tbl
+   Filter: (InitPlan 2).col1
+   InitPlan 1
+     ->  Seq Scan on ref_tbl
+   InitPlan 2
+     ->  Seq Scan on ref_tbl ref_tbl_1
+(6 rows)
+
+SET SESSION AUTHORIZATION regress_rls_bob;
+-- No SJE here, because the two rls_tbl RTEs have different checkAsUser values
+EXPLAIN (COSTS OFF)
+SELECT * FROM rls_tbl r1, rls_view r2 WHERE r1.a = r2.a;
+               QUERY PLAN                
+-----------------------------------------
+ Hash Join
+   Hash Cond: (r1.a = rls_tbl.a)
+   InitPlan 1
+     ->  Seq Scan on ref_tbl
+   InitPlan 2
+     ->  Seq Scan on ref_tbl ref_tbl_1
+   ->  Seq Scan on rls_tbl r1
+         Filter: (InitPlan 1).col1
+   ->  Hash
+         ->  Seq Scan on rls_tbl
+               Filter: (InitPlan 2).col1
+(11 rows)
+
+RESET enable_self_join_elimination;
 RESET SESSION AUTHORIZATION;
 DROP VIEW rls_view;
 DROP TABLE rls_tbl;

src/test/regress/sql/rowsecurity.sql

@@ -2175,7 +2175,7 @@ DROP USER regress_rls_dob_role2;
 CREATE TABLE ref_tbl (a int);
 INSERT INTO ref_tbl VALUES (1);
 
-CREATE TABLE rls_tbl (a int);
+CREATE TABLE rls_tbl (a int PRIMARY KEY);
 INSERT INTO rls_tbl VALUES (10);
 ALTER TABLE rls_tbl ENABLE ROW LEVEL SECURITY;
 CREATE POLICY p1 ON rls_tbl USING (EXISTS (SELECT 1 FROM ref_tbl));
@@ -2188,9 +2188,24 @@ ALTER VIEW rls_view OWNER TO regress_rls_bob;
 GRANT SELECT ON rls_view TO regress_rls_alice;
 
 SET SESSION AUTHORIZATION regress_rls_alice;
+
 SELECT * FROM ref_tbl; -- Permission denied
 SELECT * FROM rls_tbl; -- Permission denied
 SELECT * FROM rls_view; -- OK
+EXPLAIN (COSTS OFF) -- check that RLS enforcement is actually happening
+SELECT * FROM rls_view;
+
+-- Use the same view+table to test interaction of SJE with RLS
+SET enable_self_join_elimination = on;
+-- We can do SJE here, although presently an unused InitPlan survives
+EXPLAIN (COSTS OFF)
+SELECT * FROM rls_view r1, rls_view r2 WHERE r1.a = r2.a;
+SET SESSION AUTHORIZATION regress_rls_bob;
+-- No SJE here, because the two rls_tbl RTEs have different checkAsUser values
+EXPLAIN (COSTS OFF)
+SELECT * FROM rls_tbl r1, rls_view r2 WHERE r1.a = r2.a;
+
+RESET enable_self_join_elimination;
 RESET SESSION AUTHORIZATION;
 
 DROP VIEW rls_view;