v21-0001-Add-soft-error-handling-to-some-expression-nodes.patch

From be8670035cf508e36be780d5e4d4e721a0075672 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Thu, 21 Sep 2023 10:11:20 +0900
Subject: [PATCH v21 1/5] Add soft error handling to some expression nodes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This changes the CoerceViaIO and CoerceToDomain expression evaluation
code to handle errors softly if the caller asks for it.

For CoerceViaIo, this means using InputFunctionCallSafe(), which
provides the option to handle errors softly, instead of calling the
type input function directly.

For CoerceToDomain, this simply entails replacing the ereport() in
ExecEvalConstraintCheck() by errsave().

In both cases, the ErrorSaveContext to be used is populated in the
expression's struct in the ExprEvalStep.  The ExprState.escontex must
be passed by the caller by setting ExprState.escontext before calling
ExecInitExprRec() on the expression tree containing those expressions.

Note that no call site of ExecInitExprRec() has been changed in this
commit, so there's no functional change.

Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
---
 src/backend/executor/execExpr.c       | 27 ++++------
 src/backend/executor/execExprInterp.c | 36 ++++++--------
 src/backend/jit/llvm/llvmjit.c        |  4 ++
 src/backend/jit/llvm/llvmjit_expr.c   | 71 +++++++++++++++------------
 src/backend/jit/llvm/llvmjit_types.c  |  3 ++
 src/include/executor/execExpr.h       |  5 +-
 src/include/jit/llvmjit.h             |  2 +
 src/include/jit/llvmjit_emit.h        |  9 ++++
 src/include/nodes/execnodes.h         |  7 +++
 9 files changed, 94 insertions(+), 70 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 2c62b0c9c8..9358f6007e 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -139,6 +139,7 @@ ExecInitExpr(Expr *node, PlanState *parent)
 	state->expr = node;
 	state->parent = parent;
 	state->ext_params = NULL;
+	state->escontext = NULL;
 
 	/* Insert setup steps as needed */
 	ExecCreateExprSetupSteps(state, (Node *) node);
@@ -176,6 +177,7 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params)
 	state->expr = node;
 	state->parent = NULL;
 	state->ext_params = ext_params;
+	state->escontext = NULL;
 
 	/* Insert setup steps as needed */
 	ExecCreateExprSetupSteps(state, (Node *) node);
@@ -228,6 +230,7 @@ ExecInitQual(List *qual, PlanState *parent)
 	state->expr = (Expr *) qual;
 	state->parent = parent;
 	state->ext_params = NULL;
+	state->escontext = NULL;
 
 	/* mark expression as to be used with ExecQual() */
 	state->flags = EEO_FLAG_IS_QUAL;
