SQL/JSON: Fix error-handling of some JsonBehavior expressions

To ensure that the errors of executing a JsonBehavior expression that
is coerced in the parser are caught instead of being thrown directly,
pass ErrorSaveContext to ExecInitExprRec() when initializing it.
Also, add a EEOP_JSONEXPR_COERCION_FINISH step to handle the errors
that are caught that way.

Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index ccd4863778429c744b65c124e0b8da50c26937fc..b10359e3d6be119ebf52b61c2edc7f2fc709fa9a 100644 (file)
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -4400,6 +4400,8 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
        if (jsexpr->on_error &&
                jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR)
        {
+               ErrorSaveContext *saved_escontext;
+
                jsestate->jump_error = state->steps_len;
 
                /* JUMP to end if false, that is, skip the ON ERROR expression. */
@@ -4410,15 +4412,36 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
                scratch->d.jump.jumpdone = -1;  /* set below */
                ExprEvalPushStep(state, scratch);
 
-               /* Steps to evaluate the ON ERROR expression */
+               /*
+                * Steps to evaluate the ON ERROR expression; handle errors softly to
+                * rethrow them in COERCION_FINISH step that will be added later.
+                */
+               saved_escontext = state->escontext;
+               state->escontext = escontext;
                ExecInitExprRec((Expr *) jsexpr->on_error->expr,
                                                state, resv, resnull);
+               state->escontext = saved_escontext;
 
                /* Step to coerce the ON ERROR expression if needed */
                if (jsexpr->on_error->coerce)
                        ExecInitJsonCoercion(state, jsexpr->returning, escontext,
                                                                 jsexpr->omit_quotes, resv, resnull);
 
+               /*
+                * Add a COERCION_FINISH step to check for errors that may occur when
+                * coercing and rethrow them.
+                */
+               if (jsexpr->on_error->coerce ||
+                       IsA(jsexpr->on_error->expr, CoerceViaIO) ||
+                       IsA(jsexpr->on_error->expr, CoerceToDomain))
+               {
+                       scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH;
+                       scratch->resvalue = resv;
+                       scratch->resnull = resnull;
+                       scratch->d.jsonexpr.jsestate = jsestate;
+                       ExprEvalPushStep(state, scratch);
+               }
+
                /* JUMP to end to skip the ON EMPTY steps added below. */
                jumps_to_end = lappend_int(jumps_to_end, state->steps_len);
                scratch->opcode = EEOP_JUMP;
@@ -4433,6 +4456,8 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
        if (jsexpr->on_empty != NULL &&
                jsexpr->on_empty->btype != JSON_BEHAVIOR_ERROR)
        {
+               ErrorSaveContext *saved_escontext;
+
                jsestate->jump_empty = state->steps_len;
 
                /* JUMP to end if false, that is, skip the ON EMPTY expression. */
@@ -4443,14 +4468,36 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state,
                scratch->d.jump.jumpdone = -1;  /* set below */
                ExprEvalPushStep(state, scratch);
 
-               /* Steps to evaluate the ON EMPTY expression */
+               /*
+                * Steps to evaluate the ON EMPTY expression; handle errors softly to
+                * rethrow them in COERCION_FINISH step that will be added later.
+                */
+               saved_escontext = state->escontext;
+               state->escontext = escontext;
                ExecInitExprRec((Expr *) jsexpr->on_empty->expr,
                                                state, resv, resnull);
+               state->escontext = saved_escontext;
 
                /* Step to coerce the ON EMPTY expression if needed */
                if (jsexpr->on_empty->coerce)
                        ExecInitJsonCoercion(state, jsexpr->returning, escontext,
                                                                 jsexpr->omit_quotes, resv, resnull);
+
+               /*
+                * Add a COERCION_FINISH step to check for errors that may occur when
+                * coercing and rethrow them.
+                */
+               if (jsexpr->on_empty->coerce ||
+                       IsA(jsexpr->on_empty->expr, CoerceViaIO) ||
+                       IsA(jsexpr->on_empty->expr, CoerceToDomain))
+               {
+
+                       scratch->opcode = EEOP_JSONEXPR_COERCION_FINISH;
+                       scratch->resvalue = resv;
+                       scratch->resnull = resnull;
+                       scratch->d.jsonexpr.jsestate = jsestate;
+                       ExprEvalPushStep(state, scratch);
+               }
        }
 
        foreach(lc, jumps_to_end)

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index d8735286c4d40b30d436ad773e981048f5bc380f..4c9b2a8c17828e7e914eaec1e42eb92f164ebbc9 100644 (file)
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -4558,6 +4558,12 @@ ExecEvalJsonCoercionFinish(ExprState *state, ExprEvalStep *op)
                *op->resvalue = (Datum) 0;
                *op->resnull = true;
                jsestate->error.value = BoolGetDatum(true);
+
+               /*
+                * Reset for next use such as for catching errors when coercing a
+                * JsonBehavior expression.
+                */
+               jsestate->escontext.error_occurred = false;
        }
 }
 

diff --git a/src/test/regress/expected/sqljson_jsontable.out b/src/test/regress/expected/sqljson_jsontable.out
index 5fd43be36778394b4346a19f588b114f2bc9c050..19817b4be8cbb5348c62114abf3a128040d4af67 100644 (file)
--- a/src/test/regress/expected/sqljson_jsontable.out
+++ b/src/test/regress/expected/sqljson_jsontable.out
@@ -227,7 +227,11 @@ SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
 
 SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
     COLUMNS (js1 jsonb_test_domain PATH '$.a2' DEFAULT 'foo'::jsonb_test_domain ON EMPTY));
-ERROR:  value for domain jsonb_test_domain violates check constraint "jsonb_test_domain_check"
+ js1 
+-----
+ 
+(1 row)
+
 SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
     COLUMNS (js1 jsonb_test_domain PATH '$.a2' DEFAULT 'foo1'::jsonb_test_domain ON EMPTY));
  js1  

diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out
index 074aedb2dd1ac3393490b8a5a856da4f838a48f0..ec8caee91c7c3466f0e6d883326a639d9f92a852 100644 (file)
--- a/src/test/regress/expected/sqljson_queryfuncs.out
+++ b/src/test/regress/expected/sqljson_queryfuncs.out
@@ -1232,7 +1232,11 @@ DROP TABLE test_jsonb_mutability;
 DROP FUNCTION ret_setint;
 CREATE DOMAIN queryfuncs_test_domain AS text CHECK (value <> 'foo');
 SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING queryfuncs_test_domain DEFAULT 'foo'::queryfuncs_test_domain ON EMPTY);
-ERROR:  value for domain queryfuncs_test_domain violates check constraint "queryfuncs_test_domain_check"
+ json_value 
+------------
+ 
+(1 row)
+
 SELECT JSON_VALUE(jsonb '{"d1": "H"}', '$.a2' RETURNING queryfuncs_test_domain DEFAULT 'foo1'::queryfuncs_test_domain ON EMPTY);
  json_value 
 ------------