MIME-Version: 1.0
References: <19418-591ba1f29862ef5b@postgresql.org> <2abdb464-27f5-4759-bb0b-f09ab5b5ceab@postgresfriends.org>
 <CAMbWs4_4Zc7O4pCU_nJU_8=Y2bOS3sEXJR=WH39Kc__UuaCW3w@mail.gmail.com>
 <CAMbWs4_f-4BqvfKTt0YbkOky+Gf0Mpi2kCT3CAE8brZFnK275Q@mail.gmail.com>
 <CAMbWs4-y4CEy66yA-Hh+6Snqbok3XTMFj5h87gUk_15Hhjd3qw@mail.gmail.com>
 <501040.1772433449@sss.pgh.pa.us> <CAMbWs49P2z40f5yb_97ErwtC9ocdR5QQN0gWUZtjgot9+iGAcg@mail.gmail.com>
 <CAMbWs4-NuYP+0Zq9=Ufwy5GcM0XQnEqWjJFucaRGO4EQgiCcow@mail.gmail.com>
 <CAMbWs4-ra9mkBhUs2kmB5KxWuo80oAA33gZ=8JmRxmG3qGNxUg@mail.gmail.com>
 <CA+HiwqEtiTNV2v2P0HGa0B1TMNfmPweKXMZEOYPoQuFf3ZhsXQ@mail.gmail.com> <CAMbWs49tDE_niKLr4mzsa2BkX7fkWrorBzWheKTBziZ7z9-AuQ@mail.gmail.com>
In-Reply-To: <CAMbWs49tDE_niKLr4mzsa2BkX7fkWrorBzWheKTBziZ7z9-AuQ@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Tue, 21 Apr 2026 12:57:12 +0900
Message-ID: <CA+HiwqHq1OQHFgnpvfJjzYc1qYf1U99VeBnft9HxYkTQFqdcVg@mail.gmail.com>
Subject: Re: BUG #19418: SQL/JSON JSON_VALUE() does not conform to ISO/IEC
 9075-2:2023(E) 6.34 <JSON value constructor>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Vik Fearing <vik@postgresfriends.org>, lukas.eder@gmail.com, 
	pgsql-bugs@lists.postgresql.org, =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CA%2BHiwqHq1OQHFgnpvfJjzYc1qYf1U99VeBnft9HxYkTQFqdcVg%40mail.gmail.com>
Precedence: bulk

On Tue, Apr 21, 2026 at 9:57=E2=80=AFAM Richard Guo <guofenglinux@gmail.com=
> wrote:
> On Mon, Apr 20, 2026 at 6:05=E2=80=AFPM Amit Langote <amitlangote09@gmail=
.com> wrote:
> > Agreed that v4 is the better direction.
>
> Thanks for review!
>
> > The comment on orig_query could say "not walked" a bit more helpfully, =
e.g.
> >
> > Node       *orig_query;     /* for deparse only; not walked (func is) *=
/
>
> Sounds good.
>
> > I also noticed that the comment for 'func' is incomplete as it is and
> > this change warrants an update. Maybe a bit long, but how about:
> >
> >     Expr       *func;           /* expression producing the result:
> >                                  * Aggref/WindowFunc for *AGG,
> >                                  * CoalesceExpr for ARRAY_QUERY,
> >                                  * json[b]_xxx() call for remaining typ=
es */
>
> It seems that func is NULL for "remaining types".  How about we go
> with:
>
>   Expr       *func;           /* executable expression:
>                                * Aggref/WindowFunc for *AGG,
>                                * CoalesceExpr for ARRAY_QUERY,
>                                * NULL for other types (executor calls
>                                * underlying json[b]_xxx() functions) */

Right.

> (maybe we should place the multi-line comment above the field.)

Makes sense. Perhaps we should also move the description of individual
fields, where needed, into the comment above the struct definition
like it is done for the nearby JsonValueExpr. Like this:

/*
 * JsonConstructorExpr -
 *      wrapper over FuncExpr/Aggref/WindowFunc/CoalesceExpr for SQL/JSON
 *      constructors
 *
 * func is the executable expression:
 *  - Aggref/WindowFunc for JSON_OBJECTAGG/JSON_ARRAYAGG,
 *  - CoalesceExpr for JSON_ARRAY(query),
 *  - NULL for other types (the executor calls the underlying json[b]_xxx()
 *    function directly).
 *
 * orig_query holds the user's original subquery for JSON_ARRAY(query),
 * used only by ruleutils.c for deparsing; it is not walked because func
 * is authoritative for all other purposes.
 */
typedef struct JsonConstructorExpr
{
    Expr        xpr;
    JsonConstructorType type;   /* constructor type */
    List       *args;
    Expr       *func;           /* executable expression or NULL */
    Node       *orig_query;     /* original subquery for deparsing */
    Expr       *coercion;       /* coercion to RETURNING type */
    JsonReturning *returning;   /* RETURNING clause */
    bool        absent_on_null; /* ABSENT ON NULL? */
    bool        unique;         /* WITH UNIQUE KEYS? (JSON_OBJECT[AGG] only=
) */
    ParseLoc    location;
} JsonConstructorExpr;

--=20
Thanks, Amit Langote