Subject: Re: BUG #19418: SQL/JSON JSON_VALUE() does not conform to ISO/IEC 9075-2:2023(E) 6.34 <JSON value constructor>

From: Tom Lane <[email-masked]>
Date: Mon, 02 Mar 2026 01:37:29 -0500

Richard Guo <[email-masked]> writes:
> Regarding back-patching, I believe this fix is safe to back-patch to
> stable branches.  However, similar to a nearby bug fix, this will only
> apply to newly created views.  Existing views will continue to exhibit
> the old behavior until recreated.

Okay, but ...

> Additionally, this changes the
> user-facing output from NULL to [], so users may need to update any
> application code that relied on the NULL behavior.

... doesn't that point disqualify it from being back-patched?
People don't like unprompted behavioral changes in minor releases.
"This is what the standard says" is not strong enough to justify
changing behavior that was not obviously broken (like, say, crashing).

Another point is that the previous coding already failed to
be round-trippable, ie you wrote JSON_ARRAY() but what comes
out in view decompilation is JSON_ARRAYAGG().  This makes that
situation considerably worse.  We should endeavor to not expose
implementation details like that.  (To be clear, I don't object
if EXPLAIN shows that sort of thing.  But it shouldn't creep
into view dumps.  We've regretted doing that in the past.)

			regards, tom lane

---

From: Richard Guo <[email-masked]>
Date: Tue, 3 Mar 2026 10:03:49 +0900

> changing behavior that was not obviously broken (like, say, crashing).

Fair point.  Changing user-facing output is not something we want to
surprise users with in a minor release.  So this will be a master-only
fix.

> into view dumps.  We've regretted doing that in the past.)

That is a good point I hadn't considered.  So I think the ideal fix is
to have the parser preserve the user's original JSON_ARRAY(query)
syntax as much as possible, and then defer the JSON_ARRAYAGG rewrite
trick to the planner, perhaps during expression preprocessing.

- Richard

---

From: Richard Guo <[email-masked]>
Date: Tue, 3 Mar 2026 23:32:12 +0900

> trick to the planner, perhaps during expression preprocessing.

I tried hacking on this idea to see how it would look in practice, and
here is what I got.

- Richard

---

From: Richard Guo <[email-masked]>
Date: Wed, 4 Mar 2026 15:35:56 +0900

> I tried hacking on this idea to see how it would look in practice, and

Here is an updated version of the patch.  The main change is that it
now uses DirectFunctionCall1 to build the empty JSON array constant,
which is more efficient and consistent with other call sites.

- Richard

---

From: Richard Guo <[email-masked]>
Date: Thu, 16 Apr 2026 15:04:49 +0900

> I tried hacking on this idea to see how it would look in practice, and

After a second look at this approach, I don't like it very much.  It
manually constructed the new querytree, including Aggref,
RangeTblEntry, and JsonConstructorExpr nodes, during planning,
bypassing parse analysis entirely.  This is essentially repeating the
parser's work by hand in the planner, which is fragile and prone to
failing to handle all cases correctly.

Maybe a simpler way is to keep the JSON_ARRAYAGG rewrite trick in the
parser, as the current master does, but wrap the result in a COALESCE
to handle the empty-set case.  We can preserve a copy of the user's
original subquery in a new field of JsonConstructorExpr, and then
ruleutils.c can use this field to deparse the original
JSON_ARRAY(SELECT ...) syntax for view definitions.  You may think
this would introduce extra transform work, but it wouldn't: the
current master already transforms the original subquery to validate
the single-column constraint, then throws the result away.  We simply
keep it instead.

I tried this idea and ended up with the attached.

- Richard

---

From: Amit Langote <[email-masked]>
Date: Mon, 20 Apr 2026 18:05:33 +0900

Hi Richard,

> I tried this idea and ended up with the attached.

Agreed that v4 is the better direction.

A couple of minor nits.

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) */

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 */

-- 
Thanks, Amit Langote

---

From: Richard Guo <[email-masked]>
Date: Tue, 21 Apr 2026 09:57:34 +0900

> Agreed that v4 is the better direction.

Thanks for review!

