Skip to content

Commit aaaf944

Browse files
amitlanamitlan
committed
Add soft error handling to some expression nodes
This adjusts the code for CoerceViaIO and CoerceToDomain expression nodes to handle errors softly. For CoerceViaIo, this adds a new ExprEvalStep opcode EEOP_IOCOERCE_SAFE, which is implemented in the new accompanying 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 ExecEvalConstraintNotNull() and ExecEvalConstraintCheck() by errsave() passing it the ErrorSaveContext passed in the expression's ExprEvalStep. In both cases, the ErrorSaveContext to be used is passed by setting ExprState.escontext to point to it before calling ExecInitExprRec() on the expression tree whose errors are to be handled softly. Note that there's no functional change as of this commit as no call site of ExecInitExprRec() has been changed. This is intended for implementing new SQL/JSON expression nodes in future commits. Extracted from a much larger patch to add SQL/JSON query functions. Author: Nikita Glukhov <n.gluhov@postgrespro.ru> Author: Teodor Sigaev <teodor@sigaev.ru> Author: Oleg Bartunov <obartunov@gmail.com> Author: Alexander Korotkov <aekorotkov@gmail.com> Author: Andrew Dunstan <andrew@dunslane.net> Author: Amit Langote <amitlangote09@gmail.com> Reviewers have included (in no particular order) Andres Freund, Alexander Korotkov, Pavel Stehule, Andrew Alsup, Erik Rijkers, Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson, Justin Pryzby, Álvaro Herrera, Jian He, Peter Eisentraut Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org Discussion: https://postgr.es/m/CA+HiwqHROpf9e644D8BRqYvaAPmgBZVup-xKMDPk-nd4EpgzHw@mail.gmail.com Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
1 parent bb812ab commit aaaf944

File tree

6 files changed

+103
-3
lines changed

6 files changed

+103
-3
lines changed

