From 6e7525b6d7b397ce6caf546118fc9432b3f1bb63 Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 17 Jul 2019 18:00:15 +0900 Subject: [PATCH v4] 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 | 87 +++++++++++++++++++++++++++++++++-- src/include/nodes/pathnodes.h | 4 ++ src/test/regress/expected/inherit.out | 38 +++++++++++++++ src/test/regress/sql/inherit.sql | 21 +++++++++ 6 files changed, 161 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..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..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