rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allow FDWs to push down quals without breaking EvalPlanQual rechecks. 

This fixes a long-standing bug which was discovered while investigating
the interaction between the new join pushdown code and the EvalPlanQual
machinery: if a ForeignScan appears on the inner side of a paramaterized
nestloop, an EPQ recheck would re-return the original tuple even if
it no longer satisfied the pushed-down quals due to changed parameter
values.

This fix adds a new member to ForeignScan and ForeignScanState and a
new argument to make_foreignscan, and requires changes to FDWs which
push down quals to populate that new argument with a list of quals they
have 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.
This commit is contained in:
Robert Haas 2015-10-15 13:00:40 -04:00
parent 817588bc2b
commit 5fc4c26db5
13 changed files with 71 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
contrib/file_fdw/file_fdw.c
View File

@ -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 */ );
}
/*

14
contrib/postgres_fdw/postgres_fdw.c
View File

@ -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);
}
/*

9
doc/src/sgml/fdwhandler.sgml
View File

@ -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

19
src/backend/executor/nodeForeignscan.c
View File

@ -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

1
src/backend/nodes/copyfuncs.c
View File

@ -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);

1
src/backend/nodes/outfuncs.c
View File

@ -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);
}

1
src/backend/nodes/readfuncs.c
View File

@ -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);

7
src/backend/optimizer/plan/createplan.c
View File

@ -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 */

4
src/backend/optimizer/plan/setrefs.c
View File

@ -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 */

16
src/backend/optimizer/plan/subselect.c
View File

@ -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:

1
src/include/nodes/execnodes.h
View File

@ -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 */

6
src/include/nodes/plannodes.h
View File

@ -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;

2
src/include/optimizer/planmain.h
View File

@ -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,