From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
cc: bernddorn@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17227: segmentation fault with jsonb_to_recordset
In-reply-to: <709405.1634165890@sss.pgh.pa.us>
References: <202110131359.a6m35hpudkg7@alvherre.pgsql>
 <649678.1634135335@sss.pgh.pa.us> <675396.1634157283@sss.pgh.pa.us>
 <676464.1634158492@sss.pgh.pa.us> <709405.1634165890@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Wed, 13 Oct 2021 18:58:10 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <794909.1634225373.1@sss.pgh.pa.us>
Date: Thu, 14 Oct 2021 11:29:33 -0400
Message-ID: <794910.1634225373@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/794910.1634225373%40sss.pgh.pa.us>
Precedence: bulk

I wrote:
> The whole thing makes me itch a little bit, because this logic is
> assuming that eval_const_expressions is idempotent, something
> we can't test very well and (AFAIR) are not assuming anywhere else.
> If somebody made some sub-case not idempotent, we'd not notice until
> that case got used in a lateral function RTE.

It occurred to me that we could test for problems with double
execution of eval_const_expressions by making planner.c deliberately
call it twice in all cases.  A quick hack later, I can say that
"make check-world" doesn't reveal any such problem.  While that's
hardly conclusive, it's pretty strong evidence, since the code
coverage report shows eval_const_expressions as well covered (and
the lines not reached are mostly returning simplified Const nodes,
so hardly likely to be a problem here).

We could make a persistent check for that by enclosing the extra
calls in something like "#ifdef DOUBLE_EVAL_CONST_EXPRESSIONS"
and setting up some buildfarm member to define that.  But since
(AFAIR) we've never actually had such a bug, it seems unlikely
to be worth the trouble.

I'll go ahead with the patch as presented.

			regards, tom lane