From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Peter Smith <smithpb2250@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Tomas Vondra <tomas.vondra@postgresql.org>
Cc: David Rowley <dgrowleyml@gmail.com>
Subject: Re: shadow variables - pg15 edition
Date: Wed, 17 Aug 2022 23:42:27 -0400
Message-ID: <2230209.1660794147@sss.pgh.pa.us> (raw)
In-Reply-To: <Yv2KJiDhnqrZR7VE@paquier.xyz>
References: <20220817145434.GC26426@telsasoft.com>
<CAHut+PsgHzoenCkOE0hQBWhUukfikpRWqk4_r=zh5EkmstkXsA@mail.gmail.com>
<Yv2KJiDhnqrZR7VE@paquier.xyz>
Michael Paquier <michael@paquier.xyz> writes:
> A lot of the changes proposed here update the code so as the same
> variable gets used across more code paths by removing declarations,
> but we have two variables defined because both are aimed to be used in
> a different context (see AttachPartitionEnsureIndexes() in tablecmds.c
> for example).
> Wouldn't it be a saner approach in a lot of cases to rename the
> shadowed variables (aka the ones getting removed in your patches) and
> keep them local to the code paths where we use them?
Yeah. I do not think a patch of this sort has any business changing
the scopes of variables. That moves it out of "cosmetic cleanup"
and into "hm, I wonder if this introduces any bugs". Most hackers
are going to decide that they have better ways to spend their time
than doing that level of analysis for a very noncritical patch.
regards, tom lane
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: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, smithpb2250@gmail.com, pryzby@telsasoft.com, tomas.vondra@postgresql.org, dgrowleyml@gmail.com
Subject: Re: shadow variables - pg15 edition
In-Reply-To: <2230209.1660794147@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox