From e4a5f0a418949b9d4700399ba0a85577cd85cb7f Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 31 Aug 2017 13:22:41 -0700
Subject: [PATCH 05/16] Perform slot validity checks in a separate pass over
 expression.

This is better for JITing and allows to get rid of some code
duplication.
---
 src/backend/executor/execExpr.c       |   8 +-
 src/backend/executor/execExprInterp.c | 192 ++++++++++++++--------------------
 src/include/executor/execExpr.h       |   9 +-
 src/include/nodes/execnodes.h         |   2 +
 4 files changed, 86 insertions(+), 125 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index be9d23bc32..81549ee915 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -637,20 +637,20 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state,
 					/* regular user column */
 					scratch.d.var.attnum = variable->varattno - 1;
 					scratch.d.var.vartype = variable->vartype;
-					/* select EEOP_*_FIRST opcode to force one-time checks */
+
 					switch (variable->varno)
 					{
 						case INNER_VAR:
-							scratch.opcode = EEOP_INNER_VAR_FIRST;
+							scratch.opcode = EEOP_INNER_VAR;
 							break;
 						case OUTER_VAR:
-							scratch.opcode = EEOP_OUTER_VAR_FIRST;
+							scratch.opcode = EEOP_OUTER_VAR;
 							break;
 
 							/* INDEX_VAR is handled by default case */
 
 						default:
-							scratch.opcode = EEOP_SCAN_VAR_FIRST;
+							scratch.opcode = EEOP_SCAN_VAR;
 							break;
 					}
 				}
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 83e04471e4..50e3f8b176 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -131,7 +131,6 @@ static Datum ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnul
 static void ExecInitInterpreter(void);
 
 /* support functions */
-static void CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype);
 static TupleDesc get_cached_rowtype(Oid type_id, int32 typmod,
 				   TupleDesc *cache_field, ExprContext *econtext);
 static void ShutdownTupleDescRef(Datum arg);
@@ -139,11 +138,8 @@ static void ExecEvalRowNullInt(ExprState *state, ExprEvalStep *op,
 				   ExprContext *econtext, bool checkisnull);
 
 /* fast-path evaluation functions */
-static Datum ExecJustInnerVarFirst(ExprState *state, ExprContext *econtext, bool *isnull);
 static Datum ExecJustInnerVar(ExprState *state, ExprContext *econtext, bool *isnull);
-static Datum ExecJustOuterVarFirst(ExprState *state, ExprContext *econtext, bool *isnull);
 static Datum ExecJustOuterVar(ExprState *state, ExprContext *econtext, bool *isnull);
-static Datum ExecJustScanVarFirst(ExprState *state, ExprContext *econtext, bool *isnull);
 static Datum ExecJustScanVar(ExprState *state, ExprContext *econtext, bool *isnull);
 static Datum ExecJustConst(ExprState *state, ExprContext *econtext, bool *isnull);
 static Datum ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull);
@@ -172,6 +168,8 @@ ExecReadyInterpretedExpr(ExprState *state)
 	if (state->flags & EEO_FLAG_INTERPRETER_INITIALIZED)
 		return;
 
+	state->evalfunc = ExecInterpExprStillValid;
+
 	/* DIRECT_THREADED should not already be set */
 	Assert((state->flags & EEO_FLAG_DIRECT_THREADED) == 0);
 
@@ -195,46 +193,46 @@ ExecReadyInterpretedExpr(ExprState *state)
 		ExprEvalOp	step1 = state->steps[1].opcode;
 
 		if (step0 == EEOP_INNER_FETCHSOME &&
-			step1 == EEOP_INNER_VAR_FIRST)
+			step1 == EEOP_INNER_VAR)
 		{
-			state->evalfunc = ExecJustInnerVarFirst;
+			state->evalfunc_private = ExecJustInnerVar;
 			return;
 		}
 		else if (step0 == EEOP_OUTER_FETCHSOME &&
-				 step1 == EEOP_OUTER_VAR_FIRST)
+				 step1 == EEOP_OUTER_VAR)
 		{
-			state->evalfunc = ExecJustOuterVarFirst;
+			state->evalfunc_private = ExecJustOuterVar;
 			return;
 		}
 		else if (step0 == EEOP_SCAN_FETCHSOME &&
-				 step1 == EEOP_SCAN_VAR_FIRST)
+				 step1 == EEOP_SCAN_VAR)
 		{
-			state->evalfunc = ExecJustScanVarFirst;
+			state->evalfunc_private = ExecJustScanVar;
 			return;
 		}
 		else if (step0 == EEOP_INNER_FETCHSOME &&
 				 step1 == EEOP_ASSIGN_INNER_VAR)
 		{
-			state->evalfunc = ExecJustAssignInnerVar;
+			state->evalfunc_private = ExecJustAssignInnerVar;
 			return;
 		}
 		else if (step0 == EEOP_OUTER_FETCHSOME &&
 				 step1 == EEOP_ASSIGN_OUTER_VAR)
 		{
-			state->evalfunc = ExecJustAssignOuterVar;
+			state->evalfunc_private = ExecJustAssignOuterVar;
 			return;
 		}
 		else if (step0 == EEOP_SCAN_FETCHSOME &&
 				 step1 == EEOP_ASSIGN_SCAN_VAR)
 		{
-			state->evalfunc = ExecJustAssignScanVar;
+			state->evalfunc_private = ExecJustAssignScanVar;
 			return;
 		}
 	}
 	else if (state->steps_len == 2 &&
 			 state->steps[0].opcode == EEOP_CONST)
 	{
-		state->evalfunc = ExecJustConst;
+		state->evalfunc_private = ExecJustConst;
 		return;
 	}
 
@@ -258,7 +256,7 @@ ExecReadyInterpretedExpr(ExprState *state)
 	}
 #endif							/* EEO_USE_COMPUTED_GOTO */
 
-	state->evalfunc = ExecInterpExpr;
+	state->evalfunc_private = ExecInterpExpr;
 }
 
 
@@ -289,11 +287,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		&&CASE_EEOP_INNER_FETCHSOME,
 		&&CASE_EEOP_OUTER_FETCHSOME,
 		&&CASE_EEOP_SCAN_FETCHSOME,
-		&&CASE_EEOP_INNER_VAR_FIRST,
 		&&CASE_EEOP_INNER_VAR,
-		&&CASE_EEOP_OUTER_VAR_FIRST,
 		&&CASE_EEOP_OUTER_VAR,
-		&&CASE_EEOP_SCAN_VAR_FIRST,
 		&&CASE_EEOP_SCAN_VAR,
 		&&CASE_EEOP_INNER_SYSVAR,
 		&&CASE_EEOP_OUTER_SYSVAR,
@@ -415,22 +410,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 			EEO_NEXT();
 		}
 
-		EEO_CASE(EEOP_INNER_VAR_FIRST)
-		{
-			int			attnum = op->d.var.attnum;
-
-			/*
-			 * First time through, check whether attribute matches Var.  Might
-			 * not be ok anymore, due to schema changes.
-			 */
-			CheckVarSlotCompatibility(innerslot, attnum + 1, op->d.var.vartype);
-
-			/* Skip that check on subsequent evaluations */
-			op->opcode = EEO_OPCODE(EEOP_INNER_VAR);
-
-			/* FALL THROUGH to EEOP_INNER_VAR */
-		}
-
 		EEO_CASE(EEOP_INNER_VAR)
 		{
 			int			attnum = op->d.var.attnum;
@@ -448,18 +427,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 			EEO_NEXT();
 		}
 
-		EEO_CASE(EEOP_OUTER_VAR_FIRST)
-		{
-			int			attnum = op->d.var.attnum;
-
-			/* See EEOP_INNER_VAR_FIRST comments */
-
-			CheckVarSlotCompatibility(outerslot, attnum + 1, op->d.var.vartype);
-			op->opcode = EEO_OPCODE(EEOP_OUTER_VAR);
-
-			/* FALL THROUGH to EEOP_OUTER_VAR */
-		}
-
 		EEO_CASE(EEOP_OUTER_VAR)
 		{
 			int			attnum = op->d.var.attnum;
@@ -473,18 +440,6 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 			EEO_NEXT();
 		}
 
