Thread: Re: pgsql: pg_upgrade: simplify code layout in a few places
On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > pg_upgrade: simplify code layout in a few places > > Backpatch-through: 9.4 (9.3 didn't need improving) Hmm. We don't normally do things like this, because it breaks translatability. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Jan 5, 2018 at 02:20:59PM -0500, Robert Haas wrote: > On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > > pg_upgrade: simplify code layout in a few places > > > > Backpatch-through: 9.4 (9.3 didn't need improving) > > Hmm. We don't normally do things like this, because it breaks translatability. Oh, you mean the 'if()' statement? If so, I will revert that and add a comment so I don't do it again in that place. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
On Fri, Jan 5, 2018 at 2:24 PM, Bruce Momjian <bruce@momjian.us> wrote: > On Fri, Jan 5, 2018 at 02:20:59PM -0500, Robert Haas wrote: >> On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > pg_upgrade: simplify code layout in a few places >> > >> > Backpatch-through: 9.4 (9.3 didn't need improving) >> >> Hmm. We don't normally do things like this, because it breaks translatability. > > Oh, you mean the 'if()' statement? If so, I will revert that and add a > comment so I don't do it again in that place. Yeah. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Bruce Momjian wrote: > On Fri, Jan 5, 2018 at 02:20:59PM -0500, Robert Haas wrote: > > On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > > > pg_upgrade: simplify code layout in a few places > > > > > > Backpatch-through: 9.4 (9.3 didn't need improving) > > > > Hmm. We don't normally do things like this, because it breaks translatability. > > Oh, you mean the 'if()' statement? If so, I will revert that and add a > comment so I don't do it again in that place. See 837255cc81fb59b12f5a70ac2a8a9850bccf13e0. I don't think adding comments to every single place where this could be done (which is many places, not just in pg_upgrade) is a good idea. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Jan 5, 2018 at 2:32 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > Bruce Momjian wrote: >> On Fri, Jan 5, 2018 at 02:20:59PM -0500, Robert Haas wrote: >> > On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: >> > > pg_upgrade: simplify code layout in a few places >> > > >> > > Backpatch-through: 9.4 (9.3 didn't need improving) >> > >> > Hmm. We don't normally do things like this, because it breaks translatability. >> >> Oh, you mean the 'if()' statement? If so, I will revert that and add a >> comment so I don't do it again in that place. > > See 837255cc81fb59b12f5a70ac2a8a9850bccf13e0. I don't think adding > comments to every single place where this could be done (which is many > places, not just in pg_upgrade) is a good idea. It's also documented, of course. https://www.postgresql.org/docs/devel/static/nls-programmer.html#NLS-GUIDELINES -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Jan 5, 2018 at 02:31:51PM -0500, Robert Haas wrote: > On Fri, Jan 5, 2018 at 2:24 PM, Bruce Momjian <bruce@momjian.us> wrote: > > On Fri, Jan 5, 2018 at 02:20:59PM -0500, Robert Haas wrote: > >> On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> > pg_upgrade: simplify code layout in a few places > >> > > >> > Backpatch-through: 9.4 (9.3 didn't need improving) > >> > >> Hmm. We don't normally do things like this, because it breaks translatability. > > > > Oh, you mean the 'if()' statement? If so, I will revert that and add a > > comment so I don't do it again in that place. > > Yeah. Thanks, done. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
On Fri, Jan 5, 2018 at 02:37:58PM -0500, Robert Haas wrote: > On Fri, Jan 5, 2018 at 2:32 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > Bruce Momjian wrote: > >> On Fri, Jan 5, 2018 at 02:20:59PM -0500, Robert Haas wrote: > >> > On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> > > pg_upgrade: simplify code layout in a few places > >> > > > >> > > Backpatch-through: 9.4 (9.3 didn't need improving) > >> > > >> > Hmm. We don't normally do things like this, because it breaks translatability. > >> > >> Oh, you mean the 'if()' statement? If so, I will revert that and add a > >> comment so I don't do it again in that place. > > > > See 837255cc81fb59b12f5a70ac2a8a9850bccf13e0. I don't think adding > > comments to every single place where this could be done (which is many > > places, not just in pg_upgrade) is a good idea. > > It's also documented, of course. > > https://www.postgresql.org/docs/devel/static/nls-programmer.html#NLS-GUIDELINES OK, C comment removed. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
On 2018-01-05 14:20:59 -0500, Robert Haas wrote: > On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > > pg_upgrade: simplify code layout in a few places > > > > Backpatch-through: 9.4 (9.3 didn't need improving) > > Hmm. We don't normally do things like this, because it breaks translatability. Also, leaving translatability aside, why was *any* of this backpatched? Unless there's very good maintainability reasons we normally don't backpatch minor refactorings? Greetings, Andres Freund
On Fri, Jan 5, 2018 at 03:51:15PM -0800, Andres Freund wrote: > On 2018-01-05 14:20:59 -0500, Robert Haas wrote: > > On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > > > pg_upgrade: simplify code layout in a few places > > > > > > Backpatch-through: 9.4 (9.3 didn't need improving) > > > > Hmm. We don't normally do things like this, because it breaks translatability. > > Also, leaving translatability aside, why was *any* of this backpatched? > Unless there's very good maintainability reasons we normally don't > backpatch minor refactorings? Tom has preferred that I backpatch all safe patches so we keep that code consistent so we can backpatch other things more easily. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
On 2018-01-05 18:57:55 -0500, Bruce Momjian wrote: > On Fri, Jan 5, 2018 at 03:51:15PM -0800, Andres Freund wrote: > > On 2018-01-05 14:20:59 -0500, Robert Haas wrote: > > > On Fri, Jan 5, 2018 at 2:11 PM, Bruce Momjian <bruce@momjian.us> wrote: > > > > pg_upgrade: simplify code layout in a few places > > > > > > > > Backpatch-through: 9.4 (9.3 didn't need improving) > > > > > > Hmm. We don't normally do things like this, because it breaks translatability. > > > > Also, leaving translatability aside, why was *any* of this backpatched? > > Unless there's very good maintainability reasons we normally don't > > backpatch minor refactorings? > > Tom has preferred that I backpatch all safe patches so we keep that code > consistent so we can backpatch other things more easily. I've a hard time believing this. Tom? Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2018-01-05 18:57:55 -0500, Bruce Momjian wrote:
>> On Fri, Jan 5, 2018 at 03:51:15PM -0800, Andres Freund wrote:
>>> Also, leaving translatability aside, why was *any* of this backpatched?
>> Tom has preferred that I backpatch all safe patches so we keep that code
>> consistent so we can backpatch other things more easily.
> I've a hard time believing this. Tom?
I've been known to back-patch stuff just to keep branches consistent,
but it's always a judgement call. In this case I wouldn't have done it
(even if the patch were a good idea in HEAD) because it would cause
churn in translatable messages in the back branches. Also, the case
for cosmetic back-patching is only strong when a particular file is
already pretty similar across all branches, and I'm not sure that
holds for pg_upgrade.
regards, tom lane
On Fri, Jan 5, 2018 at 08:39:46PM -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2018-01-05 18:57:55 -0500, Bruce Momjian wrote: > >> On Fri, Jan 5, 2018 at 03:51:15PM -0800, Andres Freund wrote: > >>> Also, leaving translatability aside, why was *any* of this backpatched? > > >> Tom has preferred that I backpatch all safe patches so we keep that code > >> consistent so we can backpatch other things more easily. > > > I've a hard time believing this. Tom? > > I've been known to back-patch stuff just to keep branches consistent, > but it's always a judgement call. In this case I wouldn't have done it > (even if the patch were a good idea in HEAD) because it would cause > churn in translatable messages in the back branches. Also, the case > for cosmetic back-patching is only strong when a particular file is > already pretty similar across all branches, and I'm not sure that > holds for pg_upgrade. There was a time when pg_upgrade was similar in all branches and churning a lot with fixes, so I was going on that plan. At this point I don't think that is true anymore, so maybe we can switch just to head and PG 10 on this. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +