| From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: POC: PLpgSQL FOREACH IN JSON ARRAY |
| Date: | 2026-07-07 20:03:47 |
| Message-ID: | CAFj8pRC8uULe7Zk=j4DSUf0Xu2b=6_mEn=4PfKOYmP7rvES=1Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi
st 1. 7. 2026 v 23:40 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
> 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.
>
The iterator should know the target type for more reasons:
1. Only inside the iterator's implementation we can check things related to
slicing. PLpgSQL engine cannot have knowledge about slicing on some more
generic level.
2. Iterators can be implemented for types that have their own
implementation of composite values (jsonb objects). Iterators with
knowledge of the target type can do the correct transformation. Now, there
are not any generic casts between jsonb and composite types.
3. When iterator knows target type, then it can reduce some unwanted
transformations like JSONB->numeric->JSONB or can returns PostgreSQL scalar
types like numeric and can change cast from JSON->INT to NUMERIC->INT
(explicit casts has same result for numeric input, but implicit casts can
produce different results).
I dislike the fact, so target types (or expected types) are not enforced.
This makes the API dirty. But if I want to enforce cast to expected type
inside iterator implementation, then I need to move related code from
PL/pgSQL to core - but implemented casts rules are specific for PL/pgSQL
only.
Attached patch allows to not specify expected_type (use InvalidOid). In
this case the behaviour is exactly like you proposed, so maybe it can be a
way.
>
> * 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.
>
I changed messages to be more generic
>
> * memory management may need to be rethought a bit as well.
> Certainly the comments referencing exec_eval_cleanup do not
> belong here.
>
changed
Best regards
Pavel
>
> regards, tom lane
>
| Attachment | Content-Type | Size |
|---|---|---|
| v20260707-0001-FOREACH-scalar-IN-ARRAY-jsonb_expr.patch | text/x-patch | 37.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrew Dunstan | 2026-07-07 20:30:55 | VS 2026 support on old branches |
| Previous Message | Miłosz Bieniek | 2026-07-07 20:00:26 | Re: Fix GROUP BY ALL handling of ORDER BY operator semantics |