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