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 <[email protected]>
Discussion: https://postgr.es/m/CA+HiwqGFCs2uq7VRKi7g+FFKbP6Ea_2_HkgZb2HPhUfaAKT3ng@mail.gmail.com

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 8d7500abfbdb9ccd33234128e31a464546f5d301..20c7b1ad05aebc0e70382b861d8e8cb63ead4e7a 100644 (file)
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -624,7 +624,6 @@ postgresGetForeignRelSize(PlannerInfo *root,
 {
    PgFdwRelationInfo *fpinfo;
    ListCell   *lc;
-   RangeTblEntry *rte = planner_rt_fetch(baserel->relid, root);
 
    /*
     * We use PgFdwRelationInfo to pass various information to subsequent
@@ -663,8 +662,9 @@ postgresGetForeignRelSize(PlannerInfo *root,
     */
    if (fpinfo->use_remote_estimate)
    {
-       Oid         userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+       Oid         userid;
 
+       userid = OidIsValid(baserel->userid) ? baserel->userid : GetUserId();
        fpinfo->user = GetUserMapping(userid, fpinfo->server->serverid);
    }
    else
@@ -1510,16 +1510,14 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
 
    /*
     * Identify which user to do the remote access as.  This should match what
-    * ExecCheckRTEPerms() does.  In case of a join or aggregate, use the
-    * lowest-numbered member RTE as a representative; we would get the same
-    * result from any.
+    * ExecCheckRTEPerms() does.
     */
+   userid = OidIsValid(fsplan->checkAsUser) ? fsplan->checkAsUser : GetUserId();
    if (fsplan->scan.scanrelid > 0)
        rtindex = fsplan->scan.scanrelid;
    else
        rtindex = bms_next_member(fsplan->fs_relids, -1);
    rte = exec_rt_fetch(rtindex, estate);
-   userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
 
    /* Get info about foreign table. */
    table = GetForeignTable(rte->relid);
@@ -2633,7 +2631,6 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
    EState     *estate = node->ss.ps.state;
    PgFdwDirectModifyState *dmstate;
    Index       rtindex;
-   RangeTblEntry *rte;
    Oid         userid;
    ForeignTable *table;
    UserMapping *user;
@@ -2655,11 +2652,10 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
     * Identify which user to do the remote access as.  This should match what
     * ExecCheckRTEPerms() does.
     */
-   rtindex = node->resultRelInfo->ri_RangeTableIndex;
-   rte = exec_rt_fetch(rtindex, estate);
-   userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+   userid = OidIsValid(fsplan->checkAsUser) ? fsplan->checkAsUser : GetUserId();
 
    /* Get info about foreign table. */
+   rtindex = node->resultRelInfo->ri_RangeTableIndex;
    if (fsplan->scan.scanrelid == 0)
        dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags);
    else
@@ -3983,7 +3979,7 @@ create_foreign_modify(EState *estate,
     * Identify which user to do the remote access as.  This should match what
     * ExecCheckRTEPerms() does.
     */
-   userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+   userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
 
    /* Get info about foreign table. */
    table = GetForeignTable(RelationGetRelid(rel));

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index e301c687e37ef6e95e20053473e8965b2a3ad596..8bf2ba1c04afa4095b84ad2d17d37e76dd53b148 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -631,7 +631,7 @@ ExecCheckRTEPerms(RangeTblEntry *rte)
     * call it once in ExecCheckRTPerms and pass the userid down from there.
     * But for now, no need for the extra clutter.
     */
-   userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+   userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
 
    /*
     * We must have *all* the requiredPerms bits, but some of the bits can be

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index ac86ce90033bcf677cd1b40dc02d8b0e00e6b5cd..5013ac3377fbfaf1c54bc94102e335d79c2737cb 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4148,6 +4148,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
    /* Copy cost data from Path to Plan; no need to make FDW do this */
    copy_generic_path_info(&scan_plan->scan.plan, &best_path->path);
 
+   /* Copy user OID to access as; likewise no need to make FDW do this */
+   scan_plan->checkAsUser = rel->userid;
+
    /* Copy foreign server OID; likewise, no need to make FDW do this */
    scan_plan->fs_server = rel->serverid;
 
@@ -5794,7 +5797,8 @@ make_foreignscan(List *qptlist,
    node->operation = CMD_SELECT;
    node->resultRelation = 0;
 
-   /* fs_server will be filled in by create_foreignscan_plan */
+   /* checkAsUser, fs_server will be filled in by create_foreignscan_plan */
+   node->checkAsUser = InvalidOid;
    node->fs_server = InvalidOid;
    node->fdw_exprs = fdw_exprs;
    node->fdw_private = fdw_private;

diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index b2a723743068b8aaefac201b8e447a14f77c8031..f49cfb6cc6678b99b4ac6ffad008c78715752cbb 100644 (file)
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -128,7 +128,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
        return;
 
    /* Switch to checkAsUser if it's set */
-   user_id = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+   user_id = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
 
    /* Determine the state of RLS for this, pass checkAsUser explicitly */
    rls_status = check_enable_rls(rte->relid, rte->checkAsUser, false);

diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index ab97e71dd795dc26972f702390c4d8144c51faab..c1652cb4c51cbe1da6bc577b1d8f365531db2813 100644 (file)
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -1598,6 +1598,7 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
                             Bitmapset **attnums, List **exprs)
 {
    RangeTblEntry *rte = root->simple_rte_array[relid];
+   RelOptInfo *rel = root->simple_rel_array[relid];
    RestrictInfo *rinfo;
    int         clause_relid;
    Oid         userid;
@@ -1646,10 +1647,9 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid,
        return false;
 
    /*
-    * Check that the user has permission to read all required attributes. Use
-    * checkAsUser if it's set, in case we're accessing the table via a view.
+    * Check that the user has permission to read all required attributes.
     */
-   userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+   userid = OidIsValid(rel->userid) ? rel->userid : GetUserId();
 
    /* Table-level SELECT privilege is sufficient for all columns */
    if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK)

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index f116924d3c49992758c35f0d10ac466a227647aa..db21cf3c355a7c495b6f3f9f1131a9d529d7dbf6 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5155,10 +5155,11 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
                                Assert(rte->rtekind == RTE_RELATION);
 
                                /*
-                                * Use checkAsUser if it's set, in case we're
-                                * accessing the table via a view.
+                                * Use onerel->userid if it's set, in case
+                                * we're accessing the table via a view.
                                 */
-                               userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+                               userid = OidIsValid(onerel->userid) ?
+                                   onerel->userid : GetUserId();
 
                                /*
                                 * For simplicity, we insist on the whole
@@ -5210,7 +5211,8 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
                                        rte = planner_rt_fetch(varno, root);
                                        Assert(rte->rtekind == RTE_RELATION);
 
-                                       userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+                                       userid = OidIsValid(onerel->userid) ?
+                                           onerel->userid : GetUserId();
 
                                        vardata->acl_ok =
                                            rte->securityQuals == NIL &&
@@ -5290,10 +5292,11 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
                    vardata->freefunc = ReleaseDummy;
 
                    /*
-                    * Use checkAsUser if it's set, in case we're accessing
+                    * Use onerel->userid if it's set, in case we're accessing
                     * the table via a view.
                     */
-                   userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+                   userid = OidIsValid(onerel->userid) ?
+                       onerel->userid : GetUserId();
 
                    /*
                     * For simplicity, we insist on the whole table being
@@ -5341,7 +5344,8 @@ examine_variable(PlannerInfo *root, Node *node, int varRelid,
                            rte = planner_rt_fetch(varno, root);
                            Assert(rte->rtekind == RTE_RELATION);
 
-                           userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+                           userid = OidIsValid(onerel->userid) ?
+                               onerel->userid : GetUserId();
 
                            vardata->acl_ok =
                                rte->securityQuals == NIL &&
@@ -5402,15 +5406,17 @@ examine_simple_variable(PlannerInfo *root, Var *var,
 
        if (HeapTupleIsValid(vardata->statsTuple))
        {
+           RelOptInfo *onerel = find_base_rel(root, var->varno);
            Oid         userid;
 
            /*
             * Check if user has permission to read this column.  We require
             * all rows to be accessible, so there must be no securityQuals
-            * from security barrier views or RLS policies.  Use checkAsUser
-            * if it's set, in case we're accessing the table via a view.
+            * from security barrier views or RLS policies.  Use
+            * onerel->userid if it's set, in case we're accessing the table
+            * via a view.
             */
-           userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+           userid = OidIsValid(onerel->userid) ?  onerel->userid : GetUserId();
 
            vardata->acl_ok =
                rte->securityQuals == NIL &&
@@ -5479,7 +5485,8 @@ examine_simple_variable(PlannerInfo *root, Var *var,
                rte = planner_rt_fetch(varno, root);
                Assert(rte->rtekind == RTE_RELATION);
 
-               userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
+               userid = OidIsValid(onerel->userid) ?
+                   onerel->userid : GetUserId();
 
                vardata->acl_ok =
                    rte->securityQuals == NIL &&

diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
index 75d42c9ec3fb8511c310eccdc0260adc1650fa22..58effdb1c97c57b326b510d3f6b1032f7d1a6262 100644 (file)
--- a/src/backend/utils/misc/rls.c
+++ b/src/backend/utils/misc/rls.c
@@ -51,7 +51,7 @@
 int
 check_enable_rls(Oid relid, Oid checkAsUser, bool noError)
 {
-   Oid         user_id = checkAsUser ? checkAsUser : GetUserId();
+   Oid         user_id = OidIsValid(checkAsUser) ? checkAsUser : GetUserId();
    HeapTuple   tuple;
    Form_pg_class classform;
    bool        relrowsecurity;

diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index a544b313d36c039b24ab5f9b61d9e5b0a55e82ec..ef95429a0d0bb698908a634cf019f4114d585f0b 100644 (file)
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -901,7 +901,7 @@ typedef struct RelOptInfo
     */
    /* identifies server for the table or join */
    Oid         serverid;
-   /* identifies user to check access as */
+   /* identifies user to check access as; 0 means to check as current user */
    Oid         userid;
    /* join is only valid for current user */
    bool        useridiscurrent;

diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 5c2ab1b37921226319adae62438af44ed009230f..61cae463fb3c4c37347acd91e9efab9313f4b8e6 100644 (file)
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -703,6 +703,8 @@ typedef struct ForeignScan
    Scan        scan;
    CmdType     operation;      /* SELECT/INSERT/UPDATE/DELETE */
    Index       resultRelation; /* direct modification target's RT index */
+   Oid         checkAsUser;    /* user to perform the scan as; 0 means to
+                                * check as current user */
    Oid         fs_server;      /* OID of foreign server */
    List       *fdw_exprs;      /* expressions that FDW may evaluate */
    List       *fdw_private;    /* private data for FDW */