Don't require a user mapping for FDWs to work.

Commit fbe5a3fb73 accidentally changed
this behavior; put things back the way they were, and add some
regression tests.

Report by Andres Freund; patch by Ashutosh Bapat, with a bit of
kibitzing by me.
This commit is contained in:
Robert Haas 2016-03-28 21:50:28 -04:00
parent 868628e4fd
commit 5d4171d1c7
8 changed files with 125 additions and 22 deletions

View File

@ -173,6 +173,9 @@ SET ROLE file_fdw_user;
\t on
EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_text WHERE a > 0;
\t off
-- file FDW allows foreign tables to be accessed without user mapping
DROP USER MAPPING FOR file_fdw_user SERVER file_server;
SELECT * FROM agg_text ORDER BY a;
-- privilege tests for object
SET ROLE file_fdw_superuser;

View File

@ -322,6 +322,17 @@ EXPLAIN (VERBOSE, COSTS FALSE) SELECT * FROM agg_text WHERE a > 0;
Foreign File: @abs_srcdir@/data/agg.data
\t off
-- file FDW allows foreign tables to be accessed without user mapping
DROP USER MAPPING FOR file_fdw_user SERVER file_server;
SELECT * FROM agg_text ORDER BY a;
a | b
-----+---------
0 | 0.09561
42 | 324.78
56 | 7.8
100 | 99.097
(4 rows)
-- privilege tests for object
SET ROLE file_fdw_superuser;
ALTER FOREIGN TABLE agg_text OWNER TO file_fdw_user;
@ -333,9 +344,8 @@ SET ROLE file_fdw_superuser;
-- cleanup
RESET ROLE;
DROP EXTENSION file_fdw CASCADE;
NOTICE: drop cascades to 8 other objects
NOTICE: drop cascades to 7 other objects
DETAIL: drop cascades to server file_server
drop cascades to user mapping for file_fdw_user on server file_server
drop cascades to user mapping for file_fdw_superuser on server file_server
drop cascades to user mapping for no_priv_user on server file_server
drop cascades to foreign table agg_text

View File

@ -1958,13 +1958,30 @@ 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.
-- While creating the plan, it should throw error since there is no user mapping
-- available for view_owner.
-- 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;
ERROR: user mapping not found for "view_owner"
EXECUTE join_stmt;
ERROR: user mapping not found for "view_owner"
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;
@ -2021,6 +2038,40 @@ EXECUTE join_stmt;
----+----
(0 rows)
-- 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);
QUERY PLAN
------------------------------------------------------------------
Hash Join
Output: t1.c1, ft5.c1
Hash Cond: (t1.c1 = ft5.c1)
-> Hash Right Join
Output: t1.c1
Hash Cond: (t3.c1 = t1.c1)
-> Hash Join
Output: t3.c1
Hash Cond: (t3.c1 = ft5_1.c1)
-> Foreign Scan on public.ft5 t3
Output: t3.c1, t3.c2, t3.c3
Remote SQL: SELECT c1 FROM "S 1"."T 4"
-> Hash
Output: ft5_1.c1
-> Foreign Scan on public.ft5 ft5_1
Output: ft5_1.c1
Remote SQL: SELECT c1 FROM "S 1"."T 4"
-> Hash
Output: t1.c1
-> Foreign Scan on public.ft5 t1
Output: t1.c1
Remote SQL: SELECT c1 FROM "S 1"."T 4"
-> Hash
Output: ft5.c1
-> Foreign Scan on public.ft5
Output: ft5.c1
Remote SQL: SELECT c1 FROM "S 1"."T 4"
(27 rows)
-- recreate the dropped user mapping for further tests
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
DROP USER MAPPING FOR PUBLIC SERVER loopback;

View File

@ -3910,6 +3910,16 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
List *joinclauses;
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

View File

@ -478,11 +478,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.
-- While creating the plan, it should throw error since there is no user mapping
-- available for view_owner.
-- 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;
EXECUTE join_stmt;
RESET ROLE;
DEALLOCATE join_stmt;
@ -506,6 +505,10 @@ CREATE USER MAPPING FOR view_owner SERVER loopback;
EXPLAIN (COSTS false, VERBOSE) EXECUTE join_stmt;
EXECUTE join_stmt;
-- 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);
-- recreate the dropped user mapping for further tests
CREATE USER MAPPING FOR CURRENT_USER SERVER loopback;
DROP USER MAPPING FOR PUBLIC SERVER loopback;

View File

@ -31,7 +31,7 @@
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);
static HeapTuple find_user_mapping(Oid userid, Oid serverid, bool missing_ok);
/*
* GetForeignDataWrapper - look up the foreign-data wrapper by OID.
@ -223,7 +223,7 @@ GetUserMapping(Oid userid, Oid serverid)
bool isnull;
UserMapping *um;
tp = find_user_mapping(userid, serverid);
tp = find_user_mapping(userid, serverid, false);
um = (UserMapping *) palloc(sizeof(UserMapping));
um->umid = HeapTupleGetOid(tp);
@ -250,14 +250,23 @@ 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)
GetUserMappingId(Oid userid, Oid serverid, bool missing_ok)
{
HeapTuple tp;
Oid umid;
tp = find_user_mapping(userid, serverid);
tp = find_user_mapping(userid, serverid, missing_ok);
Assert(missing_ok || tp);
if (!tp && missing_ok)
return InvalidOid;
/* Extract the Oid */
umid = HeapTupleGetOid(tp);
@ -273,9 +282,13 @@ GetUserMappingId(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 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)
find_user_mapping(Oid userid, Oid serverid, bool missing_ok)
{
HeapTuple tp;
@ -292,10 +305,15 @@ find_user_mapping(Oid userid, Oid serverid)
ObjectIdGetDatum(serverid));
if (!HeapTupleIsValid(tp))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("user mapping not found for \"%s\"",
MappingUserName(userid))));
{
if (missing_ok)
return NULL;
else
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("user mapping not found for \"%s\"",
MappingUserName(userid))));
}
return tp;
}

View File

@ -180,11 +180,15 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptKind reloptkind)
* 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);
rel->umid = GetUserMappingId(userid, rel->serverid, true);
}
else
rel->umid = InvalidOid;
@ -435,12 +439,16 @@ 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 &&
inner_rel->umid == outer_rel->umid)
{
Assert(OidIsValid(outer_rel->umid));
joinrel->serverid = outer_rel->serverid;
joinrel->umid = outer_rel->umid;
joinrel->fdwroutine = outer_rel->fdwroutine;

View File

@ -72,7 +72,7 @@ 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);
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,