Re: SQL/JSON DEFAULT ON ERROR/ON EMPTY evaluation fail should rethrow error unconditionally

Hi Jian,

On Wed, Sep 16, 2026 at 11:21 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> Hi.
>
> Please see function ExecInitJsonExpr below code
> ``````
> /*
> * 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, false,
> 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);
> }
> ``````
> The above code relates to SQL/JSON ON ERROR, apply the same logic to ON EMPTY.
>
> 1.
> The whole expression (jsexpr->on_error->expr) could be compiled under
> the ErrorSaveContext, so any part of it
> may report an error softly; without a COERCION_FINISH step afterwards
> unconditionally, that
> error is never handled.
> That is not OK if we later want to raise the error, which is the case here.
>
> 2.
> By the time the coercion step from ExecInitJsonCoercion runs, a soft error
> may already have occurred during expression evaluation (DEFAULT ON
> ERROR, DEFAULT ON EMPTY).
> ExecEvalJsonCoercion should check for that first and, if so,
> set resnull and resvalue and return earlier.
>
> I found this issue while working on
> https://commitfest.postgresql.org/patch/5941.
>
> I don't think we need to add any extra comments.
> /*
> * Steps to evaluate the ON ERROR expression; handle errors softly to
> * rethrow them in COERCION_FINISH step that will be added later.
> */
> This comment is OK even if we unconditionally add a COERCION_FINISH step.
>
> We could executing EEOP_JSONEXPR_COERCION_FINISH only when escontext
> is non-NULL.
> But, adding EEOP_JSONEXPR_COERCION_FINISH unconditionally feels more
> intuitive and the cost seems very little.

I haven't looked at the patch, but could you specify whether your
patch fixes a live bug or is an improvement you found while working on
the linked thread? I don't understand which is the case because your
report doesn't have an example.

--
Thanks, Amit Langote