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

Commit2489d76

Browse files
committed
Make Vars be outer-join-aware.
Traditionally we used the same Var struct to represent the valueof a table column everywhere in parse and plan trees. This choicepredates our support for SQL outer joins, and it's really a prettybad idea with outer joins, because the Var's value can depend onwhere it is in the tree: it might go to NULL above an outer join.So expression nodes that are equal() per equalfuncs.c might notrepresent the same value, which is a huge correctness hazard forthe planner.To improve this, decorate Var nodes with a bitmapset showingwhich outer joins (identified by RTE indexes) may have nulledthem at the point in the parse tree where the Var appears.This allows us to trust that equal() Vars represent the same value.A certain amount of klugery is still needed to cope with caseswhere we re-order two outer joins, but it's possible to make itwork without sacrificing that core principle. PlaceHolderVarsreceive similar decoration for the same reason.In the planner, we include these outer join bitmapsets into the relidsthat an expression is considered to depend on, and in consequence alsoadd outer-join relids to the relids of join RelOptInfos. This allowsus to correctly perceive whether an expression can be calculated aboveor below a particular outer join.This change affects FDWs that want to plan foreign joins. They *must*follow suit when labeling foreign joins in order to match with thecore planner, but for many purposes (if postgres_fdw is any guide)they'd prefer to consider only base relations within the join.To support both requirements, redefine ForeignScan.fs_relids asbase+OJ relids, and add a new field fs_base_relids that's set up bythe core planner.Large though it is, this commit just does the minimum necessary toinstall the new mechanisms and get check-world passing again.Follow-up patches will perform some cleanup. (The README additionsand comments mention some stuff that will appear in the follow-up.)Patch by me; thanks to Richard Guo for review.Discussion:https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
1 parentec7e053 commit2489d76

File tree

60 files changed

