diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c index 499f24ff2872e6b6fcecf22033af1bb9f6fa891b..5ce8f90cd8ba333beaa189260c6d7790167fb8f8 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 e4d799cecd541c777e21e7b48339b83c9e4b17b9..1902f1f4eae1f570a7d4752a0cc678b0399d1931 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 4c410c791689e4e958985c700047ed3f7d352cfd..1533a6bf80c3f888ab18707d1be6cabf1f337003 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 <structfield>fdw_exprs</> expression tree. </para> + <para> + Any clauses removed from the plan node's qual list must instead be added + to <literal>fdw_recheck_quals</> in order to ensure correct behavior + at the <literal>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. + </para> + <para> Another <structname>ForeignScan</> field that can be filled by FDWs is <structfield>fdw_scan_tlist</>, which describes the tuples returned by diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index bb28a7372d1be4a8dfc4cc9bd378d531af3a745d..6165e4a6cb432b1c32a6ea0104f3d14a68015dd4 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 0b4ab2310935024259ee87a1cef9455ee3806c19..c176ff978ea306f9542caab8e2618213436fd42b 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -648,6 +648,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 df7f6e165aa3b49d39a21f0fc60fb3ec0dd40707..3e75cd1146d4fcdcb6b9311843623b24d91ccd21 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -594,6 +594,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/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 5802a73c7182aefba595592ad6d1c1dcaf7e52be..94ba6dc0b9b1efa7c3286fb052c7dfb05c31fa3e 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1798,6 +1798,7 @@ _readForeignScan(void) READ_NODE_FIELD(fdw_exprs); READ_NODE_FIELD(fdw_private); READ_NODE_FIELD(fdw_scan_tlist); + READ_NODE_FIELD(fdw_recheck_quals); READ_BITMAPSET_FIELD(fs_relids); READ_BOOL_FIELD(fsSystemCol); diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 0ee7392bcce53dc2b80650176f4c3072f4ff1c4a..791b64e7d05a3e36a593f1c6d54dbb317d294996 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -2153,6 +2153,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); } /* @@ -3738,7 +3741,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; @@ -3754,6 +3758,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 9392d61dba7c0911f2f0b9aae3a169016a9dffe6..8c6c57101c0baaf8997b15c49ae947c70595bc15 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -1133,13 +1133,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 6b32f85d6c074c0643aee36f338bf22e9a0995ea..82414d4509c79528651a2d21b85f18b25c586168 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -2394,10 +2394,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 b6895f94c397ad0c5342b9e2c48f0fcfd145b5e2..23670e1ff9bac8689e40ef8bd75e38d15efcf530 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1579,6 +1579,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 1f9213c09b0862959a95b1a1e839710d80f02ec6..92fd8e4fe4e11e2a8d2d283ce1d31f55bb9998de 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -512,6 +512,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.) @@ -524,6 +529,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 52b077a1b471516da3d4d4ada70d65afacb644f3..1fb850489fba014efdd772ad57847597a110c9bc 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,