> Node       *orig_query;     /* for deparse only; not walked (func is) */

Sounds good.

>                                  * 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) */

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

- Richard

---

From: Richard Guo <[email-masked]>
Date: Tue, 21 Apr 2026 10:12:55 +0900

> Thanks for review!

Another question I'd like to raise: is it OK to commit this patch to
master given that feature freeze has passed?  I think the answer is
yes, because this is arguably a bug fix rather than a new feature.
However, it does change user-visible behavior, and existing app code
that relies on the NULL behavior would break.  So if we commit it, we
need to add in the release notes about this incompatibility.

Thoughts?

- Richard

---

From: Tom Lane <[email-masked]>
Date: Mon, 20 Apr 2026 22:30:32 -0400

> need to add in the release notes about this incompatibility.

Well, if we definitely intend to commit a compatibility-breaking
change, I think it's better to commit it sooner not later.  If we
wait till v20, all we accomplish is to give users another year to
write code that depends on the old behavior.

However, usually at this stage of the cycle the answer to such
questions is "let the RMT decide".  Take the question to them
(cc'd).

			regards, tom lane

---

From: Richard Guo <[email-masked]>
Date: Tue, 21 Apr 2026 12:20:02 +0900

> questions is "let the RMT decide".  Take the question to them

Thanks Tom for the suggestion.

Hi RMT,

I'd like to commit a fix for JSON_ARRAY(subquery) behavior that
involves a user-visible incompatibility, and would appreciate your
go/no-go since we're past feature freeze.

Summary:

- JSON_ARRAY(SELECT ...) currently returns NULL over an empty result
set, but the SQL/JSON standard requires it to return '[]'.  Fixing
this changes user-visible output.

- The same patch also fixes a deparsing issue: views defined with
JSON_ARRAY(SELECT ...) are dumped back as the internal JSON_ARRAYAGG
rewrite instead of the original syntax.

- Richard

---

From: Amit Langote <[email-masked]>
Date: Tue, 21 Apr 2026 12:57:12 +0900

>                                * 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: Richard Guo <[email-masked]>
Date: Tue, 21 Apr 2026 14:45:01 +0900

> like it is done for the nearby JsonValueExpr. Like this:

This looks even better.  I will take this approach.  Thanks!

- Richard

---

From: Richard Guo <[email-masked]>
Date: Thu, 23 Apr 2026 09:55:10 +0900

> Thanks Tom for the suggestion.

Not sure the RMT mailing list works or not, maybe I'd better CC RMT
members.

- Richard

---

From: Michael Paquier <[email-masked]>
Date: Thu, 23 Apr 2026 15:51:11 +0900

> Not sure the RMT mailing list works or not, maybe I'd better CC RMT

The mailing list you have attached is used every year by the RMT
members for their internal discussions, attaching it to a
pgsql-hackers or pgsql-bugs thread is not the usual practice, so I
would suggest to avoid that in the future.  Adding the RMT members
directly in CC is enough if input is required for an open item.
--
Michael

---

From: Richard Guo <[email-masked]>
Date: Fri, 24 Apr 2026 21:04:03 +0900

> directly in CC is enough if input is required for an open item.

Thanks for the information.  Will avoid that in the future.  Since the
RMT members are already in CC, I'll wait for their feedback.

- Richard

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 24 Apr 2026 14:58:25 -0500

>> rewrite instead of the original syntax.

I am fine with committing this for v19, assuming there is consensus on the
patch content.

Heikki/Melanie: Any objections?

-- 
nathan

---

From: Melanie Plageman <[email-masked]>
Date: Thu, 30 Apr 2026 18:04:54 -0400

> Heikki/Melanie: Any objections?

Nope. I'm +1 on committing this now. 19 is unreleased -- I see no
reason to wait a year.

- Melanie

---

From: Richard Guo <[email-masked]>
Date: Fri, 1 May 2026 09:26:32 +0900

> Nope. I'm +1 on committing this now. 19 is unreleased -- I see no

Thank you, Nathan and Melanie.  I now have two of the three RMT
approvals, so I believe I'm good to go.  Will commit this shortly.

- Richard