+3878
-966
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+3878
-966
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4026,7 +4026,17 @@ get_relation_column_alias_ids(Var *node, RelOptInfo *foreignrel,
40264026
i=1;
40274027
foreach(lc,foreignrel->reltarget->exprs)
40284028
{
4029-
if (equal(lfirst(lc), (Node*)node))
4029+
Var*tlvar= (Var*)lfirst(lc);
4030+
4031+
/*
4032+
* Match reltarget entries only on varno/varattno. Ideally there
4033+
* would be some cross-check on varnullingrels, but it's unclear what
4034+
* to do exactly; we don't have enough context to know what that value
4035+
* should be.
4036+
*/
4037+
if (IsA(tlvar,Var)&&
4038+
tlvar->varno==node->varno&&
4039+
tlvar->varattno==node->varattno)
40304040
{
40314041
*colno=i;
40324042
return;

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,7 +1517,7 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
15171517
if (fsplan->scan.scanrelid>0)
15181518
rtindex=fsplan->scan.scanrelid;
15191519
else
1520-
rtindex=bms_next_member(fsplan->fs_relids,-1);
1520+
rtindex=bms_next_member(fsplan->fs_base_relids,-1);
15211521
rte=exec_rt_fetch(rtindex,estate);
15221522

15231523
/* Get info about foreign table. */
@@ -2414,7 +2414,7 @@ find_modifytable_subplan(PlannerInfo *root,
24142414
{
24152415
ForeignScan*fscan= (ForeignScan*)subplan;
24162416

2417-
if (bms_is_member(rtindex,fscan->fs_relids))
2417+
if (bms_is_member(rtindex,fscan->fs_base_relids))
24182418
returnfscan;
24192419
}
24202420

@@ -2838,8 +2838,8 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
28382838
* that setrefs.c won't update the string when flattening the
28392839
* rangetable. To find out what rtoffset was applied, identify the
28402840
* minimum RT index appearing in the string and compare it to the
2841-
* minimum member of plan->fs_relids. (We expect all the relids in
2842-
* the join will have been offset by the same amount; the Asserts
2841+
* minimum member of plan->fs_base_relids. (We expect all the relids
2842+
*inthe join will have been offset by the same amount; the Asserts
28432843
* below should catch it if that ever changes.)
28442844
*/
28452845
minrti=INT_MAX;
@@ -2856,7 +2856,7 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
28562856
else
28572857
ptr++;
28582858
}
2859-
rtoffset=bms_next_member(plan->fs_relids,-1)-minrti;
2859+
rtoffset=bms_next_member(plan->fs_base_relids,-1)-minrti;
28602860

28612861
/* Now we can translate the string */
28622862
relations=makeStringInfo();
@@ -2871,7 +2871,7 @@ postgresExplainForeignScan(ForeignScanState *node, ExplainState *es)
28712871
char*refname;
28722872

28732873
rti+=rtoffset;
2874-
Assert(bms_is_member(rti,plan->fs_relids));
2874+
Assert(bms_is_member(rti,plan->fs_base_relids));
28752875
rte=rt_fetch(rti,es->rtable);
28762876
Assert(rte->rtekind==RTE_RELATION);
28772877
/* This logic should agree with explain.c's ExplainTargetRel */

‎doc/src/sgml/fdwhandler.sgml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,17 @@ GetForeignJoinPaths(PlannerInfo *root,
351351
it will supply at run time in the tuples it returns.
352352
</para>
353353

354+
<note>
355+
<para>
356+
Beginning with <productname>PostgreSQL</productname> 16,
357+
<structfield>fs_relids</structfield> includes the rangetable indexes
358+
of outer joins, if any were involved in this join. The new field
359+
<structfield>fs_base_relids</structfield> includes only base
360+
relation indexes, and thus
361+
mimics <structfield>fs_relids</structfield>'s old semantics.
362+
</para>
363+
</note>
364+
354365
<para>
355366
See <xref linkend="fdw-planning"/> for additional information.
356367
</para>

‎src/backend/commands/explain.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1114,7 +1114,7 @@ ExplainPreScanNode(PlanState *planstate, Bitmapset **rels_used)
11141114
break;
11151115
caseT_ForeignScan:
11161116
*rels_used=bms_add_members(*rels_used,
1117-
((ForeignScan*)plan)->fs_relids);
1117+
((ForeignScan*)plan)->fs_base_relids);
11181118
break;
11191119
caseT_CustomScan:
11201120
*rels_used=bms_add_members(*rels_used,

‎src/backend/executor/execScan.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ ExecScanReScan(ScanState *node)
325325
* all of them.
326326
*/
327327
if (IsA(node->ps.plan,ForeignScan))
328-
relids= ((ForeignScan*)node->ps.plan)->fs_relids;
328+
relids= ((ForeignScan*)node->ps.plan)->fs_base_relids;
329329
elseif (IsA(node->ps.plan,CustomScan))
330330
relids= ((CustomScan*)node->ps.plan)->custom_relids;
331331
else

‎src/backend/nodes/makefuncs.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,13 @@ makeVar(int varno,
8080
var->varlevelsup=varlevelsup;
8181

8282
/*
83-
* Only a few callers need to make Var nodes with varnosyn/varattnosyn
84-
* different from varno/varattno. We don't provide separate arguments for
85-
* them, but just initialize them to the given varno/varattno. This
86-
* reduces code clutter and chance of error for most callers.
83+
* Only a few callers need to make Var nodes with non-null varnullingrels,
84+
* or with varnosyn/varattnosyn different from varno/varattno. We don't
85+
* provide separate arguments for them, but just initialize them to NULL
86+
* and the given varno/varattno. This reduces code clutter and chance of
87+
* error for most callers.
8788
*/
89+
var->varnullingrels=NULL;
8890
var->varnosyn= (Index)varno;
8991
var->varattnosyn=varattno;
9092

‎src/backend/nodes/nodeFuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2641,6 +2641,7 @@ expression_tree_mutator_impl(Node *node,
26412641
Var*newnode;
26422642

26432643
FLATCOPY(newnode,var,Var);
2644+
/* Assume we need not copy the varnullingrels bitmapset */
26442645
return (Node*)newnode;
26452646
}
26462647
break;
@@ -3234,7 +3235,7 @@ expression_tree_mutator_impl(Node *node,
32343235

32353236
FLATCOPY(newnode,phv,PlaceHolderVar);
32363237
MUTATE(newnode->phexpr,phv->phexpr,Expr*);
3237-
/* Assume we need not copy the relidsbitmapset */
3238+
/* Assume we need not copy the relidsbitmapsets */
32383239
return (Node*)newnode;
32393240
}
32403241
break;

‎src/backend/nodes/queryjumblefuncs.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ JumbleExpr(JumbleState *jstate, Node *node)
383383
APP_JUMB(var->varno);
384384
APP_JUMB(var->varattno);
385385
APP_JUMB(var->varlevelsup);
386+
387+
/*
388+
* We can omit varnullingrels, because it's fully determined
389+
* by varno/varlevelsup plus the Var's query location.
390+
*/
386391
}
387392
break;
388393
caseT_Const:

0 commit comments

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