On 2018/11/01 20:34, Dilip Kumar wrote:
> On Mon, Oct 29, 2018 at 2:53 PM Amit Langote wrote:
>> Anyway, why don't we just use the child table's AppendRelInfo to get the
>> parent's version of varattno instead of creating a new function? It can
>> be done as shown in the attached revised version of the portion of the
>> patch changing selfuncs.c. Please take a look.
>
> +1
Okay, here are two patches:
0001 adds a new RelOptInfo member inh_root_parent that's set for
inheritance child otherrels and contains the RT index of the inheritance
parent table mentioned in the query from which they originated.
0002 is your patch that modifies examine_variable, etc. to use the
permissions granted on parent before reading stats on otherrel inheritance
child tables. I've added your name as the author in the 2nd patch.
Thanks,
Amit
From 04c2c1c2876995322309dc791866c617e2c4a539 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 26 Oct 2018 16:45:59 +0900
Subject: [PATCH 1/2] Store inheritance root parent index in otherrel's
RelOptInfo
Although it's set by build_simple_rel, it's not being used by any
code yet.
---
src/backend/nodes/outfuncs.c | 1 +
src/backend/optimizer/util/relnode.c | 14 ++++++++++++++
src/include/nodes/relation.h | 4 ++++
3 files changed, 19 insertions(+)
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 69731ccdea..53a657c0ae 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2371,6 +2371,7 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node)
WRITE_BOOL_FIELD(consider_partitionwise_join);
WRITE_BITMAPSET_FIELD(top_parent_relids);
WRITE_NODE_FIELD(partitioned_child_rels);
+ WRITE_UINT_FIELD(inh_root_parent);
}
static void
diff --git a/src/backend/optimizer/util/relnode.c
b/src/backend/optimizer/util/relnode.c
index 39f5729b91..29ba19349f 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -215,9 +215,23 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo
*parent)
rel->top_parent_relids = parent->top_parent_relids;
else
rel->top_parent_relids = bms_copy(parent->relids);
+
+ /*
+ * For inheritance child relations, we also set inh_root_parent.
+ * Note that 'parent' might itself be a child (a sub-partitioned
+ * partition), in which case we simply use its value of
+ * inh_root_parent.
+ */
+ if (parent->rtekind == RTE_RELATION)
+ rel->inh_root_parent = parent->inh_root_parent > 0 ?
+
parent->inh_root_parent :
+
parent->relid;
}
else
+ {
rel->top_parent_relids = NULL;
+ rel->inh_root_parent = 0;
+ }
/* Check type of rtable entry */
switch (rte->rtekind)
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 88d37236f7..bbc1408d05 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -703,6 +703,10 @@ typedef struct RelOptInfo
List **partexprs; /* Non-nullable partition key
expressions. */
List **nullable_partexprs; /* Nullable partition key expressions.
*/
List *partitioned_child_rels; /* List of RT indexes. */
+
+ Index inh_root_parent; /* For otherrels, this is the
RT index of
+ *
inheritance table mentioned in the query
+ * from
which this relation originated */
} RelOptInfo;
/*
--
2.11.0
From e7ce03017391f7472a9d30b039447d66f1d956c6 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Fri, 2 Nov 2018 16:48:25 +0900
Subject: [PATCH 2/2] Use permissions granted on parent to read stats on
otherrel children
Author: Dilip Kumar
Reviewed by: Amit Langote
---
src/backend/utils/adt/selfuncs.c | 58 +++++++++++++++++++++++++++++++++++++---
1 file changed, 55 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index e0ece74bb9..698a4eca63 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4924,8 +4924,18 @@ examine_variable(PlannerInfo *root, Node *node, int
varRelid,
{
/* Get index's
table for permission check */
RangeTblEntry
*rte;
+ RelOptInfo
*rel = index->rel;
+
+ /*
+ * For a child
table, check permissions using
+ * parent's RTE.
+ */
+ if
(rel->inh_root_parent)
+ rte =
planner_rt_fetch(rel->inh_root_parent,
+
root);
+ else
+ rte =
planner_rt_fetch(rel->relid, root);
- rte =
planner_rt_fetch(index->rel->relid, root);
Assert(rte->rtekind == RTE_RELATION);
/*
@@ -4998,11 +5008,53 @@ examine_simple_variable(PlannerInfo *root, Var *var,
if (HeapTupleIsValid(vardata->statsTuple))
{
+ RelOptInfo *rel;
+ Oid relid = rte->relid;
+ int varattno = var->varattno;
+
+ rel = root->simple_rel_array[var->varno];
+
+ /*
+ * For an inheritance child table, check permissions
using the
+ * root parent's RTE and columns.
+ */
+ if (rel->inh_root_parent)
+ {
+ relid = planner_rt_fetch(rel->inh_root_parent,
root)->relid;
+
+ /*
+ * Must find out the column's attribute number
per the root
+ * parent's schema.
+ */
+ if (varattno > 0)
+ {
+ AppendRelInfo *appinfo =
root->append_rel_array[rel->relid];
+ ListCell *l;
+ bool found = false;
+
+ Assert(appinfo != NULL);
+ varattno = 1;
+ foreach(l, appinfo->translated_vars)
+ {
+ if (equal(var, lfirst(l)))
+ {
+ found = true;
+ break;
+ }
+ varattno++;
+ }
+ /*
+ * The query can only select columns
present in the parent
+ * table, so we must've found one in
translated_vars.
+ */
+ Assert(found);
+ }
+ }
/* check if user has permission to read this column */
vardata->acl_ok =
- (pg_class_aclcheck(rte->relid, GetUserId(),
+ (pg_class_aclcheck(relid, GetUserId(),
ACL_SELECT)
== ACLCHECK_OK) ||
- (pg_attribute_aclcheck(rte->relid,
var->varattno, GetUserId(),
+ (pg_attribute_aclcheck(relid, varattno,
GetUserId(),
ACL_SELECT) == ACLCHECK_OK);
}
else
--
2.11.0
