Re: Add new for_each macros for iterating over a List that do not require ListCell pointer

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

In response to

Responses

Browse pgsql-hackers by date

  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