On Tue, 19 Dec 2023 at 11:59, vignesh C <vignes...@gmail.com> wrote: > I noticed that this change can be done in several other places too.
My guess would be that ~90% of all existing foreach loops in the codebase can be easily rewritten (and simplified) using these new macros. So converting all of those would likely be quite a bit of work. In patch 0003 I only converted a few of them to get some coverage of the new macros and show how much simpler the usage of them is. > Should we start doing these changes too now? I think we should at least wait until this patchset is merged before we start changing other places. If there's some feedback on the macros and decide we change how they get called, then it would be a waste of time to have to change all the call sites. And even once these patches are merged to master, I think we should only do any bulk changes if/when we backport these macros to all supported PG versions. Backporting to PG12 is probably the hardest, since List its internal layout got heavily changed in PG13. Probably not too hard though, in Citus we've had similar macros work since PG11. I'm also not sure what the policy is for backporting patches that introduce new functions/macros in public headers. We probably even want to consider some automatic rewriting script (for the obvious cases) and/or timing the merge, to avoid having to do many rebases of the patch.
