diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 499f24ff28..5ce8f90cd8 100644 --- a/contrib/file_fdw/file_fdw.c +++ b/contrib/file_fdw/file_fdw.c @@ -563,7 +563,8 @@ fileGetForeignPlan(PlannerInfo *root, scan_relid, NIL, /* no expressions to evaluate */ best_path->fdw_private, - NIL /* no custom tlist */ ); + NIL, /* no custom tlist */ + NIL /* no remote quals */ ); } /* diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index e4d799cecd..1902f1f4ea 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -748,6 +748,7 @@ postgresGetForeignPlan(PlannerInfo *root, Index scan_relid = baserel->relid; List *fdw_private; List *remote_conds = NIL; + List *remote_exprs = NIL; List *local_exprs = NIL; List *params_list = NIL; List *retrieved_attrs; @@ -769,8 +770,8 @@ postgresGetForeignPlan(PlannerInfo *root, * * This code must match "extract_actual_clauses(scan_clauses, false)" * except for the additional decision about remote versus local execution. - * Note however that we only strip the RestrictInfo nodes from the - * local_exprs list, since appendWhereClause expects a list of + * Note however that we don't strip the RestrictInfo nodes from the + * remote_conds list, since appendWhereClause expects a list of * RestrictInfos. */ foreach(lc, scan_clauses) @@ -784,11 +785,17 @@ postgresGetForeignPlan(PlannerInfo *root, continue; if (list_member_ptr(fpinfo->remote_conds, rinfo)) + { remote_conds = lappend(remote_conds, rinfo); + remote_exprs = lappend(remote_exprs, rinfo->clause); + } else if (list_member_ptr(fpinfo->local_conds, rinfo)) local_exprs = lappend(local_exprs, rinfo->clause); else if (is_foreign_expr(root, baserel, rinfo->clause)) + { remote_conds = lappend(remote_conds, rinfo); + remote_exprs = lappend(remote_exprs, rinfo->clause); + } else local_exprs = lappend(local_exprs, rinfo->clause); } @@ -874,7 +881,8 @@ postgresGetForeignPlan(PlannerInfo *root, scan_relid, params_list, fdw_private, - NIL /* no custom tlist */ ); + NIL, /* no custom tlist */ + remote_exprs); } /* diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 4c410c7916..1533a6bf80 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -1135,6 +1135,15 @@ GetForeignServerByName(const char *name, bool missing_ok); evaluation of the fdw_exprs expression tree. + + Any clauses removed from the plan node's qual list must instead be added + to fdw_recheck_quals in order to ensure correct behavior + at the READ COMMITTED isolation level. When a concurrent + update occurs for some other table involved in the query, the executor + may need to verify that all of the original quals are still satisfied for + the tuple, possibly against a different set of parameter values. + + Another ForeignScan field that can be filled by FDWs is fdw_scan_tlist, which describes the tuples returned by diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index bb28a7372d..6165e4a6cb 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -25,6 +25,7 @@ #include "executor/executor.h" #include "executor/nodeForeignscan.h" #include "foreign/fdwapi.h" +#include "utils/memutils.h" #include "utils/rel.h" static TupleTableSlot *ForeignNext(ForeignScanState *node); @@ -72,8 +73,19 @@ ForeignNext(ForeignScanState *node) static bool ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot) { - /* There are no access-method-specific conditions to recheck. */ - return true; + ExprContext *econtext; + + /* + * extract necessary information from foreign scan node + */ + econtext = node->ss.ps.ps_ExprContext; + + /* Does the tuple meet the remote qual condition? */ + econtext->ecxt_scantuple = slot; + + ResetExprContext(econtext); + + return ExecQual(node->fdw_recheck_quals, econtext, false); } /* ---------------------------------------------------------------- @@ -135,6 +147,9 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags) scanstate->ss.ps.qual = (List *) ExecInitExpr((Expr *) node->scan.plan.qual, (PlanState *) scanstate); + scanstate->fdw_recheck_quals = (List *) + ExecInitExpr((Expr *) node->fdw_recheck_quals, + (PlanState *) scanstate); /* * tuple table initialization diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 73240406ba..e19fee4c31 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -624,6 +624,7 @@ _copyForeignScan(const ForeignScan *from) COPY_NODE_FIELD(fdw_exprs); COPY_NODE_FIELD(fdw_private); COPY_NODE_FIELD(fdw_scan_tlist); + COPY_NODE_FIELD(fdw_recheck_quals); COPY_BITMAPSET_FIELD(fs_relids); COPY_SCALAR_FIELD(fsSystemCol); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 991b4c2175..b39c772209 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -579,6 +579,7 @@ _outForeignScan(StringInfo str, const ForeignScan *node) WRITE_NODE_FIELD(fdw_exprs); WRITE_NODE_FIELD(fdw_private); WRITE_NODE_FIELD(fdw_scan_tlist); + WRITE_NODE_FIELD(fdw_recheck_quals); WRITE_BITMAPSET_FIELD(fs_relids); WRITE_BOOL_FIELD(fsSystemCol); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index f461586e08..224bf3d147 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2117,6 +2117,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path, replace_nestloop_params(root, (Node *) scan_plan->scan.plan.qual); scan_plan->fdw_exprs = (List *) replace_nestloop_params(root, (Node *) scan_plan->fdw_exprs); + scan_plan->fdw_recheck_quals = (List *) + replace_nestloop_params(root, + (Node *) scan_plan->fdw_recheck_quals); } /* @@ -3702,7 +3705,8 @@ make_foreignscan(List *qptlist, Index scanrelid, List *fdw_exprs, List *fdw_private, - List *fdw_scan_tlist) + List *fdw_scan_tlist, + List *fdw_recheck_quals) { ForeignScan *node = makeNode(ForeignScan); Plan *plan = &node->scan.plan; @@ -3718,6 +3722,7 @@ make_foreignscan(List *qptlist, node->fdw_exprs = fdw_exprs; node->fdw_private = fdw_private; node->fdw_scan_tlist = fdw_scan_tlist; + node->fdw_recheck_quals = fdw_recheck_quals; /* fs_relids will be filled in by create_foreignscan_plan */ node->fs_relids = NULL; /* fsSystemCol will be filled in by create_foreignscan_plan */ diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index e6fbe6acde..e1e1d7ab4b 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -1127,13 +1127,15 @@ set_foreignscan_references(PlannerInfo *root, } else { - /* Adjust tlist, qual, fdw_exprs in the standard way */ + /* Adjust tlist, qual, fdw_exprs, etc. in the standard way */ fscan->scan.plan.targetlist = fix_scan_list(root, fscan->scan.plan.targetlist, rtoffset); fscan->scan.plan.qual = fix_scan_list(root, fscan->scan.plan.qual, rtoffset); fscan->fdw_exprs = fix_scan_list(root, fscan->fdw_exprs, rtoffset); + fscan->fdw_recheck_quals = + fix_scan_list(root, fscan->fdw_recheck_quals, rtoffset); } /* Adjust fs_relids if needed */ diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index f3038cdffd..00fb6376d9 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -2371,10 +2371,18 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params, break; case T_ForeignScan: - finalize_primnode((Node *) ((ForeignScan *) plan)->fdw_exprs, - &context); - /* We assume fdw_scan_tlist cannot contain Params */ - context.paramids = bms_add_members(context.paramids, scan_params); + { + ForeignScan *fscan = (ForeignScan *) plan; + + finalize_primnode((Node *) fscan->fdw_exprs, + &context); + finalize_primnode((Node *) fscan->fdw_recheck_quals, + &context); + + /* We assume fdw_scan_tlist cannot contain Params */ + context.paramids = bms_add_members(context.paramids, + scan_params); + } break; case T_CustomScan: diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 303fc3c1c7..0da5fb478a 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1580,6 +1580,7 @@ typedef struct WorkTableScanState typedef struct ForeignScanState { ScanState ss; /* its first field is NodeTag */ + List *fdw_recheck_quals; /* original quals not in ss.ps.qual */ /* use struct pointer to avoid including fdwapi.h here */ struct FdwRoutine *fdwroutine; void *fdw_state; /* foreign-data wrapper can keep state here */ diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 0654d0266c..5fd45b7b95 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -509,6 +509,11 @@ typedef struct WorkTableScan * fdw_scan_tlist is never actually executed; it just holds expression trees * describing what is in the scan tuple's columns. * + * fdw_recheck_quals should contain any quals which the core system passed to + * the FDW but which were not added to scan.plan.quals; that is, it should + * contain the quals being checked remotely. This is needed for correct + * behavior during EvalPlanQual rechecks. + * * When the plan node represents a foreign join, scan.scanrelid is zero and * fs_relids must be consulted to identify the join relation. (fs_relids * is valid for simple scans as well, but will always match scan.scanrelid.) @@ -521,6 +526,7 @@ typedef struct ForeignScan List *fdw_exprs; /* expressions that FDW may evaluate */ List *fdw_private; /* private data for FDW */ List *fdw_scan_tlist; /* optional tlist describing scan tuple */ + List *fdw_recheck_quals; /* original quals not in scan.plan.quals */ Bitmapset *fs_relids; /* RTIs generated by this scan */ bool fsSystemCol; /* true if any "system column" is needed */ } ForeignScan; diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h index 52b077a1b4..1fb850489f 100644 --- a/src/include/optimizer/planmain.h +++ b/src/include/optimizer/planmain.h @@ -45,7 +45,7 @@ extern SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual, Index scanrelid, Plan *subplan); extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual, Index scanrelid, List *fdw_exprs, List *fdw_private, - List *fdw_scan_tlist); + List *fdw_scan_tlist, List *fdw_recheck_quals); extern Append *make_append(List *appendplans, List *tlist); extern RecursiveUnion *make_recursive_union(List *tlist, Plan *lefttree, Plan *righttree, int wtParam,