4 files changed, 39 insertions, 28 deletions

diff --git a/src/backend/executor/README b/src/backend/executor/README
index 1db9642be85..02745c23ed9 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -133,9 +133,14 @@ is used by the function evaluation step, thus avoiding extra work to copy
 the result values around.
 
 The last entry in a completed ExprState->steps array is always an
-EEOP_DONE step; this removes the need to test for end-of-array while
-iterating.  Also, if the expression contains any variable references (to
-user columns of the ExprContext's INNER, OUTER, or SCAN tuples), the steps
+EEOP_DONE_RETURN or EEOP_DONE_NO_RETURN step; this removes the need to
+test for end-of-array while iterating. The former is used when the
+expression returns a value directly, the latter when side-effects of
+expression initialization are the goal (e.g. for projection or
+aggregate transition value computation).
+
+Also, if the expression contains any variable references (to user
+columns of the ExprContext's INNER, OUTER, or SCAN tuples), the steps
 array begins with EEOP_*_FETCHSOME steps that ensure that the relevant
 tuples have been deconstructed to make the required columns directly
 available (cf. slot_getsomeattrs()).  This allows individual Var-fetching
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 03566c4d181..0175b152980 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -8,7 +8,7 @@
  *	using ExecInitExpr() et al.  This converts the tree into a flat array
  *	of ExprEvalSteps, which may be thought of as instructions in a program.
  *	At runtime, we'll execute steps, starting with the first, until we reach
- *	an EEOP_DONE opcode.
+ *	an EEOP_DONE_{RETURN|NO_RETURN} opcode.
  *
  *	This file contains the "compilation" logic.  It is independent of the
  *	specific execution technology we use (switch statement, computed goto,
@@ -162,7 +162,7 @@ ExecInitExpr(Expr *node, PlanState *parent)
 	ExecInitExprRec(node, state, &state->resvalue, &state->resnull);
 
 	/* Finally, append a DONE step */
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -199,7 +199,7 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params)
 	ExecInitExprRec(node, state, &state->resvalue, &state->resnull);
 
 	/* Finally, append a DONE step */
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -291,7 +291,7 @@ ExecInitQual(List *qual, PlanState *parent)
 	 * have yielded TRUE, and since its result is stored in the desired output
 	 * location, we're done.
 	 */
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -503,7 +503,7 @@ ExecBuildProjectionInfo(List *targetList,
 		}
 	}
 
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_NO_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -742,7 +742,7 @@ ExecBuildUpdateProjection(List *targetList,
 		}
 	}
 
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_NO_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -1714,7 +1714,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
 				else
 				{
 					/* Not trivial, so append a DONE step */
-					scratch.opcode = EEOP_DONE;
+					scratch.opcode = EEOP_DONE_RETURN;
 					ExprEvalPushStep(elemstate, &scratch);
 					/* and ready the subexpression */
 					ExecReadyExpr(elemstate);
@@ -3991,7 +3991,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
 
 	scratch.resvalue = NULL;
 	scratch.resnull = NULL;
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_NO_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -4258,7 +4258,7 @@ ExecBuildHash32FromAttrs(TupleDesc desc, const TupleTableSlotOps *ops,
 
 	scratch.resvalue = NULL;
 	scratch.resnull = NULL;
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -4431,7 +4431,7 @@ ExecBuildHash32Expr(TupleDesc desc, const TupleTableSlotOps *ops,
 
 	scratch.resvalue = NULL;
 	scratch.resnull = NULL;
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -4586,7 +4586,7 @@ ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc,
 
 	scratch.resvalue = NULL;
 	scratch.resnull = NULL;
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
@@ -4722,7 +4722,7 @@ ExecBuildParamSetEqual(TupleDesc desc,
 
 	scratch.resvalue = NULL;
 	scratch.resnull = NULL;
-	scratch.opcode = EEOP_DONE;
+	scratch.opcode = EEOP_DONE_RETURN;
 	ExprEvalPushStep(state, &scratch);
 
 	ExecReadyExpr(state);
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 1c3477b03c9..491ecad8dc6 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -246,7 +246,8 @@ ExecReadyInterpretedExpr(ExprState *state)
 
 	/* Simple validity checks on expression */
 	Assert(state->steps_len >= 1);
-	Assert(state->steps[state->steps_len - 1].opcode == EEOP_DONE);
+	Assert(state->steps[state->steps_len - 1].opcode == EEOP_DONE_RETURN ||
+		   state->steps[state->steps_len - 1].opcode == EEOP_DONE_NO_RETURN);
 
 	/*
 	 * Don't perform redundant initialization. This is unreachable in current
@@ -469,7 +470,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 	 */
 #if defined(EEO_USE_COMPUTED_GOTO)
 	static const void *const dispatch_table[] = {
-		&&CASE_EEOP_DONE,
+		&&CASE_EEOP_DONE_RETURN,
+		&&CASE_EEOP_DONE_NO_RETURN,
 		&&CASE_EEOP_INNER_FETCHSOME,
 		&&CASE_EEOP_OUTER_FETCHSOME,
 		&&CASE_EEOP_SCAN_FETCHSOME,
@@ -612,9 +614,16 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 
 	EEO_SWITCH()
 	{
-		EEO_CASE(EEOP_DONE)
+		EEO_CASE(EEOP_DONE_RETURN)
 		{
-			goto out;
+			*isnull = state->resnull;
+			return state->resvalue;
+		}
+
+		EEO_CASE(EEOP_DONE_NO_RETURN)
+		{
+			Assert(isnull == NULL);
+			return (Datum) 0;
 		}
 
 		EEO_CASE(EEOP_INNER_FETCHSOME)
@@ -2188,13 +2197,13 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		{
 			/* unreachable */
 			Assert(false);
-			goto out;
+			goto out_error;
 		}
 	}
 
-out:
-	*isnull = state->resnull;
-	return state->resvalue;
+out_error:
+	pg_unreachable();
+	return (Datum) 0;
 }
 
 /*
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index ceb8c8a8039..b4a7698a0b3 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -816,11 +816,8 @@ advance_transition_function(AggState *aggstate,
 static void
 advance_aggregates(AggState *aggstate)
 {
-	bool		dummynull;
-
-	ExecEvalExprSwitchContext(aggstate->phase->evaltrans,
-							  aggstate->tmpcontext,
-							  &dummynull);
+	ExecEvalExprNoReturnSwitchContext(aggstate->phase->evaltrans,
+									  aggstate->tmpcontext);
 }
 
 /*