From d0652f596bb61b07fdb2ccadb8e897885c047701 Mon Sep 17 00:00:00 2001 From: Amit Langote 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