From 9b43676efa45878779dbd0ae98b0bac8d41cbd8a Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 29 Sep 2023 13:22:15 +0900 Subject: [PATCH v23] Add soft error handling to some expression nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adjusts the CoerceViaIO and CoerceToDomain expression evaluation code to handle errors softly. For CoerceViaIo, this adds a new ExprEvalStep opcode EEOP_IOCOERCE_SAFE, which is implemented in new function ExecEvalCoerceViaIOSafe(). The only difference from EEOP_IOCOERCE's inline implementation is that the input function receives an ErrorSaveContext via the function's FunctionCallInfo.context, which it can use to handle errors softly. 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 ErrorSaveContext can be passed by setting ExprState.escontext to point to it before calling ExecInitExprRec() on the expression tree whose errors are to be suppressed. Note that no call site of ExecInitExprRec() has been changed in this commit, so there's no functional change. This is intended for implementing new SQL/JSON expression nodes in future commits that will use to it suppress errors that may occur during type coercions. Reviewed-by: Álvaro Herrera Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com --- src/backend/executor/execExpr.c | 8 ++- src/backend/executor/execExprInterp.c | 72 ++++++++++++++++++++++++++- src/backend/jit/llvm/llvmjit.c | 4 ++ src/backend/jit/llvm/llvmjit_expr.c | 6 +++ src/backend/jit/llvm/llvmjit_types.c | 4 ++ src/include/executor/execExpr.h | 4 ++ src/include/jit/llvmjit.h | 2 + src/include/jit/llvmjit_emit.h | 9 ++++ src/include/nodes/execnodes.h | 7 +++ 9 files changed, 114 insertions(+), 2 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 2c62b0c9c8..34bd2102b5 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1563,7 +1563,10 @@ ExecInitExprRec(Expr *node, ExprState *state, * We don't check permissions here as a type's input/output * function are assumed to be executable by everyone. */ - scratch.opcode = EEOP_IOCOERCE; + if (state->escontext == NULL) + scratch.opcode = EEOP_IOCOERCE; + else + scratch.opcode = EEOP_IOCOERCE_SAFE; /* lookup the source type's output function */ scratch.d.iocoerce.finfo_out = palloc0(sizeof(FmgrInfo)); @@ -1599,6 +1602,8 @@ ExecInitExprRec(Expr *node, ExprState *state, fcinfo_in->args[2].value = Int32GetDatum(-1); fcinfo_in->args[2].isnull = false; + fcinfo_in->context = (Node *) state->escontext; + ExprEvalPushStep(state, &scratch); break; } @@ -3306,6 +3311,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..4e152fdfe3 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" @@ -452,6 +453,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) &&CASE_EEOP_CASE_TESTVAL, &&CASE_EEOP_MAKE_READONLY, &&CASE_EEOP_IOCOERCE, + &&CASE_EEOP_IOCOERCE_SAFE, &&CASE_EEOP_DISTINCT, &&CASE_EEOP_NOT_DISTINCT, &&CASE_EEOP_NULLIF, @@ -1205,6 +1207,12 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_NEXT(); } + EEO_CASE(EEOP_IOCOERCE_SAFE) + { + ExecEvalCoerceViaIOSafe(state, op); + EEO_NEXT(); + } + EEO_CASE(EEOP_DISTINCT) { /* @@ -2510,6 +2518,68 @@ ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, ExprContext *econtext) errmsg("no value found for parameter %d", paramId))); } +/* + * Evaluate a CoerceViaIO node in soft-error mode. + * + * The source value is in op's result variable. + */ +void +ExecEvalCoerceViaIOSafe(ExprState *state, ExprEvalStep *op) +{ + char *str; + + /* call output function (similar to OutputFunctionCall) */ + if (*op->resnull) + { + /* output functions are not called on nulls */ + str = NULL; + } + else + { + FunctionCallInfo fcinfo_out; + + fcinfo_out = op->d.iocoerce.fcinfo_data_out; + fcinfo_out->args[0].value = *op->resvalue; + fcinfo_out->args[0].isnull = false; + + fcinfo_out->isnull = false; + str = DatumGetCString(FunctionCallInvoke(fcinfo_out)); + + /* OutputFunctionCall assumes result isn't null */ + Assert(!fcinfo_out->isnull); + } + + /* call input function (similar to InputFunctionCall) */ + if (!op->d.iocoerce.finfo_in->fn_strict || str != NULL) + { + FunctionCallInfo fcinfo_in; + + 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 */ + + /* ErrorSaveContext must be present. */ + Assert(IsA(fcinfo_in->context, ErrorSaveContext)); + + fcinfo_in->isnull = false; + *op->resvalue = FunctionCallInvoke(fcinfo_in); + + if (SOFT_ERROR_OCCURRED(fcinfo_in->context)) + { + *op->resnull = true; + *op->resvalue = (Datum) 0; + return; + } + + /* Should get null result if and only if str is NULL */ + if (str == NULL) + Assert(*op->resnull); + else + Assert(!*op->resnull); + } +} + /* * Evaluate a SQLValueFunction expression. */ @@ -3745,7 +3815,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..5a9be73957 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 StructNode; 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"); + StructNode = llvm_pg_var_type("StructNode"); 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..750acea898 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -1379,6 +1379,12 @@ llvm_compile_expr(ExprState *state) break; } + case EEOP_IOCOERCE_SAFE: + build_EvalXFunc(b, mod, "ExecEvalCoerceViaIOSafe", + v_state, op); + LLVMBuildBr(b, opblocks[opno + 1]); + break; + case EEOP_DISTINCT: case EEOP_NOT_DISTINCT: { diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c index 41ac4c6f45..c034533dc0 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; +Node StructNode; /* @@ -126,6 +128,7 @@ void *referenced_functions[] = ExecEvalRow, ExecEvalRowNotNull, ExecEvalRowNull, + ExecEvalCoerceViaIOSafe, ExecEvalSQLValueFunction, ExecEvalScalarArrayOp, ExecEvalHashedScalarArrayOp, @@ -136,6 +139,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..c4fd933154 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; @@ -168,6 +169,7 @@ typedef enum ExprEvalOp /* evaluate assorted special-purpose expression types */ EEOP_IOCOERCE, + EEOP_IOCOERCE_SAFE, EEOP_DISTINCT, EEOP_NOT_DISTINCT, EEOP_NULLIF, @@ -547,6 +549,7 @@ typedef struct ExprEvalStep bool *checknull; /* OID of domain type */ Oid resulttype; + ErrorSaveContext *escontext; } domaincheck; /* for EEOP_CONVERT_ROWTYPE */ @@ -776,6 +779,7 @@ extern void ExecEvalParamExec(ExprState *state, ExprEvalStep *op, ExprContext *econtext); extern void ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, ExprContext *econtext); +extern void ExecEvalCoerceViaIOSafe(ExprState *state, ExprEvalStep *op); extern void ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep *op); extern void ExecEvalCurrentOfExpr(ExprState *state, ExprEvalStep *op); extern void ExecEvalNextValueExpr(ExprState *state, ExprEvalStep *op); diff --git a/src/include/jit/llvmjit.h b/src/include/jit/llvmjit.h index 6d90a16f79..e4ff3be566 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 StructNode; 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 108d69ba28..24e55c4578 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