From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <amitlangote09@gmail.com>
cc: Tender Wang <tndrwang@gmail.com>, Michael Paquier <michael@paquier.xyz>,
        exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18657: Using JSON_OBJECTAGG with volatile function leads to
 segfault
In-reply-to: 
 <CA+HiwqHpU4VARMpEih6O5xd+psvODEzutf8Rkx7m+sU8OPV-bA@mail.gmail.com>
References: <18657-1b90ccce2b16bdb8@postgresql.org>
 <Zw8Gk3xtrSmQh9mD@paquier.xyz>
 <CA+HiwqFpzUYXOqcJ4LdAoLjAxDJ-e4-mVe=4z9-jC5vhV0ZwEg@mail.gmail.com>
 <CAHewXNmSvzsSqqf6fG7OEkmB2aUuCa7SM4pfVrs9vBeCWZu7yQ@mail.gmail.com>
 <CA+HiwqGDZMwi6O9w64vtd0fFTeow4GnKfWraSaJFxC-y7mCm+w@mail.gmail.com>
 <CAHewXN=hHQepA9-JHaW9z-orPE-qA2UEFNvAvLpYZr6e1PGq2Q@mail.gmail.com>
 <CA+HiwqF0iihKd28ycs5BJcYprAWUBDqHv03vqLMteBid2YDCyA@mail.gmail.com>
 <CAHewXNnc35-OX=wA+MHkj+asYa0oezLwV5by=D00ga+EawN7Fg@mail.gmail.com>
 <CA+HiwqGJUv2Y=qCRO1kL-6cfZ79zAgag_zJ_=U4q-RaNUv9r2g@mail.gmail.com>
 <71060.1729088799@sss.pgh.pa.us>
 <CA+HiwqGZV2YWvso8B4bpPAEJDuEhDos3oUTxDTfr9kpX=nOAgA@mail.gmail.com>
 <508107.1729126612@sss.pgh.pa.us> <569213.1729138329@sss.pgh.pa.us>
 <CA+HiwqHpU4VARMpEih6O5xd+psvODEzutf8Rkx7m+sU8OPV-bA@mail.gmail.com>
Comments: In-reply-to Amit Langote <amitlangote09@gmail.com>
	message dated "Thu, 17 Oct 2024 17:15:49 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <711199.1729197017.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Thu, 17 Oct 2024 16:30:17 -0400
Message-ID: <711200.1729197017@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/711200.1729197017%40sss.pgh.pa.us>
Precedence: bulk

Amit Langote <amitlangote09@gmail.com> writes:
> On Thu, Oct 17, 2024 at 1:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In the short term, I suspect the only workable fix is to undo the
>> optimization of having ExecInitExprRec not recurse into both raw_expr
>> and formatted_expr.

> This or actually I'm tempted to simply revert the whole thing
> (b6e1157e7d3) as an ill-considered refactoring, because I am not able
> to convince myself that calling ExecAggPlainTransByVal() twice, via
> both raw_expr and formatted_expr, is always safe.

Not following the concern here?  As far as nodeAgg is concerned,
they'd be two independent aggregates.

In any case, whatever we do in master, you can't "simply revert"
b6e1157e7 in released branches.  It changed the way JsonValueExpr is
represented in stored rules, and you don't get to undo that midstream.
(The commit should have included a catversion bump, in fact.)

I do think that reverting the ExecInitExprRec and
eval_const_expressions_mutator changes would be good, but we
can't undo the parser changes, at least not in v16/v17.

			regards, tom lane