From 423416f41247d5e4fa2e99de411ffa3b5e09cc8e Mon Sep 17 00:00:00 2001
From: amitlan <amitlangote09@gmail.com>
Date: Wed, 18 Jan 2023 16:49:49 +0900
Subject: [PATCH v3] Correctly set userid of subquery rel's child rels

For a RTE_SUBQUERY parent baserel's child relation that itself is a
RTE_RELATION rel, build_simple_rel() should explicitly look up the
latter's RTEPermissionInfo instead of copying the parent rel's
userid, which would be 0 given that it's a subquery rel.
---
 src/backend/optimizer/util/relnode.c  | 18 ++++++++++---
 src/test/regress/expected/inherit.out | 39 +++++++++++++++++++++++++--
 src/test/regress/sql/inherit.sql      | 28 +++++++++++++++++--
 3 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index a70a16238a..6c4550b90f 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -233,12 +233,22 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
 	rel->serverid = InvalidOid;
 	if (rte->rtekind == RTE_RELATION)
 	{
+		Assert(parent == NULL ||
+			   parent->rtekind == RTE_RELATION ||
+			   parent->rtekind == RTE_SUBQUERY);
+
 		/*
-		 * Get the userid from the relation's RTEPermissionInfo, though only
-		 * the tables mentioned in query are assigned RTEPermissionInfos.
-		 * Child relations (otherrels) simply use the parent's value.
+		 * For any RELATION rte, we need a userid with which to check
+		 * permission access. Baserels simply use their own RTEPermissionInfo's
+		 * checkAsUser.
+		 *
+		 * For otherrels normally there's no RTEPermissionInfo, so we use the
+		 * parent's, which normally has one. The exceptional case is that the
+		 * parent is a subquery, in which case the otherrel will have its own.
 		 */
-		if (parent == NULL)
+		if (rel->reloptkind == RELOPT_BASEREL ||
+			(rel->reloptkind == RELOPT_OTHER_MEMBER_REL &&
+			 parent->rtekind == RTE_SUBQUERY))
 		{
 			RTEPermissionInfo *perminfo;
 
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 2d49e765de..143271cd62 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2512,9 +2512,44 @@ explain (costs off)
 (6 rows)
 
 reset session authorization;
-revoke all on permtest_parent from regress_no_child_access;
-drop role regress_no_child_access;
+-- Check with a view over permtest_parent and a UNION ALL over the view,
+-- especially that permtest_parent's permissions are checked with the role
+-- owning the view as permtest_parent's RTE's checkAsUser.
+create role regress_permtest_view_owner;
+revoke all on permtest_grandchild from regress_permtest_view_owner;
+grant select(a, c) on permtest_parent to regress_permtest_view_owner;
+create view permtest_parent_view as
+	select a, c from permtest_parent;
+alter view permtest_parent_view owner to regress_permtest_view_owner;
+-- Like above, the 2nd arm of UNION ALL gets a hash join due to lack of access
+-- to the expression index's stats
+revoke select on permtest_parent_view from regress_no_child_access;
+grant select(a,c) on permtest_parent_view to regress_no_child_access;
+set session authorization regress_no_child_access;
+explain (costs off)
+	select p1.a, p2.c from
+	(select * from permtest_parent_view
+	 union all
+	 select * from permtest_parent_view) p1
+	inner join permtest_parent p2 on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
+                             QUERY PLAN                              
+---------------------------------------------------------------------
+ Hash Join
+   Hash Cond: (p2.a = permtest_parent.a)
+   ->  Seq Scan on permtest_grandchild p2
+   ->  Hash
+         ->  Append
+               ->  Seq Scan on permtest_grandchild permtest_parent
+                     Filter: ("left"(c, 3) ~ 'a1$'::text)
+               ->  Seq Scan on permtest_grandchild permtest_parent_1
+                     Filter: ("left"(c, 3) ~ 'a1$'::text)
+(9 rows)
+
+reset session authorization;
+drop view permtest_parent_view;
 drop table permtest_parent;
+drop role regress_permtest_view_owner;
+drop role regress_no_child_access;
 -- Verify that constraint errors across partition root / child are
 -- handled correctly (Bug #16293)
 CREATE TABLE errtst_parent (
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 195aedb5ff..73b4a26f50 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -907,9 +907,33 @@ explain (costs off)
   select p2.a, p1.c from permtest_parent p1 inner join permtest_parent p2
   on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
 reset session authorization;
-revoke all on permtest_parent from regress_no_child_access;
-drop role regress_no_child_access;
+
+-- Check with a view over permtest_parent and a UNION ALL over the view,
+-- especially that permtest_parent's permissions are checked with the role
+-- owning the view as permtest_parent's RTE's checkAsUser.
+create role regress_permtest_view_owner;
+revoke all on permtest_grandchild from regress_permtest_view_owner;
+grant select(a, c) on permtest_parent to regress_permtest_view_owner;
+create view permtest_parent_view as
+	select a, c from permtest_parent;
+alter view permtest_parent_view owner to regress_permtest_view_owner;
+-- Like above, the 2nd arm of UNION ALL gets a hash join due to lack of access
+-- to the expression index's stats
+revoke select on permtest_parent_view from regress_no_child_access;
+grant select(a,c) on permtest_parent_view to regress_no_child_access;
+set session authorization regress_no_child_access;
+explain (costs off)
+	select p1.a, p2.c from
+	(select * from permtest_parent_view
+	 union all
+	 select * from permtest_parent_view) p1
+	inner join permtest_parent p2 on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
+
+reset session authorization;
+drop view permtest_parent_view;
 drop table permtest_parent;
+drop role regress_permtest_view_owner;
+drop role regress_no_child_access;
 
 -- Verify that constraint errors across partition root / child are
 -- handled correctly (Bug #16293)
-- 
2.30.2

