v13-0001-Support-soft-error-handling-during-CoerceViaIO-e.patch

application/octet-stream

Filename: v13-0001-Support-soft-error-handling-during-CoerceViaIO-e.patch
Type: application/octet-stream
Part: 1
Message: Re: remaining sql/json patches

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v13-0001
Subject: Support soft error handling during CoerceViaIO evaluation
File+
src/backend/executor/execExpr.c 26 18
src/backend/executor/execExprInterp.c 35 19
src/backend/jit/llvm/llvmjit.c 1 0
src/backend/jit/llvm/llvmjit_expr.c 43 32
src/backend/jit/llvm/llvmjit_types.c 2 0
src/include/executor/execExpr.h 3 1
src/include/jit/llvmjit.h 1 0
src/include/nodes/execnodes.h 7 0
From d0652f596bb61b07fdb2ccadb8e897885c047701 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 30 Aug 2023 21:47:01 +0900
Subject: [PATCH v13 1/4] Support soft error handling during CoerceViaIO
 evaluation

This teaches the EEOP_IOCOERCE expression evaluation step code to
use InputFunctionCallSafe() instead of calling the type input
function directly.

This is to allow the SQL/JSON query functions to suppress errors
that may occur when evaluating coercions to handle clauses such
as NULL ON ERROR.
---
 src/backend/executor/execExpr.c       | 44 +++++++++-------
 src/backend/executor/execExprInterp.c | 54 ++++++++++++-------
 src/backend/jit/llvm/llvmjit.c        |  1 +
 src/backend/jit/llvm/llvmjit_expr.c   | 75 +++++++++++++++------------
 src/backend/jit/llvm/llvmjit_types.c  |  2 +
 src/include/executor/execExpr.h       |  4 +-
 src/include/jit/llvmjit.h             |  1 +
 src/include/nodes/execnodes.h         |  7 +++
 8 files changed, 118 insertions(+), 70 deletions(-)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 2c62b0c9c8..fcb3719ffa 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -40,6 +40,7 @@
 #include "jit/jit.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
+#include "nodes/miscnodes.h"
 #include "nodes/nodeFuncs.h"
 #include "nodes/subscripting.h"
 #include "optimizer/optimizer.h"
@@ -140,6 +141,12 @@ ExecInitExpr(Expr *node, PlanState *parent)
 	state->parent = parent;
 	state->ext_params = NULL;
 
+	/*
+	 * ExecExprEnableErrorSafe() must be called on this ExprState before
+	 * passing this on to modules that support soft error handling.
+	 */
+	state->escontext = palloc0(sizeof(ErrorSaveContext));
+
 	/* Insert setup steps as needed */
 	ExecCreateExprSetupSteps(state, (Node *) node);
 
@@ -177,6 +184,12 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params)
 	state->parent = NULL;
 	state->ext_params = ext_params;
 
+	/*
+	 * ExecExprEnableErrorSafe() must be called on this ExprState before
+	 * passing this on to modules that support soft error handling.
+	 */
+	state->escontext = palloc0(sizeof(ErrorSaveContext));
+
 	/* Insert setup steps as needed */
 	ExecCreateExprSetupSteps(state, (Node *) node);
 
@@ -229,6 +242,12 @@ ExecInitQual(List *qual, PlanState *parent)
 	state->parent = parent;
 	state->ext_params = NULL;
 
+	/*
+	 * ExecExprEnableErrorSafe() must be called on this ExprState before
+	 * passing this on to modules that support soft error handling.
+	 */
+	state->escontext = palloc0(sizeof(ErrorSaveContext));
+
 	/* mark expression as to be used with ExecQual() */
 	state->flags = EEO_FLAG_IS_QUAL;
 
@@ -374,6 +393,12 @@ ExecBuildProjectionInfo(List *targetList,
 	state->parent = parent;
 	state->ext_params = NULL;
 
+	/*
+	 * ExecExprEnableErrorSafe() must be called on this ExprState before
+	 * passing this on to modules that support soft error handling.
+	 */
+	state->escontext = palloc0(sizeof(ErrorSaveContext));
+
 	state->resultslot = slot;
 
 	/* Insert setup steps as needed */
@@ -1549,8 +1574,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 +1602,10 @@ 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;
 
 				ExprEvalPushStep(state, &scratch);
 				break;
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 24c2b60c62..d774bf9239 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,22 @@ 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,
+										   state->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();
@@ -1849,6 +1843,28 @@ out:
 	return state->resvalue;
 }
 