‎src/backend/executor/execExpr.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1560,7 +1560,10 @@ ExecInitExprRec(Expr *node, ExprState *state,
15601560
* We don't check permissions here as a type's input/output
15611561
* function are assumed to be executable by everyone.
15621562
*/
1563-
scratch.opcode = EEOP_IOCOERCE;
1563+
if (state->escontext == NULL)
1564+
scratch.opcode = EEOP_IOCOERCE;
1565+
else
1566+
scratch.opcode = EEOP_IOCOERCE_SAFE;
15641567

15651568
/* lookup the source type's output function */
15661569
scratch.d.iocoerce.finfo_out = palloc0(sizeof(FmgrInfo));
@@ -1596,6 +1599,8 @@ ExecInitExprRec(Expr *node, ExprState *state,
15961599
fcinfo_in->args[2].value = Int32GetDatum(-1);
15971600
fcinfo_in->args[2].isnull = false;
15981601

1602+
fcinfo_in->context = (Node *) state->escontext;
1603+
15991604
ExprEvalPushStep(state, &scratch);
16001605
break;
16011606
}
@@ -3303,6 +3308,7 @@ ExecInitCoerceToDomain(ExprEvalStep *scratch, CoerceToDomain *ctest,
33033308
/* we'll allocate workspace only if needed */
33043309
scratch->d.domaincheck.checkvalue = NULL;
33053310
scratch->d.domaincheck.checknull = NULL;
3311+
scratch->d.domaincheck.escontext = state->escontext;
33063312

33073313
/*
33083314
* Evaluate argument - it's fine to directly store it into resv/resnull,

‎src/backend/executor/execExprInterp.c

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
#include "executor/nodeSubplan.h"
6464
#include "funcapi.h"
6565
#include "miscadmin.h"
66+
#include "nodes/miscnodes.h"
6667
#include "nodes/nodeFuncs.h"
6768
#include "parser/parsetree.h"
6869
#include "pgstat.h"
@@ -452,6 +453,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
452453
&&CASE_EEOP_CASE_TESTVAL,
453454
&&CASE_EEOP_MAKE_READONLY,
454455
&&CASE_EEOP_IOCOERCE,
456+
&&CASE_EEOP_IOCOERCE_SAFE,
455457
&&CASE_EEOP_DISTINCT,
456458
&&CASE_EEOP_NOT_DISTINCT,
457459
&&CASE_EEOP_NULLIF,
@@ -1150,6 +1152,9 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
11501152
* Evaluate a CoerceViaIO node. This can be quite a hot path, so
11511153
* inline as much work as possible. The source value is in our
11521154
* result variable.
1155+
*
1156+
* Also look at ExecEvalCoerceViaIOSafe() if you change anything
1157+
* here.
11531158
*/
11541159
char *str;
11551160

@@ -1205,6 +1210,12 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
12051210
EEO_NEXT();
12061211
}
12071212

1213+
EEO_CASE(EEOP_IOCOERCE_SAFE)
1214+
{
1215+
ExecEvalCoerceViaIOSafe(state, op);
1216+
EEO_NEXT();
1217+
}
1218+
12081219
EEO_CASE(EEOP_DISTINCT)
12091220
{
12101221
/*
@@ -2510,6 +2521,71 @@ ExecEvalParamExtern(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
25102521
errmsg("no value found for parameter %d", paramId)));
25112522
}
25122523

2524+
/*
2525+
* Evaluate a CoerceViaIO node in soft-error mode.
2526+
*
2527+
* The source value is in op's result variable.
2528+
*
2529+
* Note: This implements EEOP_IOCOERCE_SAFE. If you change anything here,
2530+
* also look at the inline code for EEOP_IOCOERCE.
2531+
*/
2532+
void
2533+
ExecEvalCoerceViaIOSafe(ExprState *state, ExprEvalStep *op)
2534+
{
2535+
char *str;
2536+
2537+
/* call output function (similar to OutputFunctionCall) */
2538+
if (*op->resnull)
2539+
{
2540+
/* output functions are not called on nulls */
2541+
str = NULL;
2542+
}
2543+
else
2544+
{
2545+
FunctionCallInfo fcinfo_out;
2546+
2547+
fcinfo_out = op->d.iocoerce.fcinfo_data_out;
2548+
fcinfo_out->args[0].value = *op->resvalue;
2549+
fcinfo_out->args[0].isnull = false;
2550+
2551+
fcinfo_out->isnull = false;
2552+
str = DatumGetCString(FunctionCallInvoke(fcinfo_out));
2553+
2554+
/* OutputFunctionCall assumes result isn't null */
2555+
Assert(!fcinfo_out->isnull);
2556+
}
2557+
2558+
/* call input function (similar to InputFunctionCallSafe) */
2559+
if (!op->d.iocoerce.finfo_in->fn_strict || str != NULL)
2560+
{
2561+
FunctionCallInfo fcinfo_in;
2562+
2563+
fcinfo_in = op->d.iocoerce.fcinfo_data_in;
2564+
fcinfo_in->args[0].value = PointerGetDatum(str);
2565+
fcinfo_in->args[0].isnull = *op->resnull;
2566+
/* second and third arguments are already set up */
2567+
2568+
/* ErrorSaveContext must be present. */
2569+
Assert(IsA(fcinfo_in->context, ErrorSaveContext));
2570+
2571+
fcinfo_in->isnull = false;
2572+
*op->resvalue = FunctionCallInvoke(fcinfo_in);
2573+
2574+
if (SOFT_ERROR_OCCURRED(fcinfo_in->context))
2575+
{
2576+
*op->resnull = true;
2577+
*op->resvalue = (Datum) 0;
2578+
return;
2579+
}
2580+
2581+
/* Should get null result if and only if str is NULL */
2582+
if (str == NULL)
2583+
Assert(*op->resnull);
2584+
else
2585+
Assert(!*op->resnull);
2586+
}
2587+
}
2588+
25132589
/*
25142590
* Evaluate a SQLValueFunction expression.
25152591
*/
@@ -3730,7 +3806,7 @@ void
37303806
ExecEvalConstraintNotNull(ExprState *state, ExprEvalStep *op)
37313807
{
37323808
if (*op->resnull)
3733-
ereport(ERROR,
3809+
errsave((Node *) op->d.domaincheck.escontext,
37343810
(errcode(ERRCODE_NOT_NULL_VIOLATION),
37353811
errmsg("domain %s does not allow null values",
37363812
format_type_be(op->d.domaincheck.resulttype)),
@@ -3745,7 +3821,7 @@ ExecEvalConstraintCheck(ExprState *state, ExprEvalStep *op)
37453821
{
37463822
if (!*op->d.domaincheck.checknull &&
37473823
!DatumGetBool(*op->d.domaincheck.checkvalue))
3748-
ereport(ERROR,
3824+
errsave((Node *) op->d.domaincheck.escontext,
37493825
(errcode(ERRCODE_CHECK_VIOLATION),
37503826
errmsg("value for domain %s violates check constraint \"%s\"",
37513827
format_type_be(op->d.domaincheck.resulttype),

‎src/backend/jit/llvm/llvmjit_expr.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,6 +1431,12 @@ llvm_compile_expr(ExprState *state)
14311431
break;
14321432
}
14331433

1434+
case EEOP_IOCOERCE_SAFE:
1435+
build_EvalXFunc(b, mod, "ExecEvalCoerceViaIOSafe",
1436+
v_state, op);
1437+
LLVMBuildBr(b, opblocks[opno + 1]);
1438+
break;
1439+
14341440
case EEOP_DISTINCT:
14351441
case EEOP_NOT_DISTINCT:
14361442
{

‎src/backend/jit/llvm/llvmjit_types.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ void *referenced_functions[] =
162162
ExecEvalRow,
163163
ExecEvalRowNotNull,
164164
ExecEvalRowNull,
165+
ExecEvalCoerceViaIOSafe,
165166
ExecEvalSQLValueFunction,
166167
ExecEvalScalarArrayOp,
167168
ExecEvalHashedScalarArrayOp,

‎src/include/executor/execExpr.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "executor/nodeAgg.h"
1818
#include "nodes/execnodes.h"
19+
#include "nodes/miscnodes.h"
1920

2021
/* forward references to avoid circularity */
2122
struct ExprEvalStep;
@@ -168,6 +169,7 @@ typedef enum ExprEvalOp
168169

169170
/* evaluate assorted special-purpose expression types */
170171
EEOP_IOCOERCE,
172+
EEOP_IOCOERCE_SAFE,
171173
EEOP_DISTINCT,
172174
EEOP_NOT_DISTINCT,
173175
EEOP_NULLIF,
@@ -547,6 +549,7 @@ typedef struct ExprEvalStep
547549
bool *checknull;
548550
/* OID of domain type */
549551
Oid resulttype;
552+
ErrorSaveContext *escontext;
550553
} domaincheck;
551554

552555
/* for EEOP_CONVERT_ROWTYPE */
@@ -776,6 +779,7 @@ extern void ExecEvalParamExec(ExprState *state, ExprEvalStep *op,
776779
ExprContext *econtext);
777780
extern void ExecEvalParamExtern(ExprState *state, ExprEvalStep *op,
778781
ExprContext *econtext);
782+
extern void ExecEvalCoerceViaIOSafe(ExprState *state, ExprEvalStep *op);
779783
extern void ExecEvalSQLValueFunction(ExprState *state, ExprEvalStep *op);
780784
extern void ExecEvalCurrentOfExpr(ExprState *state, ExprEvalStep *op);
781785
extern void ExecEvalNextValueExpr(ExprState *state, ExprEvalStep *op);

‎src/include/nodes/execnodes.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "fmgr.h"
3535
#include "lib/ilist.h"
3636
#include "lib/pairingheap.h"
37+
#include "nodes/miscnodes.h"
3738
#include "nodes/params.h"
3839
#include "nodes/plannodes.h"
3940
#include "nodes/tidbitmap.h"
@@ -129,6 +130,12 @@ typedef struct ExprState
129130

130131
Datum *innermost_domainval;
131132
bool *innermost_domainnull;
133+
134+
/*
135+
* For expression nodes that support soft errors. Should be set to NULL
136+
* before calling ExecInitExprRec() if the caller wants errors thrown.
137+
*/
138+
ErrorSaveContext *escontext;
132139
} ExprState;
133140

134141

0 commit comments

Comments
 (0)