From: Tom Lane <tgl@sss.pgh.pa.us>
To: Pavel Stehule <pavel.stehule@gmail.com>
cc: Peter Eisentraut <peter@eisentraut.org>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: POC: PLpgSQL FOREACH IN JSON ARRAY
In-reply-to: 
 <CAFj8pRDT5LvSMNxorBROZk9Ma41Z_7_0w6Ynq3rn9X6dTocbaQ@mail.gmail.com>
References: 
 <CAFj8pRD9Jjv+m=6DP6vWWn5NnZeTPH9eoPFCnA1JE5hRKRDMxQ@mail.gmail.com>
 <d626bc1a-e6c7-4bb5-9760-b4b119ca6196@eisentraut.org>
 <4166.1773289811@sss.pgh.pa.us>
 <CAFj8pRDom7DWYnOWUtSArW2D=WJ+JKpU=oNPgO7C=KUU_pEb-w@mail.gmail.com>
 <CAFj8pRDHdO9bLM4=8=p0MF8rNpSKdw-F_=zG=YxWui8S4vn5RQ@mail.gmail.com>
 <CAFj8pRAHrHmpPmmFfm6B5ig3GzWfVVSh_3CofcQYM8utrSQoFQ@mail.gmail.com>
 <CAFj8pRAS5_1ObWfM-eNP6ei60bdLw884BpAa=Z7bAmw9WiSxFQ@mail.gmail.com>
 <CAFj8pRDT5LvSMNxorBROZk9Ma41Z_7_0w6Ynq3rn9X6dTocbaQ@mail.gmail.com>
Comments: In-reply-to Pavel Stehule <pavel.stehule@gmail.com>
	message dated "Tue, 16 Jun 2026 09:52:59 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3648847.1782942004.1@sss.pgh.pa.us>
Date: Wed, 01 Jul 2026 17:40:04 -0400
Message-ID: <3648848.1782942004@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3648848.1782942004%40sss.pgh.pa.us>
Precedence: bulk

I took a very quick look through the v20260616 patch.

I think you need to work harder on the separation of knowledge.
In particular I don't like that the type-specific setup routines
(create_foreach_a_array_iterator etc) contain so many assumptions that
they are being called from a PLpgsql FOREACH statement and nothing
else.  IMO the point of putting this into the SubscriptRoutines
infrastructure is to make a general-purpose facility that could be
used by different things.  So:

* it's not clear to me that these routines have any business knowing
about the "target type"; they certainly shouldn't contain comments
alluding to PLpgSQL's conversion abilities.  I think probably you
just want them to pass back the data type they are producing and
let PLpgSQL decide whether it wants to convert or not.

* we need to think about how the error messages could be phrased more
generically, or else not have these functions throw those errors
themselves but instead pass back an error code that the caller could
use to select an error message.  I suspect this will end up with
visible changes in the error messages produced by existing cases,
and that's okay IMO.

* memory management may need to be rethought a bit as well.
Certainly the comments referencing exec_eval_cleanup do not
belong here.

			regards, tom lane