-		EEO_CASE(EEOP_SCAN_VAR_FIRST)
-		{
-			int			attnum = op->d.var.attnum;
-
-			/* See EEOP_INNER_VAR_FIRST comments */
-
-			CheckVarSlotCompatibility(scanslot, attnum + 1, op->d.var.vartype);
-			op->opcode = EEO_OPCODE(EEOP_SCAN_VAR);
-
-			/* FALL THROUGH to EEOP_SCAN_VAR */
-		}
-
 		EEO_CASE(EEOP_SCAN_VAR)
 		{
 			int			attnum = op->d.var.attnum;
@@ -1524,7 +1479,7 @@ out:
  * expression.  This should succeed unless there have been schema changes
  * since the expression tree has been created.
  */
-static void
+void
 CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype)
 {
 	/*
@@ -1572,6 +1527,61 @@ CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype)
 	}
 }
 
+Datum
+ExecInterpExprStillValid(ExprState *state, ExprContext *econtext, bool *isNull)
+{
+	CheckExprStillValid(state, econtext, isNull);
+
+	state->evalfunc = state->evalfunc_private;
+
+	return state->evalfunc(state, econtext, isNull);
+}
+
+void
+CheckExprStillValid(ExprState *state, ExprContext *econtext, bool *isNull)
+{
+	int i = 0;
+	TupleTableSlot *innerslot;
+	TupleTableSlot *outerslot;
+	TupleTableSlot *scanslot;
+
+	innerslot = econtext->ecxt_innertuple;
+	outerslot = econtext->ecxt_outertuple;
+	scanslot = econtext->ecxt_scantuple;
+
+	for (i = 0; i < state->steps_len;i++)
+	{
+		ExprEvalStep   *op = &state->steps[i];
+
+		switch (ExecEvalStepOp(state, op))
+		{
+			case EEOP_INNER_VAR:
+				{
+					int			attnum = op->d.var.attnum;
+					CheckVarSlotCompatibility(innerslot, attnum + 1, op->d.var.vartype);
+					break;
+				}
+
+		case EEOP_OUTER_VAR:
+				{
+					int			attnum = op->d.var.attnum;
+					CheckVarSlotCompatibility(outerslot, attnum + 1, op->d.var.vartype);
+					break;
+				}
+
+		case EEOP_SCAN_VAR:
+				{
+					int			attnum = op->d.var.attnum;
+					CheckVarSlotCompatibility(scanslot, attnum + 1, op->d.var.vartype);
+					break;
+				}
+		default:
+			break;
+		}
+	}
+}
+
+
 /*
  * get_cached_rowtype: utility function to lookup a rowtype tupdesc
  *
@@ -1631,28 +1641,6 @@ ShutdownTupleDescRef(Datum arg)
  * Fast-path functions, for very simple expressions
  */
 
-/* Simple reference to inner Var, first time through */
-static Datum
-ExecJustInnerVarFirst(ExprState *state, ExprContext *econtext, bool *isnull)
-{
-	ExprEvalStep *op = &state->steps[1];
-	int			attnum = op->d.var.attnum + 1;
-	TupleTableSlot *slot = econtext->ecxt_innertuple;
-
-	/* See ExecInterpExpr()'s comments for EEOP_INNER_VAR_FIRST */
-
-	CheckVarSlotCompatibility(slot, attnum, op->d.var.vartype);
-	op->opcode = EEOP_INNER_VAR;	/* just for cleanliness */
-	state->evalfunc = ExecJustInnerVar;
-
-	/*
-	 * Since we use slot_getattr(), we don't need to implement the FETCHSOME
-	 * step explicitly, and we also needn't Assert that the attnum is in range
-	 * --- slot_getattr() will take care of any problems.
-	 */
-	return slot_getattr(slot, attnum, isnull);
-}
-
 /* Simple reference to inner Var */
 static Datum
 ExecJustInnerVar(ExprState *state, ExprContext *econtext, bool *isnull)
@@ -1661,23 +1649,11 @@ ExecJustInnerVar(ExprState *state, ExprContext *econtext, bool *isnull)
 	int			attnum = op->d.var.attnum + 1;
 	TupleTableSlot *slot = econtext->ecxt_innertuple;
 
-	/* See comments in ExecJustInnerVarFirst */
-	return slot_getattr(slot, attnum, isnull);
-}
-
-/* Simple reference to outer Var, first time through */
-static Datum
-ExecJustOuterVarFirst(ExprState *state, ExprContext *econtext, bool *isnull)
-{
-	ExprEvalStep *op = &state->steps[1];
-	int			attnum = op->d.var.attnum + 1;
-	TupleTableSlot *slot = econtext->ecxt_outertuple;
-
-	CheckVarSlotCompatibility(slot, attnum, op->d.var.vartype);
-	op->opcode = EEOP_OUTER_VAR;	/* just for cleanliness */
-	state->evalfunc = ExecJustOuterVar;
-
-	/* See comments in ExecJustInnerVarFirst */
+	/*
+	 * Since we use slot_getattr(), we don't need to implement the FETCHSOME
+	 * step explicitly, and we also needn't Assert that the attnum is in range
+	 * --- slot_getattr() will take care of any problems.
+	 */
 	return slot_getattr(slot, attnum, isnull);
 }
 
