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

Commit5043193

Browse files
committed
Allow FDWs to push down quals without breaking EvalPlanQual rechecks.
This fixes a long-standing bug which was discovered while investigatingthe interaction between the new join pushdown code and the EvalPlanQualmachinery: if a ForeignScan appears on the inner side of a paramaterizednestloop, an EPQ recheck would re-return the original tuple even ifit no longer satisfied the pushed-down quals due to changed parametervalues.This fix adds a new member to ForeignScan and ForeignScanState and anew argument to make_foreignscan, and requires changes to FDWs whichpush down quals to populate that new argument with a list of quals theyhave chosen to push down. Therefore, I'm only back-patching to 9.5,even though the bug is not new in 9.5.Etsuro Fujita, reviewed by me and by Kyotaro Horiguchi.
1 parent54e07be commit5043193

File tree

12 files changed

+70
-13
lines changed

12 files changed

+70
-13
lines changed

‎contrib/file_fdw/file_fdw.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,8 @@ fileGetForeignPlan(PlannerInfo *root,
563563
scan_relid,
564564
NIL,/* no expressions to evaluate */
565565
best_path->fdw_private,
566-
NIL/* no custom tlist */ );
566+
NIL,/* no custom tlist */
567+
NIL/* no remote quals */ );
567568
}
568569

569570
/*

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,7 @@ postgresGetForeignPlan(PlannerInfo *root,
748748
Indexscan_relid=baserel->relid;
749749
List*fdw_private;
750750
List*remote_conds=NIL;
751+
List*remote_exprs=NIL;
751752
List*local_exprs=NIL;
752753
List*params_list=NIL;
753754
List*retrieved_attrs;
@@ -769,8 +770,8 @@ postgresGetForeignPlan(PlannerInfo *root,
769770
*
770771
* This code must match "extract_actual_clauses(scan_clauses, false)"
771772
* except for the additional decision about remote versus local execution.
772-
* Note however that weonly strip the RestrictInfo nodes from the
773-
*local_exprs list, since appendWhereClause expects a list of
773+
* Note however that wedon't strip the RestrictInfo nodes from the
774+
*remote_conds list, since appendWhereClause expects a list of
774775
* RestrictInfos.
775776
*/
776777
foreach(lc,scan_clauses)
@@ -784,11 +785,17 @@ postgresGetForeignPlan(PlannerInfo *root,
784785
continue;
785786

786787
if (list_member_ptr(fpinfo->remote_conds,rinfo))
788+
{
787789
remote_conds=lappend(remote_conds,rinfo);
790+
remote_exprs=lappend(remote_exprs,rinfo->clause);
791+
}
788792
elseif (list_member_ptr(fpinfo->local_conds,rinfo))
789793
local_exprs=lappend(local_exprs,rinfo->clause);
790794
elseif (is_foreign_expr(root,baserel,rinfo->clause))
795+
{
791796
remote_conds=lappend(remote_conds,rinfo);
797+
remote_exprs=lappend(remote_exprs,rinfo->clause);
798+
}
792799
else
793800
local_exprs=lappend(local_exprs,rinfo->clause);
794801
}
@@ -874,7 +881,8 @@ postgresGetForeignPlan(PlannerInfo *root,
874881
scan_relid,
875882
params_list,
876883
fdw_private,
877-
NIL/* no custom tlist */ );
884+
NIL,/* no custom tlist */
885+
remote_exprs);
878886
}
879887

880888
/*

‎doc/src/sgml/fdwhandler.sgml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,6 +1135,15 @@ GetForeignServerByName(const char *name, bool missing_ok);
11351135
evaluation of the <structfield>fdw_exprs</> expression tree.
11361136
</para>
11371137

1138+
<para>
1139+
Any clauses removed from the plan node's qual list must instead be added
1140+
to <literal>fdw_recheck_quals</> in order to ensure correct behavior
1141+
at the <literal>READ COMMITTED</> isolation level. When a concurrent
1142+
update occurs for some other table involved in the query, the executor
1143+
may need to verify that all of the original quals are still satisfied for
1144+
the tuple, possibly against a different set of parameter values.
1145+
</para>
1146+
11381147
<para>
11391148
Another <structname>ForeignScan</> field that can be filled by FDWs
11401149
is <structfield>fdw_scan_tlist</>, which describes the tuples returned by

‎src/backend/executor/nodeForeignscan.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include"executor/executor.h"
2626
#include"executor/nodeForeignscan.h"
2727
#include"foreign/fdwapi.h"
28+
#include"utils/memutils.h"
2829
#include"utils/rel.h"
2930

3031
staticTupleTableSlot*ForeignNext(ForeignScanState*node);
@@ -72,8 +73,19 @@ ForeignNext(ForeignScanState *node)
7273
staticbool
7374
ForeignRecheck(ForeignScanState*node,TupleTableSlot*slot)
7475
{
75-
/* There are no access-method-specific conditions to recheck. */
76-
return true;
76+
ExprContext*econtext;
77+
78+
/*
79+
* extract necessary information from foreign scan node
80+
*/
81+
econtext=node->ss.ps.ps_ExprContext;
82+
83+
/* Does the tuple meet the remote qual condition? */
84+
econtext->ecxt_scantuple=slot;
85+
86+
ResetExprContext(econtext);
87+
88+
returnExecQual(node->fdw_recheck_quals,econtext, false);
7789
}
7890

7991
/* ----------------------------------------------------------------
@@ -135,6 +147,9 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
135147
scanstate->ss.ps.qual= (List*)
136148
ExecInitExpr((Expr*)node->scan.plan.qual,
137149
(PlanState*)scanstate);
150+
scanstate->fdw_recheck_quals= (List*)
151+
ExecInitExpr((Expr*)node->fdw_recheck_quals,
152+
(PlanState*)scanstate);
138153

139154
/*
140155
* tuple table initialization

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,7 @@ _copyForeignScan(const ForeignScan *from)
624624
COPY_NODE_FIELD(fdw_exprs);
625625
COPY_NODE_FIELD(fdw_private);
626626
COPY_NODE_FIELD(fdw_scan_tlist);
627+
COPY_NODE_FIELD(fdw_recheck_quals);
627628
COPY_BITMAPSET_FIELD(fs_relids);
628629
COPY_SCALAR_FIELD(fsSystemCol);
629630

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,7 @@ _outForeignScan(StringInfo str, const ForeignScan *node)
579579
WRITE_NODE_FIELD(fdw_exprs);
580580
WRITE_NODE_FIELD(fdw_private);
581581
WRITE_NODE_FIELD(fdw_scan_tlist);
582+
WRITE_NODE_FIELD(fdw_recheck_quals);
582583
WRITE_BITMAPSET_FIELD(fs_relids);
583584
WRITE_BOOL_FIELD(fsSystemCol);
584585
}

‎src/backend/optimizer/plan/createplan.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2117,6 +2117,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
21172117
replace_nestloop_params(root, (Node*)scan_plan->scan.plan.qual);
21182118
scan_plan->fdw_exprs= (List*)
21192119
replace_nestloop_params(root, (Node*)scan_plan->fdw_exprs);
2120+
scan_plan->fdw_recheck_quals= (List*)
2121+
replace_nestloop_params(root,
2122+
(Node*)scan_plan->fdw_recheck_quals);
21202123
}
21212124

21222125
/*
@@ -3702,7 +3705,8 @@ make_foreignscan(List *qptlist,
37023705
Indexscanrelid,
37033706
List*fdw_exprs,
37043707
List*fdw_private,
3705-
List*fdw_scan_tlist)
3708+
List*fdw_scan_tlist,
3709+
List*fdw_recheck_quals)
37063710
{
37073711
ForeignScan*node=makeNode(ForeignScan);
37083712
Plan*plan=&node->scan.plan;
@@ -3718,6 +3722,7 @@ make_foreignscan(List *qptlist,
37183722
node->fdw_exprs=fdw_exprs;
37193723
node->fdw_private=fdw_private;
37203724
node->fdw_scan_tlist=fdw_scan_tlist;
3725+
node->fdw_recheck_quals=fdw_recheck_quals;
37213726
/* fs_relids will be filled in by create_foreignscan_plan */
37223727
node->fs_relids=NULL;
37233728
/* fsSystemCol will be filled in by create_foreignscan_plan */

