Обсуждение: BUG #19046: Incorrect result when using json_array() with column reference in subquery combined with RIGHT JOIN

The following bug has been logged on the website:

Bug reference:      19046
Logged by:          Runyuan He
Email address:      runyuan@berkeley.edu
PostgreSQL version: 18rc1
Operating system:   Linux (x86)
Description:

Bug Description:
When using json_array() function with a column reference from table t inside
a subquery, and then performing a RIGHT JOIN with condition FALSE, the
result incorrectly returns [3, 2] instead of the expected NULL value.

Reproducible Example:
CREATE TABLE t(c INT);
INSERT INTO t VALUES (1);

SELECT sub.c FROM
(SELECT json_array(3, 2, t.c) AS c FROM t) AS sub
RIGHT JOIN t ON FALSE;
-- PostgreSQL 16.x: Returns NULL (CORRECT)
-- PostgreSQL 17.6, 17.rc1: Returns [3, 2] (INCORRECT)
-- PostgreSQL 18rc1: Returns [3, 2] (INCORRECT)

SELECT sub.c FROM
(SELECT json_array(3, 2, t.c) AS c FROM t) AS sub;
-- Returns [3, 2, 1] (CORRECT)

SELECT sub.c FROM
(SELECT json_array(3, 2, 1) AS c FROM t) AS sub
RIGHT JOIN t ON FALSE;
-- Returns Null (CORRECT)

On Wed, Sep 10, 2025 at 9:31 PM Tender Wang <tndrwang@gmail.com> wrote:
>> PG Bug reporting form <noreply@postgresql.org> 于2025年9月10日周三 18:22写道：
>>> SELECT sub.c FROM
>>> (SELECT json_array(3, 2, t.c) AS c FROM t) AS sub
>>> RIGHT JOIN t ON FALSE;

> diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
> index 6f0b338d2cd..5ef364b7f7c 100644
> --- a/src/backend/optimizer/util/clauses.c
> +++ b/src/backend/optimizer/util/clauses.c
> @@ -1115,6 +1115,8 @@ contain_nonstrict_functions_walker(Node *node, void *context)
>                 return true;
>         if (IsA(node, BooleanTest))
>                 return true;
> +       if (IsA(node, JsonConstructorExpr))
> +               return true;
>
> I added the above codes, then the query returned the correct result.
> I didn't dig more the details. Any thought?

Yeah, JsonConstructorExpr should not be treated as a non-strict
construct.  This fix looks correct to me.

I'm wondering if this is the only case we've overlooked.  How about
other Json-related expressions?

- Richard

On Thu, Sep 11, 2025 at 10:40 AM Tender Wang <tndrwang@gmail.com> wrote:
> Richard Guo <guofenglinux@gmail.com> 于2025年9月10日周三 21:29写道：
>> I'm wondering if this is the only case we've overlooked.  How about
>> other Json-related expressions?

> Yeah, I have the same question. I tried my fix on json_object/json_arrayagg/json_objectagg.
> These returned the same results.

json_object is also represented as a JsonConstructorExpr.

json_arrayagg and json_objectagg are aggregates, so the subquery won't
be pulled up in the first place.

I tested JsonExpr, which is the representation of json_value,
json_query and json_exists.  It seems that they could not produce
non-NULL output with a NULL input.  So we are good on that front.

- Richard

Richard Guo <guofenglinux@gmail.com> writes:
> I tested JsonExpr, which is the representation of json_value,
> json_query and json_exists.  It seems that they could not produce
> non-NULL output with a NULL input.  So we are good on that front.

Seems like we ought to actually look at the relevant code, not
try to test our way to an understanding of it.

            regards, tom lane

On Fri, Sep 12, 2025 at 9:57 AM Tender Wang <tndrwang@gmail.com> wrote:
> Tom Lane <tgl@sss.pgh.pa.us> 于2025年9月11日周四 11:16写道：
>> Seems like we ought to actually look at the relevant code, not
>> try to test our way to an understanding of it.

> /* JUMP to return NULL if formatted_expr evaluates to NULL */
> jumps_return_null = lappend_int(jumps_return_null, state->steps_len);
> scratch->opcode = EEOP_JUMP_IF_NULL;
> scratch->resnull = &jsestate->formatted_expr.isnull;
> scratch->d.jump.jumpdone = -1; /* set below */
> ExprEvalPushStep(state, scratch);

Yeah, this suggests that JsonExpr behaves as a strict construct.

There may be other functions that should be treated as non-strict but
currently aren't.  For now, I suggest we address JsonConstructorExpr
first.  If we encounter more such cases later, they should be
straightforward to fix as well.

- Richard

On Tue, Sep 16, 2025 at 10:05 AM Richard Guo <guofenglinux@gmail.com> wrote:
> There may be other functions that should be treated as non-strict but
> currently aren't.  For now, I suggest we address JsonConstructorExpr
> first.  If we encounter more such cases later, they should be
> straightforward to fix as well.

I don't like the test cases in the proposed patch.  First, the test
output is excessively large, resulting in a patch with over 30,000
insertions.  I don't think it's a good idea to output the entire tenk1
table.  Second, I don't think it's useful to test json_arrayagg or
json_objectagg.  As I mentioned earlier, these are aggregates, so the
subquery wouldn't be pulled up in the first place.

I've rewritten the test cases, added a commit message, and pushed the
patch.  It has also been back-patched to v16.

- Richard