Date: Tue, 24 Oct 2023 11:47:15 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Jelte Fennema <postgres@jeltef.nl>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: Add new for_each macros for iterating over a List that do not
 require ListCell pointer
Message-ID: <20231024164715.GB871220@nathanxps13>
References: <CAGECzQSwXKnxGwW1_Q5JE+8Ja20kyAbhBHO04vVrQsLcDciwXA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAGECzQSwXKnxGwW1_Q5JE+8Ja20kyAbhBHO04vVrQsLcDciwXA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/20231024164715.GB871220%40nathanxps13>
Precedence: bulk

On Tue, Oct 24, 2023 at 06:03:48PM +0200, Jelte Fennema wrote:
> Many usages of the foreach macro in the Postgres codebase only use the
> ListCell variable to then get its value. This adds macros that
> simplify iteration code for that very common use case. Instead of
> passing a ListCell you can pass a variable of the type of its
> contents. This IMHO improves readability of the code by reducing the
> total amount of code while also essentially forcing the use of useful
> variable names.
> 
> While this might seem like a small quality of life improvement, in
> practice it turns out to be very nice to use. At Microsoft we have
> been using macros very similar to these ones in the Citus codebase for
> a long time now and we pretty much never use plain foreach anymore for
> new code.

This seems reasonable to me.

> Finally, I guess there needs to be some bikeshedding on the naming. In
> the Citus codebase we call them foreach_xyz instead of the
> for_each_xyz naming pattern that is used in this patchset. I'm not
> sure what the current stance is on if foreach should be written with
> or without an underscore between for and each. Currently pg_list.h
> uses both.

I don't have a strong opinion on the matter, but if I had to choose, I
guess I'd pick foreach_*() because these macros are most closely related to
foreach().

BTW after applying your patches, initdb began failing with the following
for me:

	TRAP: failed Assert("n >= 0 && n < list->length"), File: "list.c", Line: 770, PID: 902807

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com