Обсуждение: pg_upgrade's object listing

Hello.

I found the following message recently introduced in pg_upgrade:

>        pg_log(PG_VERBOSE, "slot_name: \"%s\", plugin: \"%s\", two_phase: %s",
>               slot_info->slotname,
>               slot_info->plugin,
>               slot_info->two_phase ? "true" : "false");

If the labels correspond to the struct member names, the first label
ought to be "slotname". If not, all labels of this type, including
those adjucent, should have a more natural spelling.

What do you think about this?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

On Friday, October 27, 2023 1:21 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>
> Hello.
>
> I found the following message recently introduced in pg_upgrade:
>
> >        pg_log(PG_VERBOSE, "slot_name: \"%s\", plugin: \"%s\",
> two_phase: %s",
> >               slot_info->slotname,
> >               slot_info->plugin,
> >               slot_info->two_phase ? "true" : "false");
>
> If the labels correspond to the struct member names, the first label ought to be
> "slotname". If not, all labels of this type, including those adjucent, should have a
> more natural spelling.
>
> What do you think about this?

Thanks for reporting. But I am not sure if rename to slotname or others will be an
improvement. I think we don't have a rule to make the output the same as struct
field. Existing message also don't follow it[1]. So, the current message looks
OK to me.

[1]
        pg_log(PG_VERBOSE, "relname: \"%s.%s\", reloid: %u, reltblspace: \"%s\"",
               rel_arr->rels[relnum].nspname,
               rel_arr->rels[relnum].relname,
               rel_arr->rels[relnum].reloid,
               rel_arr->rels[relnum].tablespace);

Best Regards,
Hou zj

At Fri, 27 Oct 2023 05:56:31 +0000, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com> wrote in 
> On Friday, October 27, 2023 1:21 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> > 
> > Hello.
> > 
> > I found the following message recently introduced in pg_upgrade:
> > 
> > >        pg_log(PG_VERBOSE, "slot_name: \"%s\", plugin: \"%s\",
> > two_phase: %s",
> > >               slot_info->slotname,
> > >               slot_info->plugin,
> > >               slot_info->two_phase ? "true" : "false");
> > 
> > If the labels correspond to the struct member names, the first label ought to be
> > "slotname". If not, all labels of this type, including those adjucent, should have a
> > more natural spelling.
> > 
> > What do you think about this?
> 
> Thanks for reporting. But I am not sure if rename to slotname or others will be an
> improvement. I think we don't have a rule to make the output the same as struct
> field. Existing message also don't follow it[1]. So, the current message looks
> OK to me.
> 
> [1]
>         pg_log(PG_VERBOSE, "relname: \"%s.%s\", reloid: %u, reltblspace: \"%s\"",
>                rel_arr->rels[relnum].nspname,
>                rel_arr->rels[relnum].relname,
>                rel_arr->rels[relnum].reloid,
>                rel_arr->rels[relnum].tablespace);

Thanks for sharing your perspectie. I share similar sentiments. The
initial question arose during the message translation.  For the
subsequent one, I opted not to translate the labels as they looked to
be member names. From this viewpoint, "slot_name" is rather ambiguous.

If there's no interest in modifying it, I will retain the original
labels in translated messages, and that should suffice.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Hi,

On 2023-Oct-27, Kyotaro Horiguchi wrote:

> I found the following message recently introduced in pg_upgrade:
> 
> >        pg_log(PG_VERBOSE, "slot_name: \"%s\", plugin: \"%s\", two_phase: %s",
> >               slot_info->slotname,
> >               slot_info->plugin,
> >               slot_info->two_phase ? "true" : "false");
> 
> If the labels correspond to the struct member names, the first label
> ought to be "slotname". If not, all labels of this type, including
> those adjucent, should have a more natural spelling.
> 
> What do you think about this?

I think this shouldn't be a translatable message in the first place.

Looking at the wording of other messages in pg_upgrade --verbose,it
doesn't look like any of it is intended for user consumption.  I mean,
look at this monstrosity

        pg_log(PG_VERBOSE, "relname: \"%s.%s\", reloid: %u, reltblspace: \"%s\"",

Before 249d74394500 it used to be even more hideous.  This message comes
straight from the initial pg_upgrade commit in 2010, c2e9b2f28818, where
it was a debug message.  We seem to have promoted it to a verbose
message (commit 717f6d60859c) for no particular reason and without
careful consideration.

I honestly doubt that this sort of message is in any way useful, other
than for program debugging.  Maybe listing databases and perhaps slots
in verbose mode is OK, but tables?  I don't think so.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"I'm impressed how quickly you are fixing this obscure issue. I came from 
MS SQL and it would be hard for me to put into words how much of a better job
you all are doing on [PostgreSQL]."
 Steve Midgley, http://archives.postgresql.org/pgsql-sql/2008-08/msg00000.php

> On 27 Oct 2023, at 10:44, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

> I honestly doubt that this sort of message is in any way useful, other
> than for program debugging.  Maybe listing databases and perhaps slots
> in verbose mode is OK, but tables?  I don't think so.

Outputting this in verbose mode is unlikely to help in regular usage and
instead risk drown out other outputs.  It would be more useful to be able to
specify a logfile for objects and keep verbose output for more informational
and actionable messages.

--
Daniel Gustafsson