mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-05 09:09:22 +02:00
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:
parent
54e07be2df
commit
5043193b78
@ -563,7 +563,8 @@ fileGetForeignPlan(PlannerInfo *root,
|
|||||||
scan_relid,
|
scan_relid,
|
||||||
NIL, /* no expressions to evaluate */
|
NIL, /* no expressions to evaluate */
|
||||||
best_path->fdw_private,
|
best_path->fdw_private,
|
||||||
NIL /* no custom tlist */ );
|
NIL, /* no custom tlist */
|
||||||
|
NIL /* no remote quals */ );
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -748,6 +748,7 @@ postgresGetForeignPlan(PlannerInfo *root,
|
|||||||
Index scan_relid = baserel->relid;
|
Index scan_relid = baserel->relid;
|
||||||
List *fdw_private;
|
List *fdw_private;
|
||||||
List *remote_conds = NIL;
|
List *remote_conds = NIL;
|
||||||
|
List *remote_exprs = NIL;
|
||||||
List *local_exprs = NIL;
|
List *local_exprs = NIL;
|
||||||
List *params_list = NIL;
|
List *params_list = NIL;
|
||||||
List *retrieved_attrs;
|
List *retrieved_attrs;
|
||||||
@ -769,8 +770,8 @@ postgresGetForeignPlan(PlannerInfo *root,
|
|||||||
*
|
*
|
||||||
* This code must match "extract_actual_clauses(scan_clauses, false)"
|
* This code must match "extract_actual_clauses(scan_clauses, false)"
|
||||||
* except for the additional decision about remote versus local execution.
|
* except for the additional decision about remote versus local execution.
|
||||||
* Note however that we only strip the RestrictInfo nodes from the
|
* Note however that we don't strip the RestrictInfo nodes from the
|
||||||
* local_exprs list, since appendWhereClause expects a list of
|
* remote_conds list, since appendWhereClause expects a list of
|
||||||
* RestrictInfos.
|
* RestrictInfos.
|
||||||
*/
|
*/
|
||||||
foreach(lc, scan_clauses)
|
foreach(lc, scan_clauses)
|
||||||
@ -784,11 +785,17 @@ postgresGetForeignPlan(PlannerInfo *root,
|
|||||||
continue;
|
continue;
|
||||||
|
|
||||||
if (list_member_ptr(fpinfo->remote_conds, rinfo))
|
if (list_member_ptr(fpinfo->remote_conds, rinfo))
|
||||||
|
{
|
||||||
remote_conds = lappend(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))
|
else if (list_member_ptr(fpinfo->local_conds, rinfo))
|
||||||
local_exprs = lappend(local_exprs, rinfo->clause);
|
local_exprs = lappend(local_exprs, rinfo->clause);
|
||||||
else if (is_foreign_expr(root, baserel, rinfo->clause))
|
else if (is_foreign_expr(root, baserel, rinfo->clause))
|
||||||
|
{
|
||||||
remote_conds = lappend(remote_conds, rinfo);
|
remote_conds = lappend(remote_conds, rinfo);
|
||||||
|
remote_exprs = lappend(remote_exprs, rinfo->clause);
|
||||||
|
}
|
||||||
else
|
else
|
||||||
local_exprs = lappend(local_exprs, rinfo->clause);
|
local_exprs = lappend(local_exprs, rinfo->clause);
|
||||||
}
|
}
|
||||||
@ -874,7 +881,8 @@ postgresGetForeignPlan(PlannerInfo *root,
|
|||||||
scan_relid,
|
scan_relid,
|
||||||
params_list,
|
params_list,
|
||||||
fdw_private,
|
fdw_private,
|
||||||
NIL /* no custom tlist */ );
|
NIL, /* no custom tlist */
|
||||||
|
remote_exprs);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -1135,6 +1135,15 @@ GetForeignServerByName(const char *name, bool missing_ok);
|
|||||||
evaluation of the <structfield>fdw_exprs</> expression tree.
|
evaluation of the <structfield>fdw_exprs</> expression tree.
|
||||||
</para>
|
</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>
|
<para>
|
||||||
Another <structname>ForeignScan</> field that can be filled by FDWs
|
Another <structname>ForeignScan</> field that can be filled by FDWs
|
||||||
is <structfield>fdw_scan_tlist</>, which describes the tuples returned by
|
is <structfield>fdw_scan_tlist</>, which describes the tuples returned by
|
||||||
|
@ -25,6 +25,7 @@
|
|||||||
#include "executor/executor.h"
|
#include "executor/executor.h"
|
||||||
#include "executor/nodeForeignscan.h"
|
#include "executor/nodeForeignscan.h"
|
||||||
#include "foreign/fdwapi.h"
|
#include "foreign/fdwapi.h"
|
||||||
|
#include "utils/memutils.h"
|
||||||
#include "utils/rel.h"
|
#include "utils/rel.h"
|
||||||
|
|
||||||
static TupleTableSlot *ForeignNext(ForeignScanState *node);
|
static TupleTableSlot *ForeignNext(ForeignScanState *node);
|
||||||
@ -72,8 +73,19 @@ ForeignNext(ForeignScanState *node)
|
|||||||
static bool
|
static bool
|
||||||
ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
|
ForeignRecheck(ForeignScanState *node, TupleTableSlot *slot)
|
||||||
{
|
{
|
||||||
/* There are no access-method-specific conditions to recheck. */
|
ExprContext *econtext;
|
||||||
return true;
|
|
||||||
|
/*
|
||||||
|
* 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 *)
|
scanstate->ss.ps.qual = (List *)
|
||||||
ExecInitExpr((Expr *) node->scan.plan.qual,
|
ExecInitExpr((Expr *) node->scan.plan.qual,
|
||||||
(PlanState *) scanstate);
|
(PlanState *) scanstate);
|
||||||
|
scanstate->fdw_recheck_quals = (List *)
|
||||||
|
ExecInitExpr((Expr *) node->fdw_recheck_quals,
|
||||||
|
(PlanState *) scanstate);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* tuple table initialization
|
* tuple table initialization
|
||||||
|
@ -624,6 +624,7 @@ _copyForeignScan(const ForeignScan *from)
|
|||||||
COPY_NODE_FIELD(fdw_exprs);
|
COPY_NODE_FIELD(fdw_exprs);
|
||||||
COPY_NODE_FIELD(fdw_private);
|
COPY_NODE_FIELD(fdw_private);
|
||||||
COPY_NODE_FIELD(fdw_scan_tlist);
|
COPY_NODE_FIELD(fdw_scan_tlist);
|
||||||
|
COPY_NODE_FIELD(fdw_recheck_quals);
|
||||||
COPY_BITMAPSET_FIELD(fs_relids);
|
COPY_BITMAPSET_FIELD(fs_relids);
|
||||||
COPY_SCALAR_FIELD(fsSystemCol);
|
COPY_SCALAR_FIELD(fsSystemCol);
|
||||||
|
|
||||||
|
@ -579,6 +579,7 @@ _outForeignScan(StringInfo str, const ForeignScan *node)
|
|||||||
WRITE_NODE_FIELD(fdw_exprs);
|
WRITE_NODE_FIELD(fdw_exprs);
|
||||||
WRITE_NODE_FIELD(fdw_private);
|
WRITE_NODE_FIELD(fdw_private);
|
||||||
WRITE_NODE_FIELD(fdw_scan_tlist);
|
WRITE_NODE_FIELD(fdw_scan_tlist);
|
||||||
|
WRITE_NODE_FIELD(fdw_recheck_quals);
|
||||||
WRITE_BITMAPSET_FIELD(fs_relids);
|
WRITE_BITMAPSET_FIELD(fs_relids);
|
||||||
WRITE_BOOL_FIELD(fsSystemCol);
|
WRITE_BOOL_FIELD(fsSystemCol);
|
||||||
}
|
}
|
||||||
|
@ -2117,6 +2117,9 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
|
|||||||
replace_nestloop_params(root, (Node *) scan_plan->scan.plan.qual);
|
replace_nestloop_params(root, (Node *) scan_plan->scan.plan.qual);
|
||||||
scan_plan->fdw_exprs = (List *)
|
scan_plan->fdw_exprs = (List *)
|
||||||
replace_nestloop_params(root, (Node *) scan_plan->fdw_exprs);
|
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,
|
Index scanrelid,
|
||||||
List *fdw_exprs,
|
List *fdw_exprs,
|
||||||
List *fdw_private,
|
List *fdw_private,
|
||||||
List *fdw_scan_tlist)
|
List *fdw_scan_tlist,
|
||||||
|
List *fdw_recheck_quals)
|
||||||
{
|
{
|
||||||
ForeignScan *node = makeNode(ForeignScan);
|
ForeignScan *node = makeNode(ForeignScan);
|
||||||
Plan *plan = &node->scan.plan;
|
Plan *plan = &node->scan.plan;
|
||||||
@ -3718,6 +3722,7 @@ make_foreignscan(List *qptlist,
|
|||||||
node->fdw_exprs = fdw_exprs;
|
node->fdw_exprs = fdw_exprs;
|
||||||
node->fdw_private = fdw_private;
|
node->fdw_private = fdw_private;
|
||||||
node->fdw_scan_tlist = fdw_scan_tlist;
|
node->fdw_scan_tlist = fdw_scan_tlist;
|
||||||
|
node->fdw_recheck_quals = fdw_recheck_quals;
|
||||||
/* fs_relids will be filled in by create_foreignscan_plan */
|
/* fs_relids will be filled in by create_foreignscan_plan */
|
||||||
node->fs_relids = NULL;
|
node->fs_relids = NULL;
|
||||||
/* fsSystemCol will be filled in by create_foreignscan_plan */
|
/* fsSystemCol will be filled in by create_foreignscan_plan */
|
||||||
|
@ -1127,13 +1127,15 @@ set_foreignscan_references(PlannerInfo *root,
|
|||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
/* Adjust tlist, qual, fdw_exprs in the standard way */
|
/* Adjust tlist, qual, fdw_exprs, etc. in the standard way */
|
||||||
fscan->scan.plan.targetlist =
|
fscan->scan.plan.targetlist =
|
||||||
fix_scan_list(root, fscan->scan.plan.targetlist, rtoffset);
|
fix_scan_list(root, fscan->scan.plan.targetlist, rtoffset);
|
||||||
fscan->scan.plan.qual =
|
fscan->scan.plan.qual =
|
||||||
fix_scan_list(root, fscan->scan.plan.qual, rtoffset);
|
fix_scan_list(root, fscan->scan.plan.qual, rtoffset);
|
||||||
fscan->fdw_exprs =
|
fscan->fdw_exprs =
|
||||||
fix_scan_list(root, fscan->fdw_exprs, rtoffset);
|
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 */
|
/* Adjust fs_relids if needed */
|
||||||
|
@ -2371,10 +2371,18 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
|
|||||||
break;
|
break;
|
||||||
|
|
||||||
case T_ForeignScan:
|
case T_ForeignScan:
|
||||||
finalize_primnode((Node *) ((ForeignScan *) plan)->fdw_exprs,
|
{
|
||||||
&context);
|
ForeignScan *fscan = (ForeignScan *) plan;
|
||||||
/* We assume fdw_scan_tlist cannot contain Params */
|
|
||||||
context.paramids = bms_add_members(context.paramids, scan_params);
|
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;
|
break;
|
||||||
|
|
||||||
case T_CustomScan:
|
case T_CustomScan:
|
||||||
|
@ -1580,6 +1580,7 @@ typedef struct WorkTableScanState
|
|||||||
typedef struct ForeignScanState
|
typedef struct ForeignScanState
|
||||||
{
|
{
|
||||||
ScanState ss; /* its first field is NodeTag */
|
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 */
|
/* use struct pointer to avoid including fdwapi.h here */
|
||||||
struct FdwRoutine *fdwroutine;
|
struct FdwRoutine *fdwroutine;
|
||||||
void *fdw_state; /* foreign-data wrapper can keep state here */
|
void *fdw_state; /* foreign-data wrapper can keep state here */
|
||||||
|
@ -509,6 +509,11 @@ typedef struct WorkTableScan
|
|||||||
* fdw_scan_tlist is never actually executed; it just holds expression trees
|
* fdw_scan_tlist is never actually executed; it just holds expression trees
|
||||||
* describing what is in the scan tuple's columns.
|
* 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
|
* 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
|
* 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.)
|
* 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_exprs; /* expressions that FDW may evaluate */
|
||||||
List *fdw_private; /* private data for FDW */
|
List *fdw_private; /* private data for FDW */
|
||||||
List *fdw_scan_tlist; /* optional tlist describing scan tuple */
|
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 */
|
Bitmapset *fs_relids; /* RTIs generated by this scan */
|
||||||
bool fsSystemCol; /* true if any "system column" is needed */
|
bool fsSystemCol; /* true if any "system column" is needed */
|
||||||
} ForeignScan;
|
} ForeignScan;
|
||||||
|
@ -45,7 +45,7 @@ extern SubqueryScan *make_subqueryscan(List *qptlist, List *qpqual,
|
|||||||
Index scanrelid, Plan *subplan);
|
Index scanrelid, Plan *subplan);
|
||||||
extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
|
extern ForeignScan *make_foreignscan(List *qptlist, List *qpqual,
|
||||||
Index scanrelid, List *fdw_exprs, List *fdw_private,
|
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 Append *make_append(List *appendplans, List *tlist);
|
||||||
extern RecursiveUnion *make_recursive_union(List *tlist,
|
extern RecursiveUnion *make_recursive_union(List *tlist,
|
||||||
Plan *lefttree, Plan *righttree, int wtParam,
|
Plan *lefttree, Plan *righttree, int wtParam,
|
||||||
|
Loading…
Reference in New Issue
Block a user