@@ -1689,23 +1665,7 @@ ExecJustOuterVar(ExprState *state, ExprContext *econtext, bool *isnull)
 	int			attnum = op->d.var.attnum + 1;
 	TupleTableSlot *slot = econtext->ecxt_outertuple;
 
-	/* See comments in ExecJustInnerVarFirst */
-	return slot_getattr(slot, attnum, isnull);
-}
-
-/* Simple reference to scan Var, first time through */
-static Datum
-ExecJustScanVarFirst(ExprState *state, ExprContext *econtext, bool *isnull)
-{
-	ExprEvalStep *op = &state->steps[1];
-	int			attnum = op->d.var.attnum + 1;
-	TupleTableSlot *slot = econtext->ecxt_scantuple;
-
-	CheckVarSlotCompatibility(slot, attnum, op->d.var.vartype);
-	op->opcode = EEOP_SCAN_VAR; /* just for cleanliness */
-	state->evalfunc = ExecJustScanVar;
-
-	/* See comments in ExecJustInnerVarFirst */
+	/* See comments in ExecJustInnerVar */
 	return slot_getattr(slot, attnum, isnull);
 }
 
@@ -1717,7 +1677,7 @@ ExecJustScanVar(ExprState *state, ExprContext *econtext, bool *isnull)
 	int			attnum = op->d.var.attnum + 1;
 	TupleTableSlot *slot = econtext->ecxt_scantuple;
 
-	/* See comments in ExecJustInnerVarFirst */
+	/* See comments in ExecJustInnerVar */
 	return slot_getattr(slot, attnum, isnull);
 }
 
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 8ee0496e01..0fbc112890 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -45,12 +45,8 @@ typedef enum ExprEvalOp
 	EEOP_SCAN_FETCHSOME,
 
 	/* compute non-system Var value */
-	/* "FIRST" variants are used only the first time through */
-	EEOP_INNER_VAR_FIRST,
 	EEOP_INNER_VAR,
-	EEOP_OUTER_VAR_FIRST,
 	EEOP_OUTER_VAR,
-	EEOP_SCAN_VAR_FIRST,
 	EEOP_SCAN_VAR,
 
 	/* compute system Var value */
@@ -62,7 +58,6 @@ typedef enum ExprEvalOp
 	EEOP_WHOLEROW,
 
 	/* compute non-system Var value, assign it into ExprState's resultslot */
-	/* (these are not used if _FIRST checks would be needed) */
 	EEOP_ASSIGN_INNER_VAR,
 	EEOP_ASSIGN_OUTER_VAR,
 	EEOP_ASSIGN_SCAN_VAR,
@@ -604,6 +599,10 @@ extern void ExecReadyInterpretedExpr(ExprState *state);
 
 extern ExprEvalOp ExecEvalStepOp(ExprState *state, ExprEvalStep *op);
 
+extern void CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype);
+extern Datum ExecInterpExprStillValid(ExprState *state, ExprContext *econtext, bool *isNull);
+extern void CheckExprStillValid(ExprState *state, ExprContext *econtext, bool *isNull);
+
 /*
  * Non fast-path execution functions. These are externs instead of statics in
  * execExprInterp.c, because that allows them to be used by other methods of
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 0dc9fa8d79..8ae8179ee7 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -94,6 +94,8 @@ typedef struct ExprState
 
 	Datum	   *innermost_domainval;
 	bool	   *innermost_domainnull;
+
+	void	   *evalfunc_private;
 } ExprState;
 
 
-- 
2.14.1.2.g4274c698f4.dirty

