Re: Add new for_each macros for iterating over a List that do not require ListCell pointer - Mailing list pgsql-hackers

From Jelte Fennema
Subject Re: Add new for_each macros for iterating over a List that do not require ListCell pointer
Date
Msg-id CAGECzQTCfAkL0L+Tir2fidx6K1Xkqd8S+pZiGvcgUP2mYL_ZZQ@mail.gmail.com
Whole thread Raw
In response to Re: Add new for_each macros for iterating over a List that do not require ListCell pointer  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: Add new for_each macros for iterating over a List that do not require ListCell pointer
List pgsql-hackers
On Wed, 25 Oct 2023 at 04:55, David Rowley <dgrowleyml@gmail.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

pgsql-hackers by date:

Previous
From: "Drouvot, Bertrand"
Date:
Subject: Re: Synchronizing slots from primary to standby
Next
From: Jelte Fennema
Date:
Subject: Re: Add new for_each macros for iterating over a List that do not require ListCell pointer