+/*
+ * ExecExprEnableErrorSafe / ExecExprDisableErrorSafe
+ *		Initiate or stop "soft" error handling during expression evaluation
+ *
+ * Note that not all errors become soft errors, only the errors that can occur
+ * when evaluating expressions whose implementation supports soft error
+ * handling.  For example, CoerceViaIO, which uses InputFunctionCallSafe().
+ */
+void
+ExecExprEnableErrorSafe(ExprState *state)
+{
+	Assert(state->escontext);
+	((ErrorSaveContext *) state->escontext)->type = T_ErrorSaveContext;
+}
+
+void
+ExecExprDisableErrorSafe(ExprState *state)
+{
+	Assert(state->escontext);
+	memset(state->escontext, 0, sizeof(ErrorSaveContext));
+}
+
 /*
  * Expression evaluation callback that performs extra checks before executing
  * the expression. Declared extern so other methods of execution can use it
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 09650e2c70..3e96d11d7e 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -1024,6 +1024,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");
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 00d7b8110b..dfb5035d1f 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -92,6 +92,7 @@ llvm_compile_expr(ExprState *state)
 	LLVMValueRef v_state;
 	LLVMValueRef v_econtext;
 	LLVMValueRef v_parent;
+	LLVMValueRef v_escontext;
 
 	/* returnvalue */
 	LLVMValueRef v_isnullp;
@@ -173,6 +174,9 @@ llvm_compile_expr(ExprState *state)
 	v_parent = l_load_struct_gep(b, v_state,
 								 FIELDNO_EXPRSTATE_PARENT,
 								 "v.state.parent");
+	v_escontext = l_load_struct_gep(b, v_state,
+									FIELDNO_EXPRSTATE_ESCONTEXT,
+									"v.state.escontext");
 
 	/* build global slots */
 	v_scanslot = l_load_struct_gep(b, v_econtext,
@@ -1243,14 +1247,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;
 
@@ -1263,7 +1262,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);
@@ -1275,14 +1273,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, "");
@@ -1348,24 +1339,44 @@ llvm_compile_expr(ExprState *state)
 						LLVMBuildBr(b, b_inputcall);
 					}
 
+					/*
+					 * Call the input function.
+					 *
+					 * If the caller did ExecExprEnableErrorSafe() before
+					 * evaluating this EEOP_IOCOERCE expression,
+					 * 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] = v_escontext;
+						/*
+						 * InputFunctionCallSafe() 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
+						 * on encountering 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..2cbbab2b61 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;
@@ -136,6 +137,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..3ec185bfd2 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -416,7 +416,7 @@ 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;
 		}			iocoerce;
 
 		/* for EEOP_SQLVALUEFUNCTION */
@@ -762,6 +762,8 @@ extern ExprEvalOp ExecEvalStepOp(ExprState *state, ExprEvalStep *op);
 
 extern Datum ExecInterpExprStillValid(ExprState *state, ExprContext *econtext, bool *isNull);
 extern void CheckExprStillValid(ExprState *state, ExprContext *econtext);
+extern void ExecExprEnableErrorSafe(ExprState *state);
+extern void ExecExprDisableErrorSafe(ExprState *state);
 
 /*
  * Non fast-path execution functions. These are externs instead of statics in
diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h
index 551b585464..966ae6e600 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;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cb714f4a19..453d69dead 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -124,6 +124,13 @@ typedef struct ExprState
 	struct PlanState *parent;	/* parent PlanState node, if any */
 	ParamListInfo ext_params;	/* for compiling PARAM_EXTERN nodes */
 
+#define FIELDNO_EXPRSTATE_ESCONTEXT 13
+	/*
+	 * This contains ErrorSaveContext for soft-error capture during expression
+	 * evaluation.
+	 */
+	Node	   *escontext;
+
 	Datum	   *innermost_caseval;
 	bool	   *innermost_casenull;
 
-- 
2.35.3