| 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-05 15:36:18 |
| Message-ID: | CAFj8pRA+tuLvb4kZVEeLjNOK7-oqKwdgTuBu3ofNGkVU4RO2RQ@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.
>
I changed the interface like you proposed. It reduces some code, but
introduces some new issues:
1. Because the iterator has no knowledge about target type, the json object
values cannot be assigned to a list of target values.
Our composites and JSON objects have not compatible IO format. With
knowledge of target type, I can force transformation from
jsonb to composite. There is not any generic API that can help with this,
and it can be implemented as part of the iterator. And in
this case IO cast can be a problem (if we support it) because jsonb doesn't
preserve an order of fields.
2. When I know the target is jsonb I don't need to immediately transform
jbvNull to NULL.
3. If I don't do any transformation inside the iterator and returns just an
iterated value, then I have a problem with often usage of IO cast - that is
more strict.
when I iterate over numeric array and target is int, then I got cast
int4(numeric) - "3.14 --> 3"
when I iterate over jsonb array and target is int, then I got IO cast -
there is int4(jsonb) cast, but this is explicit and then cast "3.14" to int
fails.
Maybe we can enhance plpgsql cast functions to see inside jsonb and then
can better to choose cast function.
This inconsistency is simply visible:
(2026-07-05 17:31:13) postgres=# do $$ declare t int; begin t :=
'3.14'::numeric; end $$;
DO
(2026-07-05 17:31:22) postgres=# do $$ declare t int; begin t :=
'3.14'::jsonb; end $$;
ERROR: invalid input syntax for type integer: "3.14"
CONTEXT: PL/pgSQL assignment "t := '3.14'::jsonb"
PL/pgSQL function inline_code_block line 1 at assignment
(2026-07-05 17:31:34) postgres=# do $$ declare t int; begin t :=
'3.14'::jsonb::int; end $$;
DO
>
> * 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 think the error messages can be written more generically.
> * memory management may need to be rethought a bit as well.
> Certainly the comments referencing exec_eval_cleanup do not
> belong here.
>
I changed it,
Regards
Pavel
>
> regards, tom lane
>
| Attachment | Content-Type | Size |
|---|---|---|
| v20260705-0001-FOREACH-scalar-IN-ARRAY-jsonb_expr.patch | text/x-patch | 33.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sehrope Sarkuni | 2026-07-05 15:38:18 | Re: inconsistent jsonb 'null' conversions |
| Previous Message | Tom Lane | 2026-07-05 15:18:28 | Re: Fixing MSVC's inability to detect elog(ERROR) does not return |