From 63e6c5f4a2eeb22e0dd446a62c2b4b417d2b51f0 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 26 Jul 2024 15:59:27 +0900 Subject: [PATCH] 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 --- src/backend/executor/execExpr.c | 51 ++++++++++++++++++- src/backend/executor/execExprInterp.c | 6 +++ .../regress/expected/sqljson_jsontable.out | 6 ++- .../regress/expected/sqljson_queryfuncs.out | 6 ++- 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index ccd4863778..b10359e3d6 100644 --- 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 d8735286c4..4c9b2a8c17 100644 --- 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 5fd43be367..19817b4be8 100644 --- 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 074aedb2dd..ec8caee91c 100644 --- 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 ------------