Skip to content

Commit 599b33b

Browse files
alvherrealvherre
committed
Stop accessing checkAsUser via RTE in some cases
A future commit will move the checkAsUser field from RangeTblEntry to a new node that, unlike RTEs, will only be created for tables mentioned in the query but not for the inheritance child relations added to the query by the planner. So, checkAsUser value for a given child relation will have to be obtained by referring to that for its ancestor mentioned in the query. In preparation, it seems better to expand the use of RelOptInfo.userid during planning in place of rte->checkAsUser so that there will be fewer places to adjust for the above change. Given that the child-to-ancestor mapping is not available during the execution of a given "child" ForeignScan node, add a checkAsUser field to ForeignScan to carry the child relation's RelOptInfo.userid. Author: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CA+HiwqGFCs2uq7VRKi7g+FFKbP6Ea_2_HkgZb2HPhUfaAKT3ng@mail.gmail.com
1 parent d2a4490 commit 599b33b

File tree

9 files changed

+39
-30
lines changed

9 files changed

+39
-30
lines changed

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,6 @@ postgresGetForeignRelSize(PlannerInfo *root,
624624
{
625625
PgFdwRelationInfo *fpinfo;
626626
ListCell *lc;
627-
RangeTblEntry *rte = planner_rt_fetch(baserel->relid, root);
628627

629628
/*
630629
* We use PgFdwRelationInfo to pass various information to subsequent
@@ -663,8 +662,9 @@ postgresGetForeignRelSize(PlannerInfo *root,
663662
*/
664663
if (fpinfo->use_remote_estimate)
665664
{
666-
Oid userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
665+
Oid userid;
667666

667+
userid = OidIsValid(baserel->userid) ? baserel->userid : GetUserId();
668668
fpinfo->user = GetUserMapping(userid, fpinfo->server->serverid);
669669
}
670670
else
@@ -1510,16 +1510,14 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
15101510

15111511
/*
15121512
* Identify which user to do the remote access as. This should match what
1513-
* ExecCheckRTEPerms() does. In case of a join or aggregate, use the
1514-
* lowest-numbered member RTE as a representative; we would get the same
1515-
* result from any.
1513+
* ExecCheckRTEPerms() does.
15161514
*/
1515+
userid = OidIsValid(fsplan->checkAsUser) ? fsplan->checkAsUser : GetUserId();
15171516
if (fsplan->scan.scanrelid > 0)
15181517
rtindex = fsplan->scan.scanrelid;
15191518
else
15201519
rtindex = bms_next_member(fsplan->fs_relids, -1);
15211520
rte = exec_rt_fetch(rtindex, estate);
1522-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
15231521

15241522
/* Get info about foreign table. */
15251523
table = GetForeignTable(rte->relid);
@@ -2633,7 +2631,6 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
26332631
EState *estate = node->ss.ps.state;
26342632
PgFdwDirectModifyState *dmstate;
26352633
Index rtindex;
2636-
RangeTblEntry *rte;
26372634
Oid userid;
26382635
ForeignTable *table;
26392636
UserMapping *user;
@@ -2655,11 +2652,10 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
26552652
* Identify which user to do the remote access as. This should match what
26562653
* ExecCheckRTEPerms() does.
26572654
*/
2658-
rtindex = node->resultRelInfo->ri_RangeTableIndex;
2659-
rte = exec_rt_fetch(rtindex, estate);
2660-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
2655+
userid = OidIsValid(fsplan->checkAsUser) ? fsplan->checkAsUser : GetUserId();
26612656

26622657
/* Get info about foreign table. */
2658+
rtindex = node->resultRelInfo->ri_RangeTableIndex;
26632659
if (fsplan->scan.scanrelid == 0)
26642660
dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags);
26652661
else
@@ -3983,7 +3979,7 @@ create_foreign_modify(EState *estate,
39833979
* Identify which user to do the remote access as. This should match what
39843980
* ExecCheckRTEPerms() does.
39853981
*/
3986-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
3982+
userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
39873983

39883984
/* Get info about foreign table. */
39893985
table = GetForeignTable(RelationGetRelid(rel));

‎src/backend/executor/execMain.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
631631
* call it once in ExecCheckRTPerms and pass the userid down from there.
632632
* But for now, no need for the extra clutter.
633633
*/
634-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
634+
userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
635635

636636
/*
637637
* We must have *all* the requiredPerms bits, but some of the bits can be

‎src/backend/optimizer/plan/createplan.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4148,6 +4148,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
41484148
/* Copy cost data from Path to Plan; no need to make FDW do this */
41494149
copy_generic_path_info(&scan_plan->scan.plan, &best_path->path);
41504150

4151+
/* Copy user OID to access as; likewise no need to make FDW do this */
4152+
scan_plan->checkAsUser = rel->userid;
4153+
41514154
/* Copy foreign server OID; likewise, no need to make FDW do this */
41524155
scan_plan->fs_server = rel->serverid;
41534156

@@ -5794,7 +5797,8 @@ make_foreignscan(List *qptlist,
57945797
node->operation = CMD_SELECT;
57955798
node->resultRelation = 0;
57965799

5797-
/* fs_server will be filled in by create_foreignscan_plan */
5800+
/* checkAsUser, fs_server will be filled in by create_foreignscan_plan */
5801+
node->checkAsUser = InvalidOid;
57985802
node->fs_server = InvalidOid;
57995803
node->fdw_exprs = fdw_exprs;
58005804
node->fdw_private = fdw_private;

‎src/backend/rewrite/rowsecurity.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
128128
return;
129129

130130
/* Switch to checkAsUser if it's set */
131-
user_id = rte->checkAsUser ? rte->checkAsUser : GetUserId();
131+
user_id = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
132132

133133
/* Determine the state of RLS for this, pass checkAsUser explicitly */
134134
rls_status = check_enable_rls(rte->relid, rte->checkAsUser, false);

‎src/backend/statistics/extended_stats.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1598,6 +1598,7 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
15981598
Bitmapset **attnums, List **exprs)
15991599
{
16001600
RangeTblEntry *rte = root->simple_rte_array[relid];
1601+
RelOptInfo *rel = root->simple_rel_array[relid];
16011602
RestrictInfo *rinfo;
16021603
int clause_relid;
16031604
Oid userid;
@@ -1646,10 +1647,9 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
16461647
return false;
16471648

16481649
/*
1649-
* Check that the user has permission to read all required attributes. Use
1650-
* checkAsUser if it's set, in case we're accessing the table via a view.
1650+
* Check that the user has permission to read all required attributes.
16511651
*/
1652-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
1652+
userid = OidIsValid(rel->userid) ? rel->userid : GetUserId();
16531653

16541654
/* Table-level SELECT privilege is sufficient for all columns */
16551655
if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK)

‎src/backend/utils/adt/selfuncs.c

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5155,10 +5155,11 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
51555155
Assert(rte->rtekind == RTE_RELATION);
51565156

51575157
/*
5158-
* Use checkAsUser if it's set, in case we're
5159-
* accessing the table via a view.
5158+
* Use onerel->userid if it's set, in case
5159+
* we're accessing the table via a view.
51605160
*/
5161-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
5161+
userid = OidIsValid(onerel->userid) ?
5162+
onerel->userid : GetUserId();
51625163

51635164
/*
51645165
* For simplicity, we insist on the whole
@@ -5210,7 +5211,8 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
52105211
rte = planner_rt_fetch(varno, root);
52115212
Assert(rte->rtekind == RTE_RELATION);
52125213

5213-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
5214+
userid = OidIsValid(onerel->userid) ?
5215+
onerel->userid : GetUserId();
52145216

52155217
vardata->acl_ok =
52165218
rte->securityQuals == NIL &&
@@ -5290,10 +5292,11 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
52905292
vardata->freefunc = ReleaseDummy;
52915293

52925294
/*
5293-
* Use checkAsUser if it's set, in case we're accessing
5295+
* Use onerel->userid if it's set, in case we're accessing
52945296
* the table via a view.
52955297
*/
5296-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
5298+
userid = OidIsValid(onerel->userid) ?
5299+
onerel->userid : GetUserId();
52975300

52985301
/*
52995302
* For simplicity, we insist on the whole table being
@@ -5341,7 +5344,8 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
53415344
rte = planner_rt_fetch(varno, root);
53425345
Assert(rte->rtekind == RTE_RELATION);
53435346

5344-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
5347+
userid = OidIsValid(onerel->userid) ?
5348+
onerel->userid : GetUserId();
53455349

53465350
vardata->acl_ok =
53475351
rte->securityQuals == NIL &&
@@ -5402,15 +5406,17 @@ examine_simple_variable(PlannerInfo *root, Var *var,
54025406

54035407
if (HeapTupleIsValid(vardata->statsTuple))
54045408
{
5409+
RelOptInfo *onerel = find_base_rel(root, var->varno);
54055410
Oid userid;
54065411

54075412
/*
54085413
* Check if user has permission to read this column. We require
54095414
* all rows to be accessible, so there must be no securityQuals
5410-
* from security barrier views or RLS policies. Use checkAsUser
5411-
* if it's set, in case we're accessing the table via a view.
5415+
* from security barrier views or RLS policies. Use
5416+
* onerel->userid if it's set, in case we're accessing the table
5417+
* via a view.
54125418
*/
5413-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
5419+
userid = OidIsValid(onerel->userid) ? onerel->userid : GetUserId();
54145420

54155421
vardata->acl_ok =
54165422
rte->securityQuals == NIL &&
@@ -5479,7 +5485,8 @@ examine_simple_variable(PlannerInfo *root, Var *var,
54795485
rte = planner_rt_fetch(varno, root);
54805486
Assert(rte->rtekind == RTE_RELATION);
54815487

5482-
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
5488+
userid = OidIsValid(onerel->userid) ?
5489+
onerel->userid : GetUserId();
54835490

54845491
vardata->acl_ok =
54855492
rte->securityQuals == NIL &&

‎src/backend/utils/misc/rls.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
int
5252
check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
5353
{
54-
Oid user_id = checkAsUser ? checkAsUser : GetUserId();
54+
Oid user_id = OidIsValid(checkAsUser) ? checkAsUser : GetUserId();
5555
HeapTuple tuple;
5656
Form_pg_class classform;
5757
bool relrowsecurity;

‎src/include/nodes/pathnodes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ typedef struct RelOptInfo
901901
*/
902902
/* identifies server for the table or join */
903903
Oid serverid;
904-
/* identifies user to check access as */
904+
/* identifies user to check access as; 0 means to check as current user */
905905
Oid userid;
906906
/* join is only valid for current user */
907907
bool useridiscurrent;

‎src/include/nodes/plannodes.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,8 @@ typedef struct ForeignScan
703703
Scan scan;
704704
CmdType operation; /* SELECT/INSERT/UPDATE/DELETE */
705705
Index resultRelation; /* direct modification target's RT index */
706+
Oid checkAsUser; /* user to perform the scan as; 0 means to
707+
* check as current user */
706708
Oid fs_server; /* OID of foreign server */
707709
List *fdw_exprs; /* expressions that FDW may evaluate */
708710
List *fdw_private; /* private data for FDW */

0 commit comments

Comments
 (0)