aboutsummaryrefslogtreecommitdiff
path: root/src/backend/executor/execMain.c
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2018-09-15 13:42:33 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2018-09-15 13:42:33 -0400
commit1f4a920b7309499d2d0c4ceda5e6356e10bc51da (patch)
tree76e8e3e185484bd2f2cb32973cdbcc471b633293 /src/backend/executor/execMain.c
parent6b78231d918bba1b99d15e0bf19753bd8f826222 (diff)
downloadpostgresql-1f4a920b7309499d2d0c4ceda5e6356e10bc51da.tar.gz
postgresql-1f4a920b7309499d2d0c4ceda5e6356e10bc51da.zip
Fix failure with initplans used conditionally during EvalPlanQual rechecks.
The EvalPlanQual machinery assumes that any initplans (that is, uncorrelated sub-selects) used during an EPQ recheck would have already been evaluated during the main query; this is implicit in the fact that execPlan pointers are not copied into the EPQ estate's es_param_exec_vals. But it's possible for that assumption to fail, if the initplan is only reached conditionally. For example, a sub-select inside a CASE expression could be reached during a recheck when it had not been previously, if the CASE test depends on a column that was just updated. This bug is old, appearing to date back to my rewrite of EvalPlanQual in commit 9f2ee8f28, but was not detected until Kyle Samson reported a case. To fix, force all not-yet-evaluated initplans used within the EPQ plan subtree to be evaluated at the start of the recheck, before entering the EPQ environment. This could be inefficient, if such an initplan is expensive and goes unused again during the recheck --- but that's piling one layer of improbability atop another. It doesn't seem worth adding more complexity to prevent that, at least not in the back branches. It was convenient to use the new-in-v11 ExecEvalParamExecParams function to implement this, but I didn't like either its name or the specifics of its API, so revise that. Back-patch all the way. Rather than rewrite the patch to avoid depending on bms_next_member() in the oldest branches, I chose to back-patch that function into 9.4 and 9.3. (This isn't the first time back-patches have needed that, and it exhausted my patience.) I also chose to back-patch some test cases added by commits 71404af2a and 342a1ffa2 into 9.4 and 9.3, so that the 9.x versions of eval-plan-qual.spec are all the same. Andrew Gierth diagnosed the problem and contributed the added test cases, though the actual code changes are by me. Discussion: https://postgr.es/m/A033A40A-B234-4324-BE37-272279F7B627@tripadvisor.com
Diffstat (limited to 'src/backend/executor/execMain.c')
-rw-r--r--src/backend/executor/execMain.c40
1 files changed, 36 insertions, 4 deletions
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index c583e020a0e..85d980356b7 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -46,6 +46,7 @@
#include "commands/matview.h"
#include "commands/trigger.h"
#include "executor/execdebug.h"
+#include "executor/nodeSubplan.h"
#include "foreign/fdwapi.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
@@ -1727,8 +1728,8 @@ ExecutePlan(EState *estate,
if (TupIsNull(slot))
{
/*
- * If we know we won't need to back up, we can release
- * resources at this point.
+ * If we know we won't need to back up, we can release resources
+ * at this point.
*/
if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))
(void) ExecShutdownNode(planstate);
@@ -1778,8 +1779,8 @@ ExecutePlan(EState *estate,
if (numberTuples && numberTuples == current_tuple_count)
{
/*
- * If we know we won't need to back up, we can release
- * resources at this point.
+ * If we know we won't need to back up, we can release resources
+ * at this point.
*/
if (!(estate->es_top_eflags & EXEC_FLAG_BACKWARD))
(void) ExecShutdownNode(planstate);
@@ -3078,6 +3079,14 @@ EvalPlanQualBegin(EPQState *epqstate, EState *parentestate)
{
int i;
+ /*
+ * Force evaluation of any InitPlan outputs that could be needed
+ * by the subplan, just in case they got reset since
+ * EvalPlanQualStart (see comments therein).
+ */
+ ExecSetParamPlanMulti(planstate->plan->extParam,
+ GetPerTupleExprContext(parentestate));
+
i = list_length(parentestate->es_plannedstmt->paramExecTypes);
while (--i >= 0)
@@ -3170,9 +3179,32 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
{
int i;
+ /*
+ * Force evaluation of any InitPlan outputs that could be needed by
+ * the subplan. (With more complexity, maybe we could postpone this
+ * till the subplan actually demands them, but it doesn't seem worth
+ * the trouble; this is a corner case already, since usually the
+ * InitPlans would have been evaluated before reaching EvalPlanQual.)
+ *
+ * This will not touch output params of InitPlans that occur somewhere
+ * within the subplan tree, only those that are attached to the
+ * ModifyTable node or above it and are referenced within the subplan.
+ * That's OK though, because the planner would only attach such
+ * InitPlans to a lower-level SubqueryScan node, and EPQ execution
+ * will not descend into a SubqueryScan.
+ *
+ * The EState's per-output-tuple econtext is sufficiently short-lived
+ * for this, since it should get reset before there is any chance of
+ * doing EvalPlanQual again.
+ */
+ ExecSetParamPlanMulti(planTree->extParam,
+ GetPerTupleExprContext(parentestate));
+
+ /* now make the internal param workspace ... */
i = list_length(parentestate->es_plannedstmt->paramExecTypes);
estate->es_param_exec_vals = (ParamExecData *)
palloc0(i * sizeof(ParamExecData));
+ /* ... and copy down all values, whether really needed or not */
while (--i >= 0)
{
/* copy value if any, but not execPlan link */