| From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
|---|---|
| To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Vik Fearing <vik(at)postgresfriends(dot)org>, lukas(dot)eder(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org, Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Subject: | Re: BUG #19418: SQL/JSON JSON_VALUE() does not conform to ISO/IEC 9075-2:2023(E) 6.34 <JSON value constructor> |
| Date: | 2026-04-21 03:57:12 |
| Message-ID: | CA+HiwqHq1OQHFgnpvfJjzYc1qYf1U99VeBnft9HxYkTQFqdcVg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Tue, Apr 21, 2026 at 9:57 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> On Mon, Apr 20, 2026 at 6:05 PM Amit Langote <amitlangote09(at)gmail(dot)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 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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2026-04-21 05:45:01 | Re: BUG #19418: SQL/JSON JSON_VALUE() does not conform to ISO/IEC 9075-2:2023(E) 6.34 <JSON value constructor> |
| Previous Message | Richard Guo | 2026-04-21 03:20:02 | Re: BUG #19418: SQL/JSON JSON_VALUE() does not conform to ISO/IEC 9075-2:2023(E) 6.34 <JSON value constructor> |