@@ -373,6 +376,7 @@ ExecBuildProjectionInfo(List *targetList,
 	state->expr = (Expr *) targetList;
 	state->parent = parent;
 	state->ext_params = NULL;
+	state->escontext = NULL;
 
 	state->resultslot = slot;
 
@@ -544,6 +548,7 @@ ExecBuildUpdateProjection(List *targetList,
 		state->expr = NULL;		/* not used */
 	state->parent = parent;
 	state->ext_params = NULL;
+	state->escontext = NULL;
 
 	state->resultslot = slot;
 
@@ -1549,8 +1554,6 @@ ExecInitExprRec(Expr *node, ExprState *state,
 				CoerceViaIO *iocoerce = (CoerceViaIO *) node;
 				Oid			iofunc;
 				bool		typisvarlena;
-				Oid			typioparam;
-				FunctionCallInfo fcinfo_in;
 
 				/* evaluate argument into step's result area */
 				ExecInitExprRec(iocoerce->arg, state, resv, resnull);
@@ -1579,25 +1582,13 @@ ExecInitExprRec(Expr *node, ExprState *state,
 
 				/* lookup the result type's input function */
 				scratch.d.iocoerce.finfo_in = palloc0(sizeof(FmgrInfo));
-				scratch.d.iocoerce.fcinfo_data_in = palloc0(SizeForFunctionCallInfo(3));
-
 				getTypeInputInfo(iocoerce->resulttype,
-								 &iofunc, &typioparam);
+								 &iofunc, &scratch.d.iocoerce.typioparam);
 				fmgr_info(iofunc, scratch.d.iocoerce.finfo_in);
 				fmgr_info_set_expr((Node *) node, scratch.d.iocoerce.finfo_in);
-				InitFunctionCallInfoData(*scratch.d.iocoerce.fcinfo_data_in,
-										 scratch.d.iocoerce.finfo_in,
-										 3, InvalidOid, NULL, NULL);
 
-				/*
-				 * We can preload the second and third arguments for the input
-				 * function, since they're constants.
-				 */
-				fcinfo_in = scratch.d.iocoerce.fcinfo_data_in;
-				fcinfo_in->args[1].value = ObjectIdGetDatum(typioparam);
-				fcinfo_in->args[1].isnull = false;
-				fcinfo_in->args[2].value = Int32GetDatum(-1);
-				fcinfo_in->args[2].isnull = false;
+				/* Use the ErrorSaveContext passed by the caller. */
+				scratch.d.iocoerce.escontext = state->escontext;
 
 				ExprEvalPushStep(state, &scratch);
 				break;
@@ -1628,6 +1619,7 @@ ExecInitExprRec(Expr *node, ExprState *state,
 				elemstate->expr = acoerce->elemexpr;
 				elemstate->parent = state->parent;
 				elemstate->ext_params = state->ext_params;
+				state->escontext = NULL;
 
 				elemstate->innermost_caseval = (Datum *) palloc(sizeof(Datum));
 				elemstate->innermost_casenull = (bool *) palloc(sizeof(bool));
@@ -3306,6 +3298,7 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest,
 	/* we'll allocate workspace only if needed */
 	scratch->d.domaincheck.checkvalue = NULL;
 	scratch->d.domaincheck.checknull = NULL;
+	scratch->d.domaincheck.escontext = state->escontext;
 
 	/*
 	 * Evaluate argument - it's fine to directly store it into resv/resnull,
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 24c2b60c62..c8018da19f 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -63,6 +63,7 @@
 #include "executor/nodeSubplan.h"
 #include "funcapi.h"
 #include "miscadmin.h"
+#include "nodes/miscnodes.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parsetree.h"
 #include "pgstat.h"
@@ -1177,29 +1178,24 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 			/* call input function (similar to InputFunctionCall) */
 			if (!op->d.iocoerce.finfo_in->fn_strict || str != NULL)
 			{
-				FunctionCallInfo fcinfo_in;
-				Datum		d;
-
-				fcinfo_in = op->d.iocoerce.fcinfo_data_in;
-				fcinfo_in->args[0].value = PointerGetDatum(str);
-				fcinfo_in->args[0].isnull = *op->resnull;
-				/* second and third arguments are already set up */
-
-				fcinfo_in->isnull = false;
-				d = FunctionCallInvoke(fcinfo_in);
-				*op->resvalue = d;
+				/*
+				 * InputFunctionCallSafe() writes directly into *op->resvalue.
+				 * Return NULL if an error is reported.
+				 */
+				if (!InputFunctionCallSafe(op->d.iocoerce.finfo_in, str,
+										   op->d.iocoerce.typioparam, -1,
+										   (Node *) op->d.iocoerce.escontext,
+										   op->resvalue))
+					*op->resnull = true;
 
-				/* Should get null result if and only if str is NULL */
-				if (str == NULL)
-				{
+				/*
+				 * Should get null result if and only if str is NULL or if we
+				 * got an error above.
+				 */
+				if (str == NULL || SOFT_ERROR_OCCURRED(state->escontext))
 					Assert(*op->resnull);
-					Assert(fcinfo_in->isnull);
-				}
 				else
-				{
 					Assert(!*op->resnull);
-					Assert(!fcinfo_in->isnull);
-				}
 			}
 
 			EEO_NEXT();
@@ -3745,7 +3741,7 @@ ExecEvalConstraintCheck(ExprState *state, ExprEvalStep *op)
 {
 	if (!*op->d.domaincheck.checknull &&
 		!DatumGetBool(*op->d.domaincheck.checkvalue))
-		ereport(ERROR,
+		errsave((Node *) op->d.domaincheck.escontext,
 				(errcode(ERRCODE_CHECK_VIOLATION),
 				 errmsg("value for domain %s violates check constraint \"%s\"",
 						format_type_be(op->d.domaincheck.resulttype),
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 4dfaf79743..125e1e73ae 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -70,12 +70,14 @@ LLVMTypeRef StructHeapTupleTableSlot;
 LLVMTypeRef StructMinimalTupleTableSlot;
 LLVMTypeRef StructMemoryContextData;
 LLVMTypeRef StructFunctionCallInfoData;
+LLVMTypeRef StructFmgrInfo;
 LLVMTypeRef StructExprContext;
 LLVMTypeRef StructExprEvalStep;
 LLVMTypeRef StructExprState;
 LLVMTypeRef StructAggState;
 LLVMTypeRef StructAggStatePerGroupData;
 LLVMTypeRef StructAggStatePerTransData;
+LLVMTypeRef StructErrorSaveContext;
 
 LLVMValueRef AttributeTemplate;
 
@@ -1118,6 +1120,7 @@ llvm_create_types(void)
 	StructExprEvalStep = llvm_pg_var_type("StructExprEvalStep");
 	StructExprState = llvm_pg_var_type("StructExprState");
 	StructFunctionCallInfoData = llvm_pg_var_type("StructFunctionCallInfoData");
+	StructFmgrInfo = llvm_pg_var_type("StructFmgrInfo");
 	StructMemoryContextData = llvm_pg_var_type("StructMemoryContextData");
 	StructTupleTableSlot = llvm_pg_var_type("StructTupleTableSlot");
 	StructHeapTupleTableSlot = llvm_pg_var_type("StructHeapTupleTableSlot");
@@ -1127,6 +1130,7 @@ llvm_create_types(void)
 	StructAggState = llvm_pg_var_type("StructAggState");
 	StructAggStatePerGroupData = llvm_pg_var_type("StructAggStatePerGroupData");
 	StructAggStatePerTransData = llvm_pg_var_type("StructAggStatePerTransData");
+	StructErrorSaveContext = llvm_pg_var_type("StructErrorSaveContext");
 
 	AttributeTemplate = LLVMGetNamedFunction(llvm_types_module, "AttributeTemplate");
 }
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 4b51aa1ce0..7d44a4c9f4 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1251,14 +1251,9 @@ llvm_compile_expr(ExprState *state)
 
 			case EEOP_IOCOERCE:
 				{
-					FunctionCallInfo fcinfo_out,
-								fcinfo_in;
-					LLVMValueRef v_fn_out,
-								v_fn_in;
-					LLVMValueRef v_fcinfo_out,
-								v_fcinfo_in;
-					LLVMValueRef v_fcinfo_in_isnullp;
-					LLVMValueRef v_retval;
+					FunctionCallInfo fcinfo_out;
+					LLVMValueRef v_fn_out;
+					LLVMValueRef v_fcinfo_out;
 					LLVMValueRef v_resvalue;
 					LLVMValueRef v_resnull;
 
@@ -1271,7 +1266,6 @@ llvm_compile_expr(ExprState *state)
 					LLVMBasicBlockRef b_inputcall;
 
 					fcinfo_out = op->d.iocoerce.fcinfo_data_out;
-					fcinfo_in = op->d.iocoerce.fcinfo_data_in;
 
 					b_skipoutput = l_bb_before_v(opblocks[opno + 1],
 												 "op.%d.skipoutputnull", opno);
@@ -1283,14 +1277,7 @@ llvm_compile_expr(ExprState *state)
 												"op.%d.inputcall", opno);
 
 					v_fn_out = llvm_function_reference(context, b, mod, fcinfo_out);
-					v_fn_in = llvm_function_reference(context, b, mod, fcinfo_in);
 					v_fcinfo_out = l_ptr_const(fcinfo_out, l_ptr(StructFunctionCallInfoData));
-					v_fcinfo_in = l_ptr_const(fcinfo_in, l_ptr(StructFunctionCallInfoData));
-
-					v_fcinfo_in_isnullp =
-						LLVMBuildStructGEP(b, v_fcinfo_in,
-										   FIELDNO_FUNCTIONCALLINFODATA_ISNULL,
-										   "v_fcinfo_in_isnull");
 
 					/* output functions are not called on nulls */
 					v_resnull = LLVMBuildLoad(b, v_resnullp, "");
@@ -1356,24 +1343,44 @@ llvm_compile_expr(ExprState *state)
 						LLVMBuildBr(b, b_inputcall);
 					}
 
+					/*
+					 * Call the input function.
+					 *
+					 * If op->d.iocoerce.escontext references an
+					 * ErrorSaveContext, InputFunctionCallSafe() would return
+					 * false upon encountering an error.
+					 */
 					LLVMPositionBuilderAtEnd(b, b_inputcall);
-					/* set arguments */
-					/* arg0: output */
-					LLVMBuildStore(b, v_output,
-								   l_funcvaluep(b, v_fcinfo_in, 0));
-					LLVMBuildStore(b, v_resnull,
-								   l_funcnullp(b, v_fcinfo_in, 0));
-
-					/* arg1: ioparam: preset in execExpr.c */
-					/* arg2: typmod: preset in execExpr.c  */
-
-					/* reset fcinfo_in->isnull */
-					LLVMBuildStore(b, l_sbool_const(0), v_fcinfo_in_isnullp);
-					/* and call function */
-					v_retval = LLVMBuildCall(b, v_fn_in, &v_fcinfo_in, 1,
-											 "funccall_iocoerce_in");
+					{
+						Oid			ioparam = op->d.iocoerce.typioparam;
+						LLVMValueRef v_params[6];
+						LLVMValueRef v_success;
+
+						v_params[0] = l_ptr_const(op->d.iocoerce.finfo_in,
+												  l_ptr(StructFmgrInfo));
+						v_params[1] = v_output;
+						v_params[2] = l_oid_const(lc, ioparam);
+						v_params[3] = l_int32_const(lc, -1);
+						v_params[4] = l_ptr_const(op->d.iocoerce.escontext,
+												  l_ptr(StructErrorSaveContext));
 
-					LLVMBuildStore(b, v_retval, v_resvaluep);
+						/*
+						 * InputFunctionCallSafe() will write directly into
+						 * *op->resvalue.
+						 */
+						v_params[5] = v_resvaluep;
+
+						v_success = LLVMBuildCall(b, llvm_pg_func(mod, "InputFunctionCallSafe"),
+												  v_params, lengthof(v_params),
+												  "funccall_iocoerce_in_safe");
+
+						/*
+						 * Return null if InputFunctionCallSafe() encountered
+						 * an error.
+						 */
+						v_resnullp = LLVMBuildICmp(b, LLVMIntEQ, v_success,
+												   l_sbool_const(0), "");
+					}
 
 					LLVMBuildBr(b, opblocks[opno + 1]);
 					break;
diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index 41ac4c6f45..e1e9625038 100644
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -59,6 +59,7 @@ AggStatePerTransData StructAggStatePerTransData;
 ExprContext StructExprContext;
 ExprEvalStep StructExprEvalStep;
 ExprState	StructExprState;
+FmgrInfo	StructFmgrInfo;
 FunctionCallInfoBaseData StructFunctionCallInfoData;
 HeapTupleData StructHeapTupleData;
 MemoryContextData StructMemoryContextData;
@@ -66,6 +67,7 @@ TupleTableSlot StructTupleTableSlot;
 HeapTupleTableSlot StructHeapTupleTableSlot;
 MinimalTupleTableSlot StructMinimalTupleTableSlot;
 TupleDescData StructTupleDescData;
+ErrorSaveContext StructErrorSaveContext;
 
 
 /*
@@ -136,6 +138,7 @@ void	   *referenced_functions[] =
 	ExecEvalJsonConstructor,
 	ExecEvalJsonIsPredicate,
 	MakeExpandedObjectReadOnlyInternal,
+	InputFunctionCallSafe,
 	slot_getmissingattrs,
 	slot_getsomeattrs_int,
 	strlen,
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index 048573c2bc..59f3b043c6 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -16,6 +16,7 @@
 
 #include "executor/nodeAgg.h"
 #include "nodes/execnodes.h"
+#include "nodes/miscnodes.h"
 
 /* forward references to avoid circularity */
 struct ExprEvalStep;
@@ -416,7 +417,8 @@ typedef struct ExprEvalStep
 			FunctionCallInfo fcinfo_data_out;
 			/* lookup and call info for result type's input function */
 			FmgrInfo   *finfo_in;
-			FunctionCallInfo fcinfo_data_in;
+			Oid			typioparam;
+			ErrorSaveContext *escontext;
 		}			iocoerce;
 
 		/* for EEOP_SQLVALUEFUNCTION */
@@ -547,6 +549,7 @@ typedef struct ExprEvalStep
 			bool	   *checknull;
 			/* OID of domain type */
 			Oid			resulttype;
+			ErrorSaveContext *escontext;
 		}			domaincheck;
 
 		/* for EEOP_CONVERT_ROWTYPE */
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 6d90a16f79..5b7681eba9 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -75,6 +75,7 @@ extern PGDLLIMPORT LLVMTypeRef StructTupleTableSlot;
 extern PGDLLIMPORT LLVMTypeRef StructHeapTupleTableSlot;
 extern PGDLLIMPORT LLVMTypeRef StructMinimalTupleTableSlot;
 extern PGDLLIMPORT LLVMTypeRef StructMemoryContextData;
+extern PGDLLIMPORT LLVMTypeRef StructFmgrInfo;
 extern PGDLLIMPORT LLVMTypeRef StructFunctionCallInfoData;
 extern PGDLLIMPORT LLVMTypeRef StructExprContext;
 extern PGDLLIMPORT LLVMTypeRef StructExprEvalStep;
@@ -82,6 +83,7 @@ extern PGDLLIMPORT LLVMTypeRef StructExprState;
 extern PGDLLIMPORT LLVMTypeRef StructAggState;
 extern PGDLLIMPORT LLVMTypeRef StructAggStatePerTransData;
 extern PGDLLIMPORT LLVMTypeRef StructAggStatePerGroupData;
+extern PGDLLIMPORT LLVMTypeRef StructErrorSaveContext;
 
 extern PGDLLIMPORT LLVMValueRef AttributeTemplate;
 
diff --git a/src/include/jit/llvmjit_emit.h b/src/include/jit/llvmjit_emit.h
index 5e74543be4..ead46a64ae 100644
--- a/src/include/jit/llvmjit_emit.h
+++ b/src/include/jit/llvmjit_emit.h
@@ -85,6 +85,15 @@ l_sizet_const(size_t i)
 	return LLVMConstInt(TypeSizeT, i, false);
 }
 
+/*
+ * Emit constant oid.
+ */
+static inline LLVMValueRef
+l_oid_const(LLVMContextRef lc, Oid i)
+{
+	return LLVMConstInt(LLVMInt32TypeInContext(lc), i, false);
+}
+
 /*
  * Emit constant boolean, as used for storage (e.g. global vars, structs).
  */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cb714f4a19..61d97b30bc 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -34,6 +34,7 @@
 #include "fmgr.h"
 #include "lib/ilist.h"
 #include "lib/pairingheap.h"
+#include "nodes/miscnodes.h"
 #include "nodes/params.h"
 #include "nodes/plannodes.h"
 #include "nodes/tidbitmap.h"
@@ -129,6 +130,12 @@ typedef struct ExprState
 
 	Datum	   *innermost_domainval;
 	bool	   *innermost_domainnull;
+
+	/*
+	 * For expression nodes that support soft errors.  Should be set to NULL
+	 * before calling ExecInitExprRec() if the caller wants errors thrown.
+	 */
+	ErrorSaveContext *escontext;
 } ExprState;
 
 
-- 
2.35.3