v2-0002-Correctly-set-userid-of-subquery-rel-s-child-rels.patch

application/octet-stream

Filename: v2-0002-Correctly-set-userid-of-subquery-rel-s-child-rels.patch
Type: application/octet-stream
Part: 1
Message: Re: ExecRTCheckPerms() and many prunable partitions (checkAsUser)

Patch

Format: format-patch
Series: patch v2-0002
Subject: Correctly set userid of subquery rel's child rels
File+
src/backend/optimizer/util/relnode.c 7 3
src/test/regress/expected/inherit.out 43 0
src/test/regress/sql/inherit.sql 28 0
From c9b0198b34774da0197d96a1791ff9e52d651708 Mon Sep 17 00:00:00 2001
From: amitlan <amitlangote09@gmail.com>
Date: Sun, 11 Dec 2022 17:57:17 +0900
Subject: [PATCH v2 2/2] Correctly set userid of subquery rel's child rels

For a subquery parent baserel's child relation, 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.

Expand the test case added in 553d2ec27 to cover both this change
and the case where an inheritance parent is accessed via a view such
that any permission checks on the parent are to be done as the view
owner.
---
 src/backend/optimizer/util/relnode.c  | 10 +++++--
 src/test/regress/expected/inherit.out | 43 +++++++++++++++++++++++++++
 src/test/regress/sql/inherit.sql      | 28 +++++++++++++++++
 3 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 7085cf3c41..8114427ecc 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -228,10 +228,14 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
 	{
 		/*
 		 * 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.
+		 * the tables mentioned in the query are assigned RTEPermissionInfos.
+		 * So, for child relations (otherrels), simply use the parent's value,
+		 * unless the parent is a subquery base rel.
 		 */
-		if (parent == NULL)
+		Assert(parent == NULL ||
+			   (parent->rtekind == RTE_RELATION ||
+				parent->rtekind == RTE_SUBQUERY));
+		if (parent == NULL || parent->rtekind == RTE_SUBQUERY)
 		{
 			RTEPermissionInfo *perminfo;
 
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 2d49e765de..46a4289db1 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2511,9 +2511,52 @@ explain (costs off)
                Filter: ("left"(c, 3) ~ 'a1$'::text)
 (6 rows)
 
+reset session authorization;
+-- 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;
+create schema regress_permtest_schema;
+grant all on schema regress_permtest_schema to regress_permtest_view_owner;
+grant select(a,c) on permtest_parent to regress_permtest_view_owner;
+set session authorization regress_permtest_view_owner;
+create view regress_permtest_schema.permtest_parent_view as
+	select a, c from permtest_parent;
+-- Like above, the 2nd arm of UNION ALL gets a hash join due to lack of access
+-- to the expression index's stats
+explain (costs off)
+  select p2.a, p1.c
+  from regress_permtest_schema.permtest_parent_view p1
+	inner join regress_permtest_schema.permtest_parent_view p2
+  on p1.a = p2.a and p1.c ~ 'a1$'
+  union all
+  select p2.a, p1.c
+  from regress_permtest_schema.permtest_parent_view p1
+	inner join regress_permtest_schema.permtest_parent_view p2
+  on p1.a = p2.a and left(p1.c, 3) ~ 'a1$';
+                             QUERY PLAN                              
+---------------------------------------------------------------------
+ Append
+   ->  Nested Loop
+         Join Filter: (permtest_parent.a = permtest_parent_1.a)
+         ->  Seq Scan on permtest_grandchild permtest_parent
+               Filter: (c ~ 'a1$'::text)
+         ->  Seq Scan on permtest_grandchild permtest_parent_1
+   ->  Hash Join
+         Hash Cond: (permtest_parent_3.a = permtest_parent_2.a)
+         ->  Seq Scan on permtest_grandchild permtest_parent_3
+         ->  Hash
+               ->  Seq Scan on permtest_grandchild permtest_parent_2
+                     Filter: ("left"(c, 3) ~ 'a1$'::text)
+(12 rows)
+
 reset session authorization;
 revoke all on permtest_parent from regress_no_child_access;
 drop role regress_no_child_access;
+revoke all on permtest_parent from regress_permtest_view_owner;
+drop view regress_permtest_schema.permtest_parent_view;
+drop schema regress_permtest_schema;
+drop role regress_permtest_view_owner;
 drop table permtest_parent;
 -- Verify that constraint errors across partition root / child are
 -- handled correctly (Bug #16293)
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 195aedb5ff..d2363768bc 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -907,8 +907,36 @@ 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;
+
+-- 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;
+create schema regress_permtest_schema;
+grant all on schema regress_permtest_schema to regress_permtest_view_owner;
+grant select(a,c) on permtest_parent to regress_permtest_view_owner;
+set session authorization regress_permtest_view_owner;
+create view regress_permtest_schema.permtest_parent_view as
+	select a, c from permtest_parent;
+-- Like above, the 2nd arm of UNION ALL gets a hash join due to lack of access
+-- to the expression index's stats
+explain (costs off)
+  select p2.a, p1.c
+  from regress_permtest_schema.permtest_parent_view p1
+	inner join regress_permtest_schema.permtest_parent_view p2
+  on p1.a = p2.a and p1.c ~ 'a1$'
+  union all
+  select p2.a, p1.c
+  from regress_permtest_schema.permtest_parent_view p1
+	inner join regress_permtest_schema.permtest_parent_view 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;
+revoke all on permtest_parent from regress_permtest_view_owner;
+drop view regress_permtest_schema.permtest_parent_view;
+drop schema regress_permtest_schema;
+drop role regress_permtest_view_owner;
 drop table permtest_parent;
 
 -- Verify that constraint errors across partition root / child are
-- 
2.35.3