Hi Jian, On Wed, Sep 16, 2026 at 11:21 AM jian he <[email protected]> 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
