Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit45639a0

Browse files
committed
Avoid invalidating all foreign-join cached plans when user mappings change.
We must not push down a foreign join when the foreign tables involvedshould be accessed under different user mappings. Previously we triedto enforce that rule literally during planning, but that meant that theresulting plans were dependent on the current contents of thepg_user_mapping catalog, and we had to blow away all cached planscontaining any remote join when anything at all changed in pg_user_mapping.This could have been improved somewhat, but the fact that a syscache invalcallback has very limited info about what changed made it hard to do betterwithin that design. Instead, let's change the planner to not consider usermappings per se, but to allow a foreign join if both RTEs have the samecheckAsUser value. If they do, then they necessarily will use the sameuser mapping at runtime, and we don't need to know specifically which onethat is. Post-plan-time changes in pg_user_mapping no longer require anyplan invalidation.This rule does give up some optimization ability, to wit where two foreigntable references come from views with different owners or one's from a viewand one's directly in the query, but nonetheless the same user mappingwould have applied. We'll sacrifice the first case, but to not regressmore than we have to in the second case, allow a foreign join involvingboth zero and nonzero checkAsUser values if the nonzero one is the same asthe prevailing effective userID. In that case, mark the plan as onlyrunnable by that userID.The plancache code already had a notion of plans being userID-specific,in order to support RLS. It was a little confused though, in particularlacking clarity of thought as to whether it was the rewritten query or justthe finished plan that's dependent on the userID. Rearrange that code sothat it's clearer what depends on which, and so that the same logic appliesto both RLS-injected role dependency and foreign-join-injected roledependency.Note that this patch doesn't remove the other issue mentioned in theoriginal complaint, which is that while we'll reliably stop using a foreignjoin if it's disallowed in a new context, we might fail to start using aforeign join if it's now allowed, but we previously created a genericcached plan that didn't use one. It was agreed that the chance of winningthat way was not high enough to justify the much larger number of planinvalidations that would have to occur if we tried to cause it to happen.In passing, clean up randomly-varying spelling of EXPLAIN commands inpostgres_fdw.sql, and fix a COSTS ON example that had been allowed toleak into the committed tests.This reverts most of commitsfbe5a3f and5d4171d, which were theprevious attempt at ensuring we wouldn't push down foreign joins thatspan permissions contexts.Etsuro Fujita and Tom LaneDiscussion: <d49c1e5b-f059-20f4-c132-e9752ee0113e@lab.ntt.co.jp>
1 parent533e9c6 commit45639a0

File tree

19 files changed

+579
-716
lines changed

19 files changed

+579
-716
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 240 additions & 252 deletions
Large diffs are not rendered by default.

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 39 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ enum FdwScanPrivateIndex
6767
FdwScanPrivateRetrievedAttrs,
6868
/* Integer representing the desired fetch_size */
6969
FdwScanPrivateFetchSize,
70-
/* Oid of user mapping to be used while connecting to the foreign server */
71-
FdwScanPrivateUserMappingOid,
7270

7371
/*
7472
* String describing join i.e. names of relations being joined and types
@@ -1226,11 +1224,10 @@ postgresGetForeignPlan(PlannerInfo *root,
12261224
* Build the fdw_private list that will be available to the executor.
12271225
* Items in the list must match order in enum FdwScanPrivateIndex.
12281226
*/
1229-
fdw_private=list_make5(makeString(sql.data),
1227+
fdw_private=list_make4(makeString(sql.data),
12301228
remote_conds,
12311229
retrieved_attrs,
1232-
makeInteger(fpinfo->fetch_size),
1233-
makeInteger(foreignrel->umid));
1230+
makeInteger(fpinfo->fetch_size));
12341231
if (foreignrel->reloptkind==RELOPT_JOINREL)
12351232
fdw_private=lappend(fdw_private,
12361233
makeString(fpinfo->relation_name->data));
@@ -1262,7 +1259,11 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
12621259
ForeignScan*fsplan= (ForeignScan*)node->ss.ps.plan;
12631260
EState*estate=node->ss.ps.state;
12641261
PgFdwScanState*fsstate;
1262+
RangeTblEntry*rte;
1263+
Oiduserid;
1264+
ForeignTable*table;
12651265
UserMapping*user;
1266+
intrtindex;
12661267
intnumParams;
12671268

