public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Langote <[email protected]>
To: Richard Guo <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Vik Fearing <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Álvaro Herrera <[email protected]>
Subject: Re: BUG #19418: SQL/JSON JSON_VALUE() does not conform to ISO/IEC 9075-2:2023(E) 6.34 <JSON value constructor>
Date: Tue, 21 Apr 2026 12:57:12 +0900
Message-ID: <CA+HiwqHq1OQHFgnpvfJjzYc1qYf1U99VeBnft9HxYkTQFqdcVg@mail.gmail.com> (raw)
In-Reply-To: <CAMbWs49tDE_niKLr4mzsa2BkX7fkWrorBzWheKTBziZ7z9-AuQ@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<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>
	<[email protected]>
	<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>

On Tue, Apr 21, 2026 at 9:57 AM Richard Guo <[email protected]> wrote:
> On Mon, Apr 20, 2026 at 6:05 PM Amit Langote <[email protected]> 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 types */
>
> 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;

-- 
Thanks, Amit Langote






reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: BUG #19418: SQL/JSON JSON_VALUE() does not conform to ISO/IEC 9075-2:2023(E) 6.34 <JSON value constructor>
  In-Reply-To: <CA+HiwqHq1OQHFgnpvfJjzYc1qYf1U99VeBnft9HxYkTQFqdcVg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox