v3-0001-Use-root-parent-s-permissions-when-reading-child-.patch

From 7a338503728be5faec9c89aaf434e3da18b057d8 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 17 Jul 2019 18:00:15 +0900
Subject: [PATCH v3] Use root parent's permissions when reading child table
 stats

Author: Dilip Kumar, Amit Langote
---
 src/backend/nodes/outfuncs.c          |  1 +
 src/backend/optimizer/util/relnode.c  | 17 +++++++
 src/backend/utils/adt/selfuncs.c      | 87 +++++++++++++++++++++++++++++++++--
 src/include/nodes/pathnodes.h         |  4 ++
 src/test/regress/expected/inherit.out | 38 +++++++++++++++
 src/test/regress/sql/inherit.sql      | 21 +++++++++
 6 files changed, 163 insertions(+), 5 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index e6ce8e2110..7710f3d21d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2255,6 +2255,7 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node)
 	WRITE_BITMAPSET_FIELD(direct_lateral_relids);
 	WRITE_BITMAPSET_FIELD(lateral_relids);
 	WRITE_UINT_FIELD(relid);
+	WRITE_UINT_FIELD(inh_root_relid);
 	WRITE_OID_FIELD(reltablespace);
 	WRITE_ENUM_FIELD(rtekind, RTEKind);
 	WRITE_INT_FIELD(min_attr);
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 85415381fb..f17afcfaa1 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -263,6 +263,18 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
 			rel->top_parent_relids = bms_copy(parent->relids);
 
 		/*
+		 * For inheritance child relations, we also need to remember
+		 * the root parent.
+		 */
+		if (parent->rtekind == RTE_RELATION)
+			rel->inh_root_relid = parent->inh_root_relid > 0 ?
+										parent->inh_root_relid :
+										parent->relid;
+		else
+			/* Child relation of flattened UNION ALL subquery. */
+			rel->inh_root_relid = relid;
+
+		/*
 		 * Also propagate lateral-reference information from appendrel parent
 		 * rels to their child rels.  We intentionally give each child rel the
 		 * same minimum parameterization, even though it's quite possible that
@@ -283,6 +295,11 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
 	else
 	{
 		rel->top_parent_relids = NULL;
+		/*
+		 * For baserels, we set ourselves as the root, because it simplifies
+		 * code elsewhere.
+		 */
+		rel->inh_root_relid = relid;
 		rel->direct_lateral_relids = NULL;
 		rel->lateral_relids = NULL;
 		rel->lateral_referencers = NULL;
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 17101298fb..6e71bb2c2b 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4594,7 +4594,12 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
 								RangeTblEntry *rte;
 								Oid			userid;
 
-								rte = planner_rt_fetch(index->rel->relid, root);
+								/*
+								 * Check permissions using the root parent's
+								 * RTE.
+								 */
+								rte = planner_rt_fetch(index->rel->inh_root_relid,
+													   root);
 								Assert(rte->rtekind == RTE_RELATION);
 
 								/*
@@ -4678,6 +4683,76 @@ examine_simple_variable(PlannerInfo *root, Var *var,
 		if (HeapTupleIsValid(vardata->statsTuple))
 		{
 			Oid			userid;
+			RelOptInfo *rel = find_base_rel(root, var->varno);
+			AppendRelInfo *appinfo = NULL;
+			RangeTblEntry *root_rte;
+			int			varattno = var->varattno;
+
+			/* Check permissions using the root parent's RTE. */
+			root_rte = planner_rt_fetch(rel->inh_root_relid, root);
+
+			/*
+			 * If the Var comes from a child relation, we must find out which
+			 * of the root parent's attribute it corresponds to and check
+			 * permissions using the parent attribute's permissions instead
+			 * of the child attribute's.
+			 */
+			if (varattno > 0 && rel->inh_root_relid != var->varno)
+			{
+				ListCell   *l;
+				int			parent_varattno;
+				bool		found;
+
+				appinfo = root->append_rel_array[var->varno];
+				Assert(appinfo != NULL);
+
+				/*
+				 * Partitions are mapped to their immediate parent, not the
+				 * root parent, so must be ready to walk up multiple
+				 * AppendRelInfos.  Beware not to translate the attribute
+				 * number to a flattened UNION ALL subquery parent.
+				 */
+				while (appinfo &&
+					   planner_rt_fetch(appinfo->parent_relid,
+										root)->rtekind == RTE_RELATION)
+				{
+					parent_varattno = 1;
+					found = false;
+					foreach(l, appinfo->translated_vars)
+					{
+						Var	   *childvar = lfirst_node(Var, l);
+
+						/* Ignore dropped attributes of the parent. */
+						if (childvar == NULL)
+						{
+							parent_varattno++;
+							continue;
+						}
+
+						if (varattno == childvar->varattno)
+						{
+							found = true;
+							break;
+						}
+						parent_varattno++;
+					}
+
+					varattno = parent_varattno;
+
+					/* If the parent is itself a child, continue up. */
+					appinfo = root->append_rel_array[appinfo->parent_relid];
+				}
+
+				/*
+				 * In rare cases, the Var may be local to the child table, in
+				 * which case fall back to checking the child's permissions.
+				 */
+				if (!found)
+				{
+					varattno = var->varattno;
+					root_rte = rte;
+				}
+			}
 
 			/*
 			 * Check if user has permission to read this column.  We require
@@ -4685,13 +4760,15 @@ examine_simple_variable(PlannerInfo *root, Var *var,
 			 * from security barrier views or RLS policies.  Use checkAsUser
 			 * if it's set, in case we're accessing the table via a view.
 			 */