12681269
/*
@@ -1278,36 +1279,20 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
12781279
node->fdw_state= (void*)fsstate;
12791280

12801281
/*
1281-
* Obtain the foreign server where to connect and user mapping to use for
1282-
* connection. For base relations we obtain this information from
1283-
* catalogs. For join relations, this information is frozen at the time of
1284-
* planning to ensure that the join is safe to pushdown. In case the
1285-
* information goes stale between planning and execution, plan will be
1286-
* invalidated and replanned.
1282+
* Identify which user to do the remote access as. This should match what
1283+
* ExecCheckRTEPerms() does. In case of a join, use the lowest-numbered
1284+
* member RTE as a representative; we would get the same result from any.
12871285
*/
12881286
if (fsplan->scan.scanrelid>0)
1289-
{
1290-
ForeignTable*table;
1291-
1292-
/*
1293-
* Identify which user to do the remote access as. This should match
1294-
* what ExecCheckRTEPerms() does.
1295-
*/
1296-
RangeTblEntry*rte=rt_fetch(fsplan->scan.scanrelid,estate->es_range_table);
1297-
Oiduserid=rte->checkAsUser ?rte->checkAsUser :GetUserId();
1298-
1299-
fsstate->rel=node->ss.ss_currentRelation;
1300-
table=GetForeignTable(RelationGetRelid(fsstate->rel));
1301-
1302-
user=GetUserMapping(userid,table->serverid);
1303-
}
1287+
rtindex=fsplan->scan.scanrelid;
13041288
else
1305-
{
1306-
Oidumid=intVal(list_nth(fsplan->fdw_private,FdwScanPrivateUserMappingOid));
1289+
rtindex=bms_next_member(fsplan->fs_relids,-1);
1290+
rte=rt_fetch(rtindex,estate->es_range_table);
1291+
userid=rte->checkAsUser ?rte->checkAsUser :GetUserId();
13071292

1308-
user=GetUserMappingById(umid);
1309-
Assert(fsplan->fs_server==user->serverid);
1310-
}
1293+
/* Get info about foreign table. */
1294+
table=GetForeignTable(rte->relid);
1295+
user=GetUserMapping(userid,table->serverid);
13111296

13121297
/*
13131298
* Get connection to the foreign server. Connection manager will
@@ -1344,9 +1329,15 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
13441329
* into local representation and error reporting during that process.
13451330
*/
13461331
if (fsplan->scan.scanrelid>0)
1332+
{
1333+
fsstate->rel=node->ss.ss_currentRelation;
13471334
fsstate->tupdesc=RelationGetDescr(fsstate->rel);
1335+
}
13481336
else
1337+
{
1338+
fsstate->rel=NULL;
13491339
fsstate->tupdesc=node->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
1340+
}
13501341

13511342
fsstate->attinmeta=TupleDescGetAttInMetadata(fsstate->tupdesc);
13521343

@@ -3965,16 +3956,6 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
39653956
List*joinclauses;
39663957
List*otherclauses;
39673958

3968-
/*
3969-
* Core code may call GetForeignJoinPaths hook even when the join relation
3970-
* doesn't have a valid user mapping associated with it. See
3971-
* build_join_rel() for details. We can't push down such join, since there
3972-
* doesn't exist a user mapping which can be used to connect to the
3973-
* foreign server.
3974-
*/
3975-
if (!OidIsValid(joinrel->umid))
3976-
return false;
3977-
39783959
/*
39793960
* We support pushing down INNER, LEFT, RIGHT and FULL OUTER joins.
39803961
* Constructing queries representing SEMI and ANTI joins is hard, hence
@@ -4151,6 +4132,20 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
41514132
fpinfo->use_remote_estimate=fpinfo_o->use_remote_estimate||
41524133
fpinfo_i->use_remote_estimate;
41534134

4135+
/* Get user mapping */
4136+
if (fpinfo->use_remote_estimate)
4137+
{
4138+
if (fpinfo_o->use_remote_estimate)
4139+
fpinfo->user=fpinfo_o->user;
4140+
else
4141+
fpinfo->user=fpinfo_i->user;
4142+
}
4143+
else
4144+
fpinfo->user=NULL;
4145+
4146+
/* Get foreign server */
4147+
fpinfo->server=fpinfo_o->server;
4148+
41544149
/*
41554150
* Since both the joining relations come from the same server, the server
41564151
* level options should have same value for both the relations. Pick from
@@ -4312,26 +4307,14 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
43124307
cost_qual_eval(&fpinfo->local_conds_cost,fpinfo->local_conds,root);
43134308

43144309
/*
4315-
* If we are going to estimatethecostsusing EXPLAIN, we will need
4316-
*connection information. Fill it here.
4310+
* If we are going to estimate costslocally, estimate the join clause
4311+
*selectivity here while we have special join info.
43174312
*/
4318-
if (fpinfo->use_remote_estimate)
4319-
fpinfo->user=GetUserMappingById(joinrel->umid);
4320-
else
4321-
{
4322-
fpinfo->user=NULL;
4323-
4324-
/*
4325-
* If we are going to estimate costs locally, estimate the join clause
4326-
* selectivity here while we have special join info.
4327-
*/
4313+
if (!fpinfo->use_remote_estimate)
43284314
fpinfo->joinclause_sel=clauselist_selectivity(root,fpinfo->joinclauses,
43294315
0,fpinfo->jointype,
43304316
extra->sjinfo);
43314317

4332-
}
4333-
fpinfo->server=GetForeignServer(joinrel->serverid);
4334-
43354318
/* Estimate costs for bare join relation */
43364319
estimate_path_cost_size(root,joinrel,NIL,NIL,&rows,
43374320
&width,&startup_cost,&total_cost);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp