Hi,
I noticed odd behavior in invalidating a cached plan with a foreign join
due to user mapping changes. Consider:
postgres=# create extension postgres_fdw;
postgres=# create server loopback foreign data wrapper postgres_fdw
options (dbname 'postgres');
postgres=# create user mapping for public server loopback;
postgres=# create table t1 (a int, b int);
postgres=# insert into t1 values (1, 1);
postgres=# create foreign table ft1 (a int, b int) server loopback
options (table_name 't1');
postgres=# analyze ft1;
postgres=# create view v1 as select * from ft1;
postgres=# create user v1_owner;
postgres=# alter view v1 owner to v1_owner;
postgres=# grant select on ft1 to v1_owner;
postgres=# prepare join_stmt as select * from ft1, v1 where ft1.a = v1.a;
postgres=# explain verbose execute join_stmt;
QUERY PLAN
--------------------------------------------------------------------------------------------------------------
Foreign Scan (cost=100.00..102.04 rows=1 width=16)
Output: ft1.a, ft1.b, ft1_1.a, ft1_1.b
Relations: (public.ft1) INNER JOIN (public.ft1)
Remote SQL: SELECT r1.a, r1.b, r5.a, r5.b FROM (public.t1 r1 INNER
JOIN public.t1 r5 ON (((r1.a = r5.a))))
(4 rows)
That's great.
postgres=# create user mapping for v1_owner server loopback;
postgres=# explain verbose execute join_stmt;
QUERY PLAN
------------------------------------------------------------------------------
Nested Loop (cost=200.00..202.07 rows=1 width=16)
Output: ft1.a, ft1.b, ft1_1.a, ft1_1.b
Join Filter: (ft1.a = ft1_1.a)
-> Foreign Scan on public.ft1 (cost=100.00..101.03 rows=1 width=8)
Output: ft1.a, ft1.b
Remote SQL: SELECT a, b FROM public.t1
-> Foreign Scan on public.ft1 ft1_1 (cost=100.00..101.03 rows=1
width=8)
Output: ft1_1.a, ft1_1.b
Remote SQL: SELECT a, b FROM public.t1
(9 rows)
That's also great. Since ft1 and v1 use different user mappings, the
join should not be pushed down.
postgres=# drop user mapping for v1_owner server loopback;
postgres=# explain verbose execute join_stmt;
QUERY PLAN
------------------------------------------------------------------------------
Nested Loop (cost=200.00..202.07 rows=1 width=16)
Output: ft1.a, ft1.b, ft1_1.a, ft1_1.b
Join Filter: (ft1.a = ft1_1.a)
-> Foreign Scan on public.ft1 (cost=100.00..101.03 rows=1 width=8)
Output: ft1.a, ft1.b
Remote SQL: SELECT a, b FROM public.t1
-> Foreign Scan on public.ft1 ft1_1 (cost=100.00..101.03 rows=1
width=8)
Output: ft1_1.a, ft1_1.b
Remote SQL: SELECT a, b FROM public.t1
(9 rows)
Seems odd to me. Both relations use the same user mapping as before, so
the join should be pushed down.
Another thing I noticed is that the code in plancache.c would invalidate
a cached plan with a foreign join due to user mapping changes even in
the case where user mappings are meaningless to the FDW.
To fix the first, I'd like to propose (1) replacing the existing
has_foreign_join flag in the CachedPlan data structure with a new flag,
say uses_user_mapping, that indicates whether a cached plan uses any
user mapping regardless of whether the cached plan has foreign joins or
not (likewise, replace the hasForeignJoin flag in PlannedStmt), and (2)
invalidating the cached plan if the uses_user_mapping flag is true. I
thought that we invalidate the cached plan if the flag is true and there
is a possibility of performing foreign joins, but it seems to me that
updates on the corresponding catalog are infrequent enough that such a
detailed tracking is not worth the effort. One benefit of using the
proposed approach is that we could make the FDW's handling of user
mappings in BeginForeignScan more efficient; currently, there is
additional overhead caused by catalog re-lookups to obtain the user
mapping information for the simple-foreign-table-scan case where user
mappings mean something to the FDW as in postgres_fdw (probably, updates
on the catalogs are infrequent, though), but we could improve the
efficiency by using the validated user mapping information created at
planning time for that case as well as for the foreign-join case. For
that, I'd like to propose storing the validated user mapping information
into ForeignScan, not fdw_private. There is a discussion about using an
ExtensibleNode [1] for that, but the proposed approach would make the
FDW's job much simpler.
I don't think the above change is sufficient to fix the second. The
root reason for that is that since currently, we allow the user mapping
OID (rel->umid) to be InvalidOid in two cases: (1) user mappings mean
something to the FDW but it can't get any user mapping at planning time
and (2) user mappings are meaningless to the FDW, we cannot distinguish
these two cases. So, I'd like to introduce a new callback routine to
specify that user mappings mean something to the FDW as proposed by Tom
[2], and use that to reject the former case, which allows us to set the
above uses_user_mapping flag appropriately, ie, set the flag to true
only if user mapping changes require forcing a replan. This would
change the postgres_fdw's behavior that it allows to prepare statements
involving foreign tables without any user mappings as long as those
aren't required at planning time, but I'm not sure that it's a good idea
to keep that behavior because ISTM that such a behavior would make
people sloppy about creating user mappings, which could lead to latent
security problems [2].
Attached is a proposed patch for that.
Best regards,
Etsuro Fujita
[1]
https://www.postgresql.org/message-id/CA%2BTgmoZK6BB4RTb5paz45Vme%3Dq6Z3D7FF2-VKdVyQCS1ps-cGw%40mail.gmail.com
[2] https://www.postgresql.org/message-id/18299.1457923919%40sss.pgh.pa.us
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***************
*** 128,133 **** static void fileBeginForeignScan(ForeignScanState *node, int eflags);
--- 128,134 ----
static TupleTableSlot *fileIterateForeignScan(ForeignScanState *node);
static void fileReScanForeignScan(ForeignScanState *node);
static void fileEndForeignScan(ForeignScanState *node);
+ static bool fileNeedUserMapping(void);
static bool fileAnalyzeForeignTable(Relation relation,
AcquireSampleRowsFunc *func,
BlockNumber *totalpages);
***************
*** 171,176 **** file_fdw_handler(PG_FUNCTION_ARGS)
--- 172,178 ----
fdwroutine->IterateForeignScan = fileIterateForeignScan;
fdwroutine->ReScanForeignScan = fileReScanForeignScan;
fdwroutine->EndForeignScan = fileEndForeignScan;
+ fdwroutine->NeedUserMapping = fileNeedUserMapping;
fdwroutine->AnalyzeForeignTable = fileAnalyzeForeignTable;
fdwroutine->IsForeignScanParallelSafe = fileIsForeignScanParallelSafe;
***************
*** 727,732 **** fileEndForeignScan(ForeignScanState *node)
--- 729,744 ----
}
/*
+ * fileNeedUserMapping
+ * Test whether any user mapping is needed for scanning a foreign table
+ */
+ static bool
+ fileNeedUserMapping(void)
+ {
+ return false;
+ }
+
+ /*
* fileAnalyzeForeignTable
* Test whether analyzing this foreign table is supported
*/
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***************
*** 2086,2117 **** EXECUTE join_stmt;
40 |
(10 rows)
! -- change the session user to view_owner and execute the statement. Because of
! -- change in session user, the plan should get invalidated and created again.
! -- The join will not be pushed down since the joining relations do not have a
! -- valid user mapping.
SET SESSION ROLE view_owner;
EXPLAIN (COSTS OFF, VERBOSE) EXECUTE join_stmt;
! QUERY PLAN
! ------------------------------------------------------------------
! Limit
! Output: t1.c1, t2.c1
! -> Sort
! Output: t1.c1, t2.c1
! Sort Key: t1.c1, t2.c1
! -> Hash Left Join
! Output: t1.c1, t2.c1
! Hash Cond: (t1.c1 = t2.c1)
! -> Foreign Scan on public.ft4 t1
! Output: t1.c1, t1.c2, t1.c3
! Remote SQL: SELECT c1 FROM "S 1"."T 3"
! -> Hash
! Output: t2.c1
! -> Foreign Scan on public.ft5 t2
! Output: t2.c1
! Remote SQL: SELECT c1 FROM "S 1"."T 4"
! (16 rows)
!
RESET ROLE;
DEALLOCATE join_stmt;
CREATE VIEW v_ft5 AS SELECT * FROM ft5;
--- 2086,2098 ----
40 |
(10 rows)
! -- change the session user to view_owner and execute the statement
! -- error thrown due to no user mapping
SET SESSION ROLE view_owner;
EXPLAIN (COSTS OFF, VERBOSE) EXECUTE join_stmt;
! ERROR: user mapping not found for "view_owner"
! EXECUTE join_stmt;
! ERROR: user mapping not found for "view_owner"
RESET ROLE;
DEALLOCATE join_stmt;
CREATE VIEW v_ft5 AS SELECT * FROM ft5;
***************
*** 2168,2173 **** EXECUTE join_stmt;
--- 2149,2176 ----
----+----
(0 rows)
+ -- drop user mapping for view_owner and execute the prepared statement
+ -- both relations use the same user mapping as before, so the join should
+ -- be pushed down
+ DROP USER MAPPING FOR view_owner SERVER loopback;
+ EXPLAIN (COSTS false, VERBOSE) EXECUTE join_stmt;
+ QUERY PLAN
+ ----------------------------------------------------------------------------------------------------------------------------------------------
+ Limit
+ Output: t1.c1, ft5.c1
+ -> Foreign Scan
+ Output: t1.c1, ft5.c1
+ Relations: (public.ft5 t1) INNER JOIN (public.ft5)
+ Remote SQL: SELECT r1.c1, r6.c1 FROM ("S 1"."T 4" r1 INNER JOIN "S 1"."T 4" r6 ON (((r1.c1 = r6.c1)))) ORDER BY r1.c1 ASC NULLS LAST
+ (6 rows)
+
+ EXECUTE join_stmt;
+ c1 | c1
+ ----+----
+ (0 rows)
+
+ -- recreate the dropped user mapping for further tests
+ CREATE USER MAPPING FOR view_owner SERVER loopback;
-- If a sub-join can't be pushed down, upper level join shouldn't be either.
EXPLAIN (COSTS false, VERBOSE)
SELECT t1.c1, t2.c1 FROM (ft5 t1 JOIN v_ft5 t2 ON (t1.c1 = t2.c1)) left join (ft5 t3 JOIN v_ft5 t4 ON (t3.c1 = t4.c1)) ON (t1.c1 = t3.c1);
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 67,74 **** enum FdwScanPrivateIndex
FdwScanPrivateRetrievedAttrs,
/* Integer representing the desired fetch_size */
FdwScanPrivateFetchSize,
- /* Oid of user mapping to be used while connecting to the foreign server */
- FdwScanPrivateUserMappingOid,
/*
* String describing join i.e. names of relations being joined and types
--- 67,72 ----
***************
*** 289,294 **** static void postgresBeginForeignScan(ForeignScanState *node, int eflags);
--- 287,293 ----
static TupleTableSlot *postgresIterateForeignScan(ForeignScanState *node);
static void postgresReScanForeignScan(ForeignScanState *node);
static void postgresEndForeignScan(ForeignScanState *node);
+ static bool postgresNeedUserMapping(void);
static void postgresAddForeignUpdateTargets(Query *parsetree,
RangeTblEntry *target_rte,
Relation target_relation);
***************
*** 426,431 **** postgres_fdw_handler(PG_FUNCTION_ARGS)
--- 425,431 ----
routine->IterateForeignScan = postgresIterateForeignScan;
routine->ReScanForeignScan = postgresReScanForeignScan;
routine->EndForeignScan = postgresEndForeignScan;
+ routine->NeedUserMapping = postgresNeedUserMapping;
/* Functions for updating foreign tables */
routine->AddForeignUpdateTargets = postgresAddForeignUpdateTargets;
***************
*** 1226,1236 **** postgresGetForeignPlan(PlannerInfo *root,
* Build the fdw_private list that will be available to the executor.
* Items in the list must match order in enum FdwScanPrivateIndex.
*/
! fdw_private = list_make5(makeString(sql.data),
remote_conds,
retrieved_attrs,
! makeInteger(fpinfo->fetch_size),
! makeInteger(foreignrel->umid));
if (foreignrel->reloptkind == RELOPT_JOINREL)
fdw_private = lappend(fdw_private,
makeString(fpinfo->relation_name->data));
--- 1226,1235 ----
* Build the fdw_private list that will be available to the executor.
* Items in the list must match order in enum FdwScanPrivateIndex.
*/
! fdw_private = list_make4(makeString(sql.data),
remote_conds,
retrieved_attrs,
! makeInteger(fpinfo->fetch_size));
if (foreignrel->reloptkind == RELOPT_JOINREL)
fdw_private = lappend(fdw_private,
makeString(fpinfo->relation_name->data));
***************
*** 1278,1318 **** postgresBeginForeignScan(ForeignScanState *node, int eflags)
node->fdw_state = (void *) fsstate;
/*
- * Obtain the foreign server where to connect and user mapping to use for
- * connection. For base relations we obtain this information from
- * catalogs. For join relations, this information is frozen at the time of
- * planning to ensure that the join is safe to pushdown. In case the
- * information goes stale between planning and execution, plan will be
- * invalidated and replanned.
- */
- if (fsplan->scan.scanrelid > 0)
- {
- ForeignTable *table;
-
- /*
- * Identify which user to do the remote access as. This should match
- * what ExecCheckRTEPerms() does.
- */
- RangeTblEntry *rte = rt_fetch(fsplan->scan.scanrelid, estate->es_range_table);
- Oid userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
-
- fsstate->rel = node->ss.ss_currentRelation;
- table = GetForeignTable(RelationGetRelid(fsstate->rel));
-
- user = GetUserMapping(userid, table->serverid);
- }
- else
- {
- Oid umid = intVal(list_nth(fsplan->fdw_private, FdwScanPrivateUserMappingOid));
-
- user = GetUserMappingById(umid);
- Assert(fsplan->fs_server == user->serverid);
- }
-
- /*
* Get connection to the foreign server. Connection manager will
* establish new connection if necessary.
*/
fsstate->conn = GetConnection(user, false);
/* Assign a unique ID for my cursor */
--- 1277,1286 ----
node->fdw_state = (void *) fsstate;
/*
* Get connection to the foreign server. Connection manager will
* establish new connection if necessary.
*/
+ user = GetUserMappingById(fsplan->fs_usermapping);
fsstate->conn = GetConnection(user, false);
/* Assign a unique ID for my cursor */
***************
*** 1344,1352 **** postgresBeginForeignScan(ForeignScanState *node, int eflags)
--- 1312,1326 ----
* into local representation and error reporting during that process.
*/
if (fsplan->scan.scanrelid > 0)
+ {
+ fsstate->rel = node->ss.ss_currentRelation;
fsstate->tupdesc = RelationGetDescr(fsstate->rel);
+ }
else
+ {
+ fsstate->rel = NULL;
fsstate->tupdesc = node->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
+ }
fsstate->attinmeta = TupleDescGetAttInMetadata(fsstate->tupdesc);
***************
*** 1487,1492 **** postgresEndForeignScan(ForeignScanState *node)
--- 1461,1476 ----
}
/*
+ * postgresNeedUserMapping
+ * Test whether any user mapping is needed for scanning a foreign table
+ */
+ static bool
+ postgresNeedUserMapping(void)
+ {
+ return true;
+ }
+
+ /*
* postgresAddForeignUpdateTargets
* Add resjunk column(s) needed for update/delete on a foreign table
*/
***************
*** 2251,2259 **** postgresBeginDirectModify(ForeignScanState *node, int eflags)
ForeignScan *fsplan = (ForeignScan *) node->ss.ps.plan;
EState *estate = node->ss.ps.state;
PgFdwDirectModifyState *dmstate;
- RangeTblEntry *rte;
- Oid userid;
- ForeignTable *table;
UserMapping *user;
int numParams;
--- 2235,2240 ----
***************
*** 2269,2290 **** postgresBeginDirectModify(ForeignScanState *node, int eflags)
dmstate = (PgFdwDirectModifyState *) palloc0(sizeof(PgFdwDirectModifyState));
node->fdw_state = (void *) dmstate;
- /*
- * Identify which user to do the remote access as. This should match what
- * ExecCheckRTEPerms() does.
- */
- rte = rt_fetch(fsplan->scan.scanrelid, estate->es_range_table);
- userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
-
/* Get info about foreign table. */
dmstate->rel = node->ss.ss_currentRelation;
- table = GetForeignTable(RelationGetRelid(dmstate->rel));
- user = GetUserMapping(userid, table->serverid);
/*
* Get connection to the foreign server. Connection manager will
* establish new connection if necessary.
*/
dmstate->conn = GetConnection(user, false);
/* Initialize state variable */
--- 2250,2263 ----
dmstate = (PgFdwDirectModifyState *) palloc0(sizeof(PgFdwDirectModifyState));
node->fdw_state = (void *) dmstate;
/* Get info about foreign table. */
dmstate->rel = node->ss.ss_currentRelation;
/*
* Get connection to the foreign server. Connection manager will
* establish new connection if necessary.
*/
+ user = GetUserMappingById(fsplan->fs_usermapping);
dmstate->conn = GetConnection(user, false);
/* Initialize state variable */
***************
*** 3966,3981 **** foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
List *otherclauses;
/*
- * Core code may call GetForeignJoinPaths hook even when the join relation
- * doesn't have a valid user mapping associated with it. See
- * build_join_rel() for details. We can't push down such join, since there
- * doesn't exist a user mapping which can be used to connect to the
- * foreign server.
- */
- if (!OidIsValid(joinrel->umid))
- return false;
-
- /*
* We support pushing down INNER, LEFT, RIGHT and FULL OUTER joins.
* Constructing queries representing SEMI and ANTI joins is hard, hence
* not considered right now.
--- 3939,3944 ----
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
***************
*** 494,505 **** GRANT ALL ON ft5 TO view_owner;
PREPARE join_stmt AS SELECT t1.c1, t2.c1 FROM ft4 t1 LEFT JOIN ft5 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c1, t2.c1 OFFSET 10 LIMIT 10;
EXPLAIN (COSTS OFF, VERBOSE) EXECUTE join_stmt;
EXECUTE join_stmt;
! -- change the session user to view_owner and execute the statement. Because of
! -- change in session user, the plan should get invalidated and created again.
! -- The join will not be pushed down since the joining relations do not have a
! -- valid user mapping.
SET SESSION ROLE view_owner;
EXPLAIN (COSTS OFF, VERBOSE) EXECUTE join_stmt;
RESET ROLE;
DEALLOCATE join_stmt;
--- 494,504 ----
PREPARE join_stmt AS SELECT t1.c1, t2.c1 FROM ft4 t1 LEFT JOIN ft5 t2 ON (t1.c1 = t2.c1) ORDER BY t1.c1, t2.c1 OFFSET 10 LIMIT 10;
EXPLAIN (COSTS OFF, VERBOSE) EXECUTE join_stmt;
EXECUTE join_stmt;
! -- change the session user to view_owner and execute the statement
! -- error thrown due to no user mapping
SET SESSION ROLE view_owner;
EXPLAIN (COSTS OFF, VERBOSE) EXECUTE join_stmt;
+ EXECUTE join_stmt;
RESET ROLE;
DEALLOCATE join_stmt;
***************
*** 522,527 **** EXECUTE join_stmt;
--- 521,534 ----
CREATE USER MAPPING FOR view_owner SERVER loopback;
EXPLAIN (COSTS false, VERBOSE) EXECUTE join_stmt;
EXECUTE join_stmt;
+ -- drop user mapping for view_owner and execute the prepared statement
+ -- both relations use the same user mapping as before, so the join should
+ -- be pushed down
+ DROP USER MAPPING FOR view_owner SERVER loopback;
+ EXPLAIN (COSTS false, VERBOSE) EXECUTE join_stmt;
+ EXECUTE join_stmt;
+ -- recreate the dropped user mapping for further tests
+ CREATE USER MAPPING FOR view_owner SERVER loopback;
-- If a sub-join can't be pushed down, upper level join shouldn't be either.
EXPLAIN (COSTS false, VERBOSE)
*** a/doc/src/sgml/fdwhandler.sgml
--- b/doc/src/sgml/fdwhandler.sgml
***************
*** 283,288 **** EndForeignScan (ForeignScanState *node);
--- 283,300 ----
to remote servers should be cleaned up.
</para>
+ <para>
+ <programlisting>
+ bool
+ NeedUserMapping (void);
+ </programlisting>
+
+ Test whether any user mapping is needed for scanning a foreign table.
+ This is called at the beginning of query planning. This function takes
+ no arguments, and return <literal>true</> if the FDW needs user mappings
+ and <literal>false</> otherwise.
+ </para>
+
</sect2>
<sect2 id="fdw-callbacks-join-scan">
*** a/src/backend/executor/execParallel.c
--- b/src/backend/executor/execParallel.c
***************
*** 160,166 **** ExecSerializePlan(Plan *plan, EState *estate)
pstmt->relationOids = NIL;
pstmt->invalItems = NIL; /* workers can't replan anyway... */
pstmt->hasRowSecurity = false;
! pstmt->hasForeignJoin = false;
/* Return serialized copy of our dummy PlannedStmt. */
return nodeToString(pstmt);
--- 160,166 ----
pstmt->relationOids = NIL;
pstmt->invalItems = NIL; /* workers can't replan anyway... */
pstmt->hasRowSecurity = false;
! pstmt->usesUserMapping = false;
/* Return serialized copy of our dummy PlannedStmt. */
return nodeToString(pstmt);
*** a/src/backend/foreign/foreign.c
--- b/src/backend/foreign/foreign.c
***************
*** 31,37 ****
extern Datum pg_options_to_table(PG_FUNCTION_ARGS);
extern Datum postgresql_fdw_validator(PG_FUNCTION_ARGS);
! static HeapTuple find_user_mapping(Oid userid, Oid serverid, bool missing_ok);
/*
* GetForeignDataWrapper - look up the foreign-data wrapper by OID.
--- 31,37 ----
extern Datum pg_options_to_table(PG_FUNCTION_ARGS);
extern Datum postgresql_fdw_validator(PG_FUNCTION_ARGS);
! static HeapTuple find_user_mapping(Oid userid, Oid serverid);
/*
* GetForeignDataWrapper - look up the foreign-data wrapper by OID.
***************
*** 223,229 **** GetUserMapping(Oid userid, Oid serverid)
bool isnull;
UserMapping *um;
! tp = find_user_mapping(userid, serverid, false);
um = (UserMapping *) palloc(sizeof(UserMapping));
um->umid = HeapTupleGetOid(tp);
--- 223,229 ----
bool isnull;
UserMapping *um;
! tp = find_user_mapping(userid, serverid);
um = (UserMapping *) palloc(sizeof(UserMapping));
um->umid = HeapTupleGetOid(tp);
***************
*** 250,272 **** GetUserMapping(Oid userid, Oid serverid)
*
* If no mapping is found for the supplied user, we also look for
* PUBLIC mappings (userid == InvalidOid).
- *
- * If missing_ok is true, the function returns InvalidOid when it does not find
- * required user mapping. Otherwise, find_user_mapping() throws error if it
- * does not find required user mapping.
*/
Oid
! GetUserMappingId(Oid userid, Oid serverid, bool missing_ok)
{
HeapTuple tp;
Oid umid;
! tp = find_user_mapping(userid, serverid, missing_ok);
!
! Assert(missing_ok || tp);
!
! if (!tp && missing_ok)
! return InvalidOid;
/* Extract the Oid */
umid = HeapTupleGetOid(tp);
--- 250,263 ----
*
* If no mapping is found for the supplied user, we also look for
* PUBLIC mappings (userid == InvalidOid).
*/
Oid
! GetUserMappingId(Oid userid, Oid serverid)
{
HeapTuple tp;
Oid umid;
! tp = find_user_mapping(userid, serverid);
/* Extract the Oid */
umid = HeapTupleGetOid(tp);
***************
*** 282,294 **** GetUserMappingId(Oid userid, Oid serverid, bool missing_ok)
*
* If no mapping is found for the supplied user, we also look for
* PUBLIC mappings (userid == InvalidOid).
- *
- * If missing_ok is true, the function returns NULL, if it does not find
- * the required user mapping. Otherwise, it throws error if it does not
- * find the required user mapping.
*/
static HeapTuple
! find_user_mapping(Oid userid, Oid serverid, bool missing_ok)
{
HeapTuple tp;
--- 273,281 ----
*
* If no mapping is found for the supplied user, we also look for
* PUBLIC mappings (userid == InvalidOid).
*/
static HeapTuple
! find_user_mapping(Oid userid, Oid serverid)
{
HeapTuple tp;
***************
*** 305,319 **** find_user_mapping(Oid userid, Oid serverid, bool missing_ok)
ObjectIdGetDatum(serverid));
if (!HeapTupleIsValid(tp))
! {
! if (missing_ok)
! return NULL;
! else
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("user mapping not found for \"%s\"",
! MappingUserName(userid))));
! }
return tp;
}
--- 292,301 ----
ObjectIdGetDatum(serverid));
if (!HeapTupleIsValid(tp))
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("user mapping not found for \"%s\"",
! MappingUserName(userid))));
return tp;
}
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 97,103 **** _copyPlannedStmt(const PlannedStmt *from)
COPY_SCALAR_FIELD(nParamExec);
COPY_SCALAR_FIELD(hasRowSecurity);
COPY_SCALAR_FIELD(parallelModeNeeded);
! COPY_SCALAR_FIELD(hasForeignJoin);
return newnode;
}
--- 97,103 ----
COPY_SCALAR_FIELD(nParamExec);
COPY_SCALAR_FIELD(hasRowSecurity);
COPY_SCALAR_FIELD(parallelModeNeeded);
! COPY_SCALAR_FIELD(usesUserMapping);
return newnode;
}
***************
*** 652,657 **** _copyForeignScan(const ForeignScan *from)
--- 652,658 ----
*/
COPY_SCALAR_FIELD(operation);
COPY_SCALAR_FIELD(fs_server);
+ COPY_SCALAR_FIELD(fs_usermapping);
COPY_NODE_FIELD(fdw_exprs);
COPY_NODE_FIELD(fdw_private);
COPY_NODE_FIELD(fdw_scan_tlist);
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 273,279 **** _outPlannedStmt(StringInfo str, const PlannedStmt *node)
WRITE_INT_FIELD(nParamExec);
WRITE_BOOL_FIELD(hasRowSecurity);
WRITE_BOOL_FIELD(parallelModeNeeded);
! WRITE_BOOL_FIELD(hasForeignJoin);
}
/*
--- 273,279 ----
WRITE_INT_FIELD(nParamExec);
WRITE_BOOL_FIELD(hasRowSecurity);
WRITE_BOOL_FIELD(parallelModeNeeded);
! WRITE_BOOL_FIELD(usesUserMapping);
}
/*
***************
*** 610,615 **** _outForeignScan(StringInfo str, const ForeignScan *node)
--- 610,616 ----
WRITE_ENUM_FIELD(operation, CmdType);
WRITE_OID_FIELD(fs_server);
+ WRITE_OID_FIELD(fs_usermapping);
WRITE_NODE_FIELD(fdw_exprs);
WRITE_NODE_FIELD(fdw_private);
WRITE_NODE_FIELD(fdw_scan_tlist);
***************
*** 2018,2024 **** _outPlannerGlobal(StringInfo str, const PlannerGlobal *node)
WRITE_BOOL_FIELD(hasRowSecurity);
WRITE_BOOL_FIELD(parallelModeOK);
WRITE_BOOL_FIELD(parallelModeNeeded);
! WRITE_BOOL_FIELD(hasForeignJoin);
}
static void
--- 2019,2025 ----
WRITE_BOOL_FIELD(hasRowSecurity);
WRITE_BOOL_FIELD(parallelModeOK);
WRITE_BOOL_FIELD(parallelModeNeeded);
! WRITE_BOOL_FIELD(usesUserMapping);
}
static void
*** a/src/backend/nodes/readfuncs.c
--- b/src/backend/nodes/readfuncs.c
***************
*** 1402,1408 **** _readPlannedStmt(void)
READ_INT_FIELD(nParamExec);
READ_BOOL_FIELD(hasRowSecurity);
READ_BOOL_FIELD(parallelModeNeeded);
! READ_BOOL_FIELD(hasForeignJoin);
READ_DONE();
}
--- 1402,1408 ----
READ_INT_FIELD(nParamExec);
READ_BOOL_FIELD(hasRowSecurity);
READ_BOOL_FIELD(parallelModeNeeded);
! READ_BOOL_FIELD(usesUserMapping);
READ_DONE();
}
***************
*** 1806,1811 **** _readForeignScan(void)
--- 1806,1812 ----
READ_ENUM_FIELD(operation, CmdType);
READ_OID_FIELD(fs_server);
+ READ_OID_FIELD(fs_usermapping);
READ_NODE_FIELD(fdw_exprs);
READ_NODE_FIELD(fdw_private);
READ_NODE_FIELD(fdw_scan_tlist);
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 3246,3259 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
/* Likewise, copy the relids that are represented by this foreign scan */
scan_plan->fs_relids = best_path->path.parent->relids;
! /*
! * If a join between foreign relations was pushed down, remember it. The
! * push-down safety of the join depends upon the server and user mapping
! * being same. That can change between planning and execution time, in
! * which case the plan should be invalidated.
! */
! if (scan_relid == 0)
! root->glob->hasForeignJoin = true;
/*
* Replace any outer-relation variables with nestloop params in the qual,
--- 3246,3256 ----
/* Likewise, copy the relids that are represented by this foreign scan */
scan_plan->fs_relids = best_path->path.parent->relids;
! /* Likewise, copy user mapping OID */
! scan_plan->fs_usermapping = rel->umid;
!
! /* Detect whether the FDW uses any user mapping */
! root->glob->usesUserMapping = OidIsValid(rel->umid);
/*
* Replace any outer-relation variables with nestloop params in the qual,
*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
***************
*** 220,226 **** standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
glob->lastPlanNodeId = 0;
glob->transientPlan = false;
glob->hasRowSecurity = false;
! glob->hasForeignJoin = false;
/*
* Assess whether it's feasible to use parallel mode for this query. We
--- 220,226 ----
glob->lastPlanNodeId = 0;
glob->transientPlan = false;
glob->hasRowSecurity = false;
! glob->usesUserMapping = false;
/*
* Assess whether it's feasible to use parallel mode for this query. We
***************
*** 417,423 **** standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
result->nParamExec = glob->nParamExec;
result->hasRowSecurity = glob->hasRowSecurity;
result->parallelModeNeeded = glob->parallelModeNeeded;
! result->hasForeignJoin = glob->hasForeignJoin;
return result;
}
--- 417,423 ----
result->nParamExec = glob->nParamExec;
result->hasRowSecurity = glob->hasRowSecurity;
result->parallelModeNeeded = glob->parallelModeNeeded;
! result->usesUserMapping = glob->usesUserMapping;
return result;
}
*** a/src/backend/optimizer/util/relnode.c
--- b/src/backend/optimizer/util/relnode.c
***************
*** 16,21 ****
--- 16,22 ----
#include "miscadmin.h"
#include "catalog/pg_class.h"
+ #include "foreign/fdwapi.h"
#include "foreign/foreign.h"
#include "optimizer/clauses.h"
#include "optimizer/cost.h"
***************
*** 179,195 **** build_simple_rel(PlannerInfo *root, int relid, RelOptKind reloptkind)
* Note that if the plan ends up depending on the user OID in any way
* - e.g. if it depends on the computed user mapping OID - we must
* ensure that it gets invalidated in the case of a user OID change.
! * See RevalidateCachedQuery and more generally the hasForeignJoin
* flags in PlannerGlobal and PlannedStmt.
*
* It's possible, and not necessarily an error, for rel->umid to be
* InvalidOid even though rel->serverid is set. That just means there
* is a server with no user mapping.
*/
! Oid userid;
! userid = OidIsValid(rte->checkAsUser) ? rte->checkAsUser : GetUserId();
! rel->umid = GetUserMappingId(userid, rel->serverid, true);
}
else
rel->umid = InvalidOid;
--- 180,200 ----
* Note that if the plan ends up depending on the user OID in any way
* - e.g. if it depends on the computed user mapping OID - we must
* ensure that it gets invalidated in the case of a user OID change.
! * See RevalidateCachedQuery and more generally the usesUserMapping
* flags in PlannerGlobal and PlannedStmt.
*
* It's possible, and not necessarily an error, for rel->umid to be
* InvalidOid even though rel->serverid is set. That just means there
* is a server with no user mapping.
*/
! if (rel->fdwroutine->NeedUserMapping())
! {
! Oid userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
! rel->umid = GetUserMappingId(userid, rel->serverid);
! }
! else
! rel->umid = InvalidOid;
}
else
rel->umid = InvalidOid;
***************
*** 441,450 **** build_join_rel(PlannerInfo *root,
* Otherwise those fields are left invalid, so FDW API will not be called
* for the join relation.
*
! * For FDWs like file_fdw, which ignore user mapping, the user mapping id
! * associated with the joining relation may be invalid. A valid serverid
! * distinguishes between a pushed down join with no user mapping and a
! * join which can not be pushed down because of user mapping mismatch.
*/
if (OidIsValid(outer_rel->serverid) &&
inner_rel->serverid == outer_rel->serverid &&
--- 446,454 ----
* Otherwise those fields are left invalid, so FDW API will not be called
* for the join relation.
*
! * Note: if the FDW doesn't need user mappings, the user mapping OID is
! * InvalidOid. So, to give the FDW a chance to push down joins in that
! * case as well, allow combining when both sides have InvalidOid.
*/
if (OidIsValid(outer_rel->serverid) &&
inner_rel->serverid == outer_rel->serverid &&
*** a/src/backend/utils/cache/plancache.c
--- b/src/backend/utils/cache/plancache.c
***************
*** 122,128 **** InitPlanCache(void)
CacheRegisterSyscacheCallback(NAMESPACEOID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(OPEROID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(AMOPOPID, PlanCacheSysCallback, (Datum) 0);
! /* User mapping change may invalidate plans with pushed down foreign join */
CacheRegisterSyscacheCallback(USERMAPPINGOID, PlanCacheUserMappingCallback, (Datum) 0);
}
--- 122,128 ----
CacheRegisterSyscacheCallback(NAMESPACEOID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(OPEROID, PlanCacheSysCallback, (Datum) 0);
CacheRegisterSyscacheCallback(AMOPOPID, PlanCacheSysCallback, (Datum) 0);
! /* User mapping change may invalidate plans with user mappings */
CacheRegisterSyscacheCallback(USERMAPPINGOID, PlanCacheUserMappingCallback, (Datum) 0);
}
***************
*** 610,624 **** RevalidateCachedQuery(CachedPlanSource *plansource)
plansource->is_valid = false;
/*
! * If we have a join pushed down to the foreign server and the current
! * user is different from the one for which the plan was created,
! * invalidate the generic plan since user mapping for the new user might
! * make the join unsafe to push down, or change which user mapping is
! * used.
*/
if (plansource->is_valid &&
plansource->gplan &&
! plansource->gplan->has_foreign_join &&
plansource->planUserId != GetUserId())
plansource->gplan->is_valid = false;
--- 610,622 ----
plansource->is_valid = false;
/*
! * If the plan uses user mappings and the current user is different from
! * the one for which the plan was created, force a replan since the user
! * mapping for the new user might change.
*/
if (plansource->is_valid &&
plansource->gplan &&
! plansource->gplan->uses_user_mapping &&
plansource->planUserId != GetUserId())
plansource->gplan->is_valid = false;
***************
*** 1011,1027 **** BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
plan->is_valid = true;
/*
! * Walk through the plist and set hasForeignJoin if any of the plans have
! * it set.
*/
! plan->has_foreign_join = false;
foreach(lc, plist)
{
PlannedStmt *plan_stmt = (PlannedStmt *) lfirst(lc);
if (IsA(plan_stmt, PlannedStmt))
! plan->has_foreign_join =
! plan->has_foreign_join || plan_stmt->hasForeignJoin;
}
/* assign generation number to new plan */
--- 1009,1025 ----
plan->is_valid = true;
/*
! * Walk through the plist and set uses_user_mapping if any of the plans
! * have it set.
*/
! plan->uses_user_mapping = false;
foreach(lc, plist)
{
PlannedStmt *plan_stmt = (PlannedStmt *) lfirst(lc);
if (IsA(plan_stmt, PlannedStmt))
! plan->uses_user_mapping =
! plan->uses_user_mapping || plan_stmt->usesUserMapping;
}
/* assign generation number to new plan */
***************
*** 1891,1897 **** PlanCacheSysCallback(Datum arg, int cacheid, uint32 hashvalue)
* PlanCacheUserMappingCallback
* Syscache inval callback function for user mapping cache invalidation.
*
! * Invalidates plans which have pushed down foreign joins.
*/
static void
PlanCacheUserMappingCallback(Datum arg, int cacheid, uint32 hashvalue)
--- 1889,1895 ----
* PlanCacheUserMappingCallback
* Syscache inval callback function for user mapping cache invalidation.
*
! * Invalidates plans which use user mappings.
*/
static void
PlanCacheUserMappingCallback(Datum arg, int cacheid, uint32 hashvalue)
***************
*** 1911,1922 **** PlanCacheUserMappingCallback(Datum arg, int cacheid, uint32 hashvalue)
continue;
/*
! * If the plan has pushed down foreign joins, those join may become
! * unsafe to push down because of user mapping changes. Invalidate
! * only the generic plan, since changes to user mapping do not
! * invalidate the parse tree.
*/
! if (plansource->gplan && plansource->gplan->has_foreign_join)
plansource->gplan->is_valid = false;
}
}
--- 1909,1918 ----
continue;
/*
! * Invalidate only the generic plan, since changes to user mapping do
! * not invalidate the parse tree.
*/
! if (plansource->gplan && plansource->gplan->uses_user_mapping)
plansource->gplan->is_valid = false;
}
}
*** a/src/include/foreign/fdwapi.h
--- b/src/include/foreign/fdwapi.h
***************
*** 52,57 **** typedef void (*ReScanForeignScan_function) (ForeignScanState *node);
--- 52,59 ----
typedef void (*EndForeignScan_function) (ForeignScanState *node);
+ typedef bool (*NeedUserMapping_function) (void);
+
typedef void (*GetForeignJoinPaths_function) (PlannerInfo *root,
RelOptInfo *joinrel,
RelOptInfo *outerrel,
***************
*** 177,182 **** typedef struct FdwRoutine
--- 179,185 ----
IterateForeignScan_function IterateForeignScan;
ReScanForeignScan_function ReScanForeignScan;
EndForeignScan_function EndForeignScan;
+ NeedUserMapping_function NeedUserMapping;
/*
* Remaining functions are optional. Set the pointer to NULL for any that
*** a/src/include/foreign/foreign.h
--- b/src/include/foreign/foreign.h
***************
*** 72,78 **** typedef struct ForeignTable
extern ForeignServer *GetForeignServer(Oid serverid);
extern ForeignServer *GetForeignServerByName(const char *name, bool missing_ok);
extern UserMapping *GetUserMapping(Oid userid, Oid serverid);
! extern Oid GetUserMappingId(Oid userid, Oid serverid, bool missing_ok);
extern UserMapping *GetUserMappingById(Oid umid);
extern ForeignDataWrapper *GetForeignDataWrapper(Oid fdwid);
extern ForeignDataWrapper *GetForeignDataWrapperByName(const char *name,
--- 72,78 ----
extern ForeignServer *GetForeignServer(Oid serverid);
extern ForeignServer *GetForeignServerByName(const char *name, bool missing_ok);
extern UserMapping *GetUserMapping(Oid userid, Oid serverid);
! extern Oid GetUserMappingId(Oid userid, Oid serverid);
extern UserMapping *GetUserMappingById(Oid umid);
extern ForeignDataWrapper *GetForeignDataWrapper(Oid fdwid);
extern ForeignDataWrapper *GetForeignDataWrapperByName(const char *name,
*** a/src/include/nodes/plannodes.h
--- b/src/include/nodes/plannodes.h
***************
*** 73,79 **** typedef struct PlannedStmt
bool hasRowSecurity; /* row security applied? */
bool parallelModeNeeded; /* parallel mode required to execute? */
! bool hasForeignJoin; /* Plan has a pushed down foreign join */
} PlannedStmt;
/* macro for fetching the Plan associated with a SubPlan node */
--- 73,80 ----
bool hasRowSecurity; /* row security applied? */
bool parallelModeNeeded; /* parallel mode required to execute? */
!
! bool usesUserMapping; /* does use any user mapping */
} PlannedStmt;
/* macro for fetching the Plan associated with a SubPlan node */
***************
*** 534,539 **** typedef struct ForeignScan
--- 535,541 ----
Scan scan;
CmdType operation; /* SELECT/INSERT/UPDATE/DELETE */
Oid fs_server; /* OID of foreign server */
+ Oid fs_usermapping; /* OID of user mapping */
List *fdw_exprs; /* expressions that FDW may evaluate */
List *fdw_private; /* private data for FDW */
List *fdw_scan_tlist; /* optional tlist describing scan tuple */
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
***************
*** 127,133 **** typedef struct PlannerGlobal
bool parallelModeNeeded; /* parallel mode actually required? */
! bool hasForeignJoin; /* does have a pushed down foreign join */
} PlannerGlobal;
/* macro for fetching the Plan associated with a SubPlan node */
--- 127,133 ----
bool parallelModeNeeded; /* parallel mode actually required? */
! bool usesUserMapping; /* does use any user mapping */
} PlannerGlobal;
/* macro for fetching the Plan associated with a SubPlan node */
*** a/src/include/utils/plancache.h
--- b/src/include/utils/plancache.h
***************
*** 135,141 **** typedef struct CachedPlan
* changes from this value */
int generation; /* parent's generation number for this plan */
int refcount; /* count of live references to this struct */
! bool has_foreign_join; /* plan has pushed down a foreign join */
MemoryContext context; /* context containing this CachedPlan */
} CachedPlan;
--- 135,141 ----
* changes from this value */
int generation; /* parent's generation number for this plan */
int refcount; /* count of live references to this struct */
! bool uses_user_mapping; /* does CachedPlan use any user mapping */
MemoryContext context; /* context containing this CachedPlan */
} CachedPlan;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
