public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nathan Bossart <[email protected]>
To: Jelte Fennema <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Add new for_each macros for iterating over a List that do not require ListCell pointer
Date: Tue, 24 Oct 2023 11:47:15 -0500
Message-ID: <20231024164715.GB871220@nathanxps13> (raw)
In-Reply-To: <CAGECzQSwXKnxGwW1_Q5JE+8Ja20kyAbhBHO04vVrQsLcDciwXA@mail.gmail.com>
References: <CAGECzQSwXKnxGwW1_Q5JE+8Ja20kyAbhBHO04vVrQsLcDciwXA@mail.gmail.com>

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

view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Add new for_each macros for iterating over a List that do not require ListCell pointer
  In-Reply-To: <20231024164715.GB871220@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox