From c9b0198b34774da0197d96a1791ff9e52d651708 Mon Sep 17 00:00:00 2001 From: amitlan Date: Sun, 11 Dec 2022 17:57:17 +0900 Subject: [PATCH v2 2/2] Correctly set userid of subquery rel's child rels For a subquery parent baserel's child relation, build_simple_rel() should explicitly look up the latter's RTEPermissionInfo instead of copying the parent rel's userid, which would be 0 given that it's a subquery rel. Expand the test case added in 553d2ec27 to cover both this change and the case where an inheritance parent is accessed via a view such that any permission checks on the parent are to be done as the view owner. --- src/backend/optimizer/util/relnode.c | 10 +++++-- src/test/regress/expected/inherit.out | 43 +++++++++++++++++++++++++++ src/test/regress/sql/inherit.sql | 28 +++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 7085cf3c41..8114427ecc 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -228,10 +228,14 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent) { /* * Get the userid from the relation's RTEPermissionInfo, though only - * the tables mentioned in query are assigned RTEPermissionInfos. - * Child relations (otherrels) simply use the parent's value. + * the tables mentioned in the query are assigned RTEPermissionInfos. + * So, for child relations (otherrels), simply use the parent's value, + * unless the parent is a subquery base rel. */ - if (parent == NULL) + Assert(parent == NULL || + (parent->rtekind == RTE_RELATION || + parent->rtekind == RTE_SUBQUERY)); + if (parent == NULL || parent->rtekind == RTE_SUBQUERY) { RTEPermissionInfo *perminfo; diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out index 2d49e765de..46a4289db1 100644 --- a/src/test/regress/expected/inherit.out +++ b/src/test/regress/expected/inherit.out @@ -2511,9 +2511,52 @@ explain (costs off) Filter: ("left"(c, 3) ~ 'a1$'::text) (6 rows) +reset session authorization; +-- Check with a view over permtest_parent and a UNION ALL over the view, +-- especially that permtest_parent's permissions are checked with the role +-- owning the view as permtest_parent's RTE's checkAsUser. +create role regress_permtest_view_owner; +create schema regress_permtest_schema; +grant all on schema regress_permtest_schema to regress_permtest_view_owner; +grant select(a,c) on permtest_parent to regress_permtest_view_owner; +set session authorization regress_permtest_view_owner; +create view regress_permtest_schema.permtest_parent_view as + select a, c from permtest_parent; +-- Like above, the 2nd arm of UNION ALL gets a hash join due to lack of access +-- to the expression index's stats +explain (costs off) + select p2.a, p1.c + from regress_permtest_schema.permtest_parent_view p1 + inner join regress_permtest_schema.permtest_parent_view p2 + on p1.a = p2.a and p1.c ~ 'a1$' + union all + select p2.a, p1.c + from regress_permtest_schema.permtest_parent_view p1 + inner join regress_permtest_schema.permtest_parent_view p2 + on p1.a = p2.a and left(p1.c, 3) ~ 'a1$'; + QUERY PLAN +--------------------------------------------------------------------- + Append + -> Nested Loop + Join Filter: (permtest_parent.a = permtest_parent_1.a) + -> Seq Scan on permtest_grandchild permtest_parent + Filter: (c ~ 'a1$'::text) + -> Seq Scan on permtest_grandchild permtest_parent_1 + -> Hash Join + Hash Cond: (permtest_parent_3.a = permtest_parent_2.a) + -> Seq Scan on permtest_grandchild permtest_parent_3 + -> Hash + -> Seq Scan on permtest_grandchild permtest_parent_2 + Filter: ("left"(c, 3) ~ 'a1$'::text) +(12 rows) + reset session authorization; revoke all on permtest_parent from regress_no_child_access; drop role regress_no_child_access; +revoke all on permtest_parent from regress_permtest_view_owner; +drop view regress_permtest_schema.permtest_parent_view; +drop schema regress_permtest_schema; +drop role regress_permtest_view_owner; drop table permtest_parent; -- Verify that constraint errors across partition root / child are -- handled correctly (Bug #16293) diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql index 195aedb5ff..d2363768bc 100644 --- a/src/test/regress/sql/inherit.sql +++ b/src/test/regress/sql/inherit.sql @@ -907,8 +907,36 @@ explain (costs off) select p2.a, p1.c from permtest_parent p1 inner join permtest_parent p2 on p1.a = p2.a and left(p1.c, 3) ~ 'a1$'; reset session authorization; + +-- Check with a view over permtest_parent and a UNION ALL over the view, +-- especially that permtest_parent's permissions are checked with the role +-- owning the view as permtest_parent's RTE's checkAsUser. +create role regress_permtest_view_owner; +create schema regress_permtest_schema; +grant all on schema regress_permtest_schema to regress_permtest_view_owner; +grant select(a,c) on permtest_parent to regress_permtest_view_owner; +set session authorization regress_permtest_view_owner; +create view regress_permtest_schema.permtest_parent_view as + select a, c from permtest_parent; +-- Like above, the 2nd arm of UNION ALL gets a hash join due to lack of access +-- to the expression index's stats +explain (costs off) + select p2.a, p1.c + from regress_permtest_schema.permtest_parent_view p1 + inner join regress_permtest_schema.permtest_parent_view p2 + on p1.a = p2.a and p1.c ~ 'a1$' + union all + select p2.a, p1.c + from regress_permtest_schema.permtest_parent_view p1 + inner join regress_permtest_schema.permtest_parent_view 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; +revoke all on permtest_parent from regress_permtest_view_owner; +drop view regress_permtest_schema.permtest_parent_view; +drop schema regress_permtest_schema; +drop role regress_permtest_view_owner; drop table permtest_parent; -- Verify that constraint errors across partition root / child are -- 2.35.3