From 2b1fc96fe35e0164425387ac5c5b875e2ccd2e05 Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 17 Jul 2019 18:00:15 +0900 Subject: [PATCH v6] 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 | 15 ++++++ src/backend/utils/adt/selfuncs.c | 98 +++++++++++++++++++++++++++++++++++ src/include/nodes/pathnodes.h | 4 ++ src/test/regress/expected/inherit.out | 38 ++++++++++++++ src/test/regress/sql/inherit.sql | 21 ++++++++ 6 files changed, 177 insertions(+) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index b0dcd02ff6..9856aaa1de 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..8a2053609d 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -263,6 +263,16 @@ 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; + 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 +293,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..611d1d2814 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -4616,6 +4616,27 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid, rte->securityQuals == NIL && (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) == ACLCHECK_OK); + + /* + * If the user doesn't have permissions to + * access an inheritance child relation, check + * the root parent's permissions instead, that + * is, of the table mentioned in the query. + */ + if (!vardata->acl_ok && + index->rel->relid != index->rel->inh_root_relid) + { + rte = planner_rt_fetch(index->rel->relid, + root); + Assert(rte->rtekind == RTE_RELATION); + userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); + + /* see the comments above */ + vardata->acl_ok = + rte->securityQuals == NIL && + (pg_class_aclcheck(rte->relid, userid, + ACL_SELECT) == ACLCHECK_OK); + } } else { @@ -4678,6 +4699,7 @@ examine_simple_variable(PlannerInfo *root, Var *var, if (HeapTupleIsValid(vardata->statsTuple)) { Oid userid; + RelOptInfo *rel; /* * Check if user has permission to read this column. We require @@ -4693,6 +4715,82 @@ examine_simple_variable(PlannerInfo *root, Var *var, ACL_SELECT) == ACLCHECK_OK) || (pg_attribute_aclcheck(rte->relid, var->varattno, userid, ACL_SELECT) == ACLCHECK_OK)); + + /* + * If the user doesn't have permissions to access an inheritance + * child relation or specifically this column, check the root + * parent's permissions instead, that is, of the table mentioned + * in the query. To do that we must find out which of the root + * parent's attribute it corresponds to. + */ + if (!vardata->acl_ok && var->varattno > 0 && + (rel = find_base_rel(root, var->varno))->inh_root_relid != var->varno) + { + AppendRelInfo *appinfo; + int varattno = var->varattno; + int parent_varattno; + bool found = false; + + 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) + { + ListCell *l; + + 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, we've got to live with having no access to this + * column's stats. + */ + if (!found) + return; + + rte = planner_rt_fetch(rel->inh_root_relid, root); + userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); + + vardata->acl_ok = + rte->securityQuals == NIL && + ((pg_class_aclcheck(rte->relid, userid, + ACL_SELECT) == ACLCHECK_OK) || + (pg_attribute_aclcheck(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