From: | Jelte Fennema <postgres(at)jeltef(dot)nl> |
---|---|
To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Add new for_each macros for iterating over a List that do not require ListCell pointer |
Date: | 2023-10-25 10:05:41 |
Message-ID: | CAGECzQTCfAkL0L+Tir2fidx6K1Xkqd8S+pZiGvcgUP2mYL_ZZQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, 25 Oct 2023 at 04:55, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> With foreach(), we commonly do "if (lc == NULL)" at the end of loops
> as a way of checking if we did "break" to terminate the loop early.
Afaict it's done pretty infrequently. The following crude attempt at
an estimate estimates it's only done about ~1.5% of the time a foreach
is used:
$ rg 'lc == NULL' | wc -l
13
$ rg '\bforeach\(lc,' -S | wc -l
899
> Doing the equivalent with the new macros won't be safe as the list
> element's value we broke on may be set to NULL. I think it might be a
> good idea to document the fact that this wouldn't be safe with the new
> macros, or better yet, document the correct way to determine if we
> broke out the loop early. I imagine someone will want to do some
> conversion work at some future date and it would be good if we could
> avoid introducing bugs during that process.
>
> I wonder if we should even bother setting the variable to NULL at the
> end of the loop. It feels like someone might just end up mistakenly
> checking for NULLs even if we document that it's not safe. If we left
> the variable pointing to the last list element then the user of the
> macro is more likely to notice their broken code. It'd also save a bit
> of instruction space.
Makes sense. Addressed this now by mentioning this limitation and
possible workarounds in the comments of the new macros and by not
setting the loop variable to NULL/0. I don't think there's an easy way
to add this feature to these new macros natively, it's a limitation of
not having a second variable. This seems fine to me, since these new
macros are meant as an addition to foreach() instead of a complete
replacement.
Attachment | Content-Type | Size |
---|---|---|
v3-0001-Add-macros-for-looping-through-a-list-without-nee.patch | application/octet-stream | 6.1 KB |
v3-0002-Use-new-for_each_xyz-macros-in-a-few-places.patch | application/octet-stream | 7.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Jelte Fennema | 2023-10-25 10:39:01 | Re: Add new for_each macros for iterating over a List that do not require ListCell pointer |
Previous Message | tender wang | 2023-10-25 09:58:21 | Re: BUG #18167: cannot create partitioned tables when default_tablespace is set |