v16-0001-Support-for-soft-error-handling-during-CoerceVia.patch

From 4f7216fc9b27d6bd5f0d2615912187e841d477b4 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 30 Aug 2023 21:47:01 +0900
Subject: [PATCH v16 1/5] Support for soft error handling during CoerceViaIO
 evaluation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This changes the CoerceViaIO expression evaluation code to use
InputFunctionCallSafe(), which provides the option to handle errors
softly, instead of calling the type input function directly.  To
pass it the necessary ErrorSaveContext, ExprState.escontext must
be set to point to it before calling ExecInitExprRec() for an
an expression tree that might possibly contain a CoerceViaIO node.

Note that no caller of ExecInitExprRec() has been changed to pass
the ErrorSaveContext, so there's no functional change yet.

Reviewed-by: Álvaro Herrera
---
 src/backend/executor/execExpr.c       | 26 ++++------
 src/backend/executor/execExprInterp.c | 33 ++++++-------
 src/backend/jit/llvm/llvmjit.c        |  3 ++
 src/backend/jit/llvm/llvmjit_expr.c   | 71 +++++++++++++++------------
 src/backend/jit/llvm/llvmjit_types.c  |  3 ++
 src/include/executor/execExpr.h       |  4 +-
 src/include/jit/llvmjit.h             |  2 +
 src/include/nodes/execnodes.h         |  7 +++
 8 files changed, 80 insertions(+), 69 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 2c62b0c9c8..66d0ae101b 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));
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 24c2b60c62..86dff69f4d 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,23 @@ 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.
+				 */
+				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();
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 09650e2c70..431d4511c5 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -85,6 +85,7 @@ LLVMTypeRef StructExprState;
 LLVMTypeRef StructAggState;
 LLVMTypeRef StructAggStatePerGroupData;
 LLVMTypeRef StructAggStatePerTransData;
+LLVMTypeRef StructErrorSaveContext;
 
 LLVMValueRef AttributeTemplate;
 
@@ -1024,6 +1025,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");
@@ -1033,6 +1035,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 2ac335e238..59e20ebcbd 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -1249,14 +1249,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;
 
@@ -1269,7 +1264,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);
@@ -1281,14 +1275,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, "");
@@ -1354,24 +1341,44 @@ llvm_compile_expr(ExprState *state)
 						LLVMBuildBr(b, b_inputcall);
 					}
 
+					/*
+					 * Call the input function.
+					 *
+					 * If op->d.iocoerce.escontext has been set,
+					 * InputFunctionCallSafe() would return false if an error
+					 * occurred during the input processing.
+					 */
 					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");
+					{
+						/* ioparam and typmod preset in execExpr.c */
+						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_int32_const(ioparam);
+						v_params[3] = l_int32_const(-1);
+						v_params[4] = l_ptr_const(op->d.iocoerce.escontext,
+												  l_ptr(StructErrorSaveContext));
+						/*
+						 * InputFunctionCallSafe() will writes directly into
+						 * *op->resvalue.
+						 */
+						v_params[5] = v_resvaluep;
 
-					LLVMBuildStore(b, v_retval, v_resvaluep);
+						v_success = LLVMBuildCall(b, llvm_pg_func(mod, "InputFunctionCallSafe"),
+												  v_params, lengthof(v_params),
+												  "funccall_iocoerce_in_safe");
+
+						/*
+						 * Return null if InputFunctionCallSafe() returned false
+						 * because of an error.
+						 */
+						v_resnullp = LLVMBuildZExt(b, v_success,
+												   TypeStorageBool, "");
+					}
 
 					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..8dd92f8fc0 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 */
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 551b585464..c22ba3787f 100644
--- a/src/include/jit/llvmjit.h
+++ b/src/include/jit/llvmjit.h
@@ -71,6 +71,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;
@@ -78,6 +79,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/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cb714f4a19..cc4ed9279c 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;
+
+	/*
+	 * Used by the callers of ExecInitExprRec() to pass the ErrorSaveContext
+	 * that they want the expression's code to use.
+	 */
+	ErrorSaveContext *escontext;
 } ExprState;
 
 
-- 
2.35.3