From 4fdeda8933a34406f5fe16388c74cfbf70da469d Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 21 Nov 2019 11:24:38 +0900 Subject: [PATCH v7] 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 | 102 ++++++++++++++++++++++++++++++++++ src/include/nodes/pathnodes.h | 6 ++ src/test/regress/expected/inherit.out | 59 ++++++++++++++++++++ src/test/regress/sql/inherit.sql | 24 ++++++++ 6 files changed, 207 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 03e02423b2..611ba0e491 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 024f325eb0..fbff732f9b 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -4612,6 +4612,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->inh_root_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 { @@ -4674,6 +4695,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 @@ -4689,6 +4711,86 @@ 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 attribute, 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 the child relation's attribute corresponds + * to. + */ + if (!vardata->acl_ok && var->varattno > 0 && + (rel = find_base_rel(root, var->varno))->inh_root_relid != var->varno) + { + AppendRelInfo *appinfo; + Index varno = var->varno; + 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; + varno = appinfo->parent_relid; + + /* 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; + + Assert(varno == rel->inh_root_relid); + 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..6649fb9535 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -489,6 +489,11 @@ 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 that are inheritance child tables, this + * is the RT index of inheritance root table that is mentioned in + * the query from which this relation originated; for baserels + * and otherrels which are themselves inheritance root tables, + * 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 +672,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..0943ba42e5 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -2335,3 +2335,62 @@ 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); +create index on permtest_parent (left(c, 3)); +insert into permtest_parent select 1, 'a', left(md5(i::text), 5) from generate_series(0, 100) 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 ~ 'a1$'; + QUERY PLAN +------------------------------------------ + Nested Loop + Join Filter: (p1.a = p2.a) + -> Seq Scan on permtest_grandchild p1 + Filter: (c ~ 'a1$'::text) + -> Seq Scan on permtest_grandchild p2 +(5 rows) + +explain (costs off) select * from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and left(p1.c, 3) ~ 'a1$'; + QUERY PLAN +---------------------------------------------- + Nested Loop + Join Filter: (p1.a = p2.a) + -> Seq Scan on permtest_grandchild p1 + Filter: ("left"(c, 3) ~ 'a1$'::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 ~ 'a1$'; + QUERY PLAN +------------------------------------------ + Nested Loop + Join Filter: (p1.a = p2.a) + -> Seq Scan on permtest_grandchild p1 + Filter: (c ~ 'a1$'::text) + -> Seq Scan on permtest_grandchild p2 +(5 rows) + +explain (costs off) select * from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and left(p1.c, 3) ~ 'a1$'; + QUERY PLAN +---------------------------------------------- + Nested Loop + Join Filter: (p1.a = p2.a) + -> Seq Scan on permtest_grandchild p1 + Filter: ("left"(c, 3) ~ 'a1$'::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..ca21bbb757 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -845,3 +845,27 @@ 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); +create index on permtest_parent (left(c, 3)); +insert into permtest_parent select 1, 'a', left(md5(i::text), 5) from generate_series(0, 100) 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 ~ 'a1$'; +explain (costs off) select * from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and left(p1.c, 3) ~ 'a1$'; +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 ~ 'a1$'; +explain (costs off) select * 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; +drop table permtest_parent; -- 2.11.0