‎src/backend/optimizer/plan/setrefs.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1127,13 +1127,15 @@ set_foreignscan_references(PlannerInfo *root,
11271127
}
11281128
else
11291129
{
1130-
/* Adjust tlist, qual, fdw_exprs in the standard way */
1130+
/* Adjust tlist, qual, fdw_exprs, etc. in the standard way */
11311131
fscan->scan.plan.targetlist=
11321132
fix_scan_list(root,fscan->scan.plan.targetlist,rtoffset);
11331133
fscan->scan.plan.qual=
11341134
fix_scan_list(root,fscan->scan.plan.qual,rtoffset);
11351135
fscan->fdw_exprs=
11361136
fix_scan_list(root,fscan->fdw_exprs,rtoffset);
1137+
fscan->fdw_recheck_quals=
1138+
fix_scan_list(root,fscan->fdw_recheck_quals,rtoffset);
11371139
}
11381140

11391141
/* Adjust fs_relids if needed */

‎src/backend/optimizer/plan/subselect.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2371,10 +2371,18 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
23712371
break;
23722372

23732373
caseT_ForeignScan:
2374-
finalize_primnode((Node*) ((ForeignScan*)plan)->fdw_exprs,
2375-
&context);
2376-
/* We assume fdw_scan_tlist cannot contain Params */
2377-
context.paramids=bms_add_members(context.paramids,scan_params);
2374+
{
2375+
ForeignScan*fscan= (ForeignScan*)plan;
2376+
2377+
finalize_primnode((Node*)fscan->fdw_exprs,
2378+
&context);
2379+
finalize_primnode((Node*)fscan->fdw_recheck_quals,
2380+
&context);
2381+
2382+
/* We assume fdw_scan_tlist cannot contain Params */
2383+
context.paramids=bms_add_members(context.paramids,
2384+
scan_params);
2385+
}
23782386
break;
23792387

23802388
caseT_CustomScan:

‎src/include/nodes/execnodes.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,6 +1580,7 @@ typedef struct WorkTableScanState
15801580
typedefstructForeignScanState
15811581
{
15821582
ScanStatess;/* its first field is NodeTag */
1583+
List*fdw_recheck_quals;/* original quals not in ss.ps.qual */
15831584
/* use struct pointer to avoid including fdwapi.h here */
15841585
structFdwRoutine*fdwroutine;
15851586
void*fdw_state;/* foreign-data wrapper can keep state here */

‎src/include/nodes/plannodes.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,11 @@ typedef struct WorkTableScan
509509
* fdw_scan_tlist is never actually executed; it just holds expression trees
510510
* describing what is in the scan tuple's columns.
511511
*
512+
* fdw_recheck_quals should contain any quals which the core system passed to
513+
* the FDW but which were not added to scan.plan.quals; that is, it should
514+
* contain the quals being checked remotely. This is needed for correct
515+
* behavior during EvalPlanQual rechecks.
516+
*
512517
* When the plan node represents a foreign join, scan.scanrelid is zero and
513518
* fs_relids must be consulted to identify the join relation. (fs_relids
514519
* is valid for simple scans as well, but will always match scan.scanrelid.)
@@ -521,6 +526,7 @@ typedef struct ForeignScan
521526
List*fdw_exprs;/* expressions that FDW may evaluate */
522527
List*fdw_private;/* private data for FDW */
523528
List*fdw_scan_tlist;/* optional tlist describing scan tuple */
529+
List*fdw_recheck_quals;/* original quals not in scan.plan.quals */
524530
Bitmapset*fs_relids;/* RTIs generated by this scan */
525531
boolfsSystemCol;/* true if any "system column" is needed */
526532
}ForeignScan;

‎src/include/optimizer/planmain.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ extern SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual,
4545
Indexscanrelid,Plan*subplan);
4646
externForeignScan*make_foreignscan(List*qptlist,List*qpqual,
4747
Indexscanrelid,List*fdw_exprs,List*fdw_private,
48-
List*fdw_scan_tlist);
48+
List*fdw_scan_tlist,List*fdw_recheck_quals);
4949
externAppend*make_append(List*appendplans,List*tlist);
5050
externRecursiveUnion*make_recursive_union(List*tlist,
5151
Plan*lefttree,Plan*righttree,intwtParam,

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp