Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY

Hi,

On Mon, Sep 28, 2026 at 1:03 PM Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com> wrote:
> сб, 26 сент. 2026 г. в 21:29, jian he <jian(dot)universality(at)gmail(dot)com>:
>> On Fri, Sep 4, 2026 at 6:38 PM Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com> wrote:
>> >
>> > Attached is v2 of the patch.
>> >
>> > --
>> > Regards,
>> > Rachitskiy Andrey
>>
>>
>> + EEO_CASE(EEOP_JSONEXPR_RESET)
>> + {
>> + ExecEvalJsonExprReset(state, op);
>> +
>> + EEO_NEXT();
>> + }
>> +
>>
>> --- a/src/include/executor/execExpr.h
>> +++ b/src/include/executor/execExpr.h
>> @@ -265,6 +265,7 @@ typedef enum ExprEvalOp
>> EEOP_XMLEXPR,
>> EEOP_JSON_CONSTRUCTOR,
>> EEOP_IS_JSON,
>> + EEOP_JSONEXPR_RESET,
>> EEOP_JSONEXPR_PATH,
>> EEOP_JSONEXPR_COERCION,
>> EEOP_JSONEXPR_COERCION_FINISH,
>>
>> This seems unnecessary.
>> In EEOP_JSONEXPR_PATH, we can
>> if document or jsonpath is NULL, we can just go to jump_end (return
>> NULL) or jump_eval_coercion (NULL need coerce to constrainted domain),
>> no need to worry about ON ERROR, ON EMPTY.
>>
>> What do you think of the attachment?
>>
>>
>>
>> --
>> jian
>> https://www.enterprisedb.com/
>
>
> Hi, Jian!
>
> Thanks for posting this alternative.
>
> I considered the same shape earlier: keep the empty/error reset in EEOP_JSONEXPR_PATH and stop skipping that step on SQL NULL. Your version is simpler than adding EEOP_JSONEXPR_RESET. No new opcode, and no JIT or back-branch ABI churn.
>
> The commit message still described a RESET opcode that the diff does not add. I adjusted the subject and body to match what the patch actually does (retarget JUMP_IF_NULL at PATH, drop the CONST NULL pad, handle NULL inside ExecEvalJsonExprPath).
>
> Either approach fixes the reported cases. I am fine with whichever version a committer prefers.

Thanks, Jian and Andrey. I agree that v3’s approach of having
ExecEvalJsonExprPath() itself handle a NULL context item or path is
better than adding a new code to do that job, which as you have
pointed out is not painless to backpatch.

Here is v4 with some comment and commit message updates, plus one more
regression case for JSON_EXISTS’s default FALSE ON ERROR. I plan to
commit and back-patch it to 17 barring objections.

--
Thanks, Amit Langote