-			userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+			userid = root_rte->checkAsUser ?
+							root_rte->checkAsUser :
+							GetUserId();
 
 			vardata->acl_ok =
-				rte->securityQuals == NIL &&
-				((pg_class_aclcheck(rte->relid, userid,
+				root_rte->securityQuals == NIL &&
+				((pg_class_aclcheck(root_rte->relid, userid,
 									ACL_SELECT) == ACLCHECK_OK) ||
-				 (pg_attribute_aclcheck(rte->relid, var->varattno, userid,
+				 (pg_attribute_aclcheck(root_rte->relid, varattno, userid,
 										ACL_SELECT) == ACLCHECK_OK));
 		}
 		else
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 23a06d718e..1d48c63c75 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -489,6 +489,9 @@ typedef struct PartitionSchemeData *PartitionScheme;
  *
  *		relid - RTE index (this is redundant with the relids field, but
  *				is provided for convenience of access)
+ *		inh_root_relid - For otherrels, this is the RT index of inheritance
+ *				root table that is mentioned in the query from which this
+ *				relation originated.  For baserels, it's same as relid.
  *		rtekind - copy of RTE's rtekind field
  *		min_attr, max_attr - range of valid AttrNumbers for rel
  *		attr_needed - array of bitmapsets indicating the highest joinrel
@@ -667,6 +670,7 @@ typedef struct RelOptInfo
 
 	/* information about a base rel (not set for join rels!) */
 	Index		relid;
+	Index		inh_root_relid;
 	Oid			reltablespace;	/* containing tablespace */
 	RTEKind		rtekind;		/* RELATION, SUBQUERY, FUNCTION, etc */
 	AttrNumber	min_attr;		/* smallest attrno of rel (often <0) */
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 44d51ed711..8e0d9e4658 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2335,3 +2335,41 @@ explain (costs off) select * from range_parted order by a desc,b desc,c desc;
 (3 rows)
 
 drop table range_parted;
+-- Test for checking that lack of access permissions for a child table and
+-- hence its statistics doesn't affect plan shapes when the query is on the
+-- parent table
+create table permtest_parent (a int, b text, c text) partition by list (a);
+create table permtest_child (b text, a int, c text) partition by list (b);
+create table permtest_grandchild (c text, b text, a int);
+alter table permtest_child attach partition permtest_grandchild for values in ('a');
+alter table permtest_parent attach partition permtest_child for values in (1);
+insert into permtest_parent select 1, 'a', i || 'x' || i * 10 from generate_series(1, 1000) i;
+analyze permtest_parent;
+explain (costs off) select * from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and p1.c like '4x5%';
+                QUERY PLAN                
+------------------------------------------
+ Nested Loop
+   Join Filter: (p1.a = p2.a)
+   ->  Seq Scan on permtest_grandchild p1
+         Filter: (c ~~ '4x5%'::text)
+   ->  Seq Scan on permtest_grandchild p2
+(5 rows)
+
+create role regress_no_child_access;
+revoke all on permtest_grandchild from regress_no_child_access;
+grant all on permtest_parent to regress_no_child_access;
+set session authorization regress_no_child_access;
+explain (costs off) select * from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and p1.c like '4x5%';
+                QUERY PLAN                
+------------------------------------------
+ Nested Loop
+   Join Filter: (p1.a = p2.a)
+   ->  Seq Scan on permtest_grandchild p1
+         Filter: (c ~~ '4x5%'::text)
+   ->  Seq Scan on permtest_grandchild p2
+(5 rows)
+
+reset session authorization;
+revoke all on permtest_parent from regress_no_child_access;
+drop role regress_no_child_access;
+drop table permtest_parent;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 3af1bf30a7..1b1fddc47f 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -845,3 +845,24 @@ explain (costs off) select * from range_parted order by a,b,c;
 explain (costs off) select * from range_parted order by a desc,b desc,c desc;
 
 drop table range_parted;
+
+-- Test for checking that lack of access permissions for a child table and
+-- hence its statistics doesn't affect plan shapes when the query is on the
+-- parent table
+create table permtest_parent (a int, b text, c text) partition by list (a);
+create table permtest_child (b text, a int, c text) partition by list (b);
+create table permtest_grandchild (c text, b text, a int);
+alter table permtest_child attach partition permtest_grandchild for values in ('a');
+alter table permtest_parent attach partition permtest_child for values in (1);
+insert into permtest_parent select 1, 'a', i || 'x' || i * 10 from generate_series(1, 1000) i;
+analyze permtest_parent;
+explain (costs off) select * from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and p1.c like '4x5%';
+create role regress_no_child_access;
+revoke all on permtest_grandchild from regress_no_child_access;
+grant all on permtest_parent to regress_no_child_access;
+set session authorization regress_no_child_access;
+explain (costs off) select * from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and p1.c like '4x5%';
+reset session authorization;
+revoke all on permtest_parent from regress_no_child_access;
+drop role regress_no_child_access;
+drop table permtest_parent;
-- 
2.11.0