Re: Fix help option of contrib/oid2name - Mailing list pgsql-hackers

On 2018/08/20 13:54, Michael Paquier wrote:
> On Mon, Aug 20, 2018 at 12:30:29PM +0900, Tatsuro Yamada wrote:
>> vacuumlo:
>>     Document
>>       - Add long options
>>       - Add environment section
> 
> Let's keep things simple by not adding long options where it is not
> especially obvious, so I would suggest to keep the patch simple and just
> add long options for connection options.

Okay.

>> oid2name:
>>     Document
>>       - Add long options
>>       - Add environment section
>>       - Remove deprecated and unhandled option "-P password"
> 
> Is this a good idea?  I doubt that it is used but keeping it would not
> cause any breakage, and removing it could.

Yeah, why I remove that code is that there is no code to handle "-P" option,
and it caused error when executing the command with the option like this:

   $ oid2name -P hoge
   oid2name: invalid option -- 'P'

Therefore, "-P" is a manual bag. I investigated more using git log command and
understood followings:

   1. -P option was removed on 4192f2d85
   2. -P option revived in only the manual on 2963d8228

So, we have 2 options, remove -P option from the manual or revive the code
regarding to -P option (I mean it is like a git revert 4192f2d85).


> oid2name supports also PGDATABASE.

As far as I know, this environment variable is also unused because
I could not get the results as I expected. So, I didn't add it to the manual.
For example, following command expected that it shows about "postgres", but
it couldn't.

   $ env |grep PG
   ...
   PGDATABASE=postgres
   ...

   $ oid2name
   All databases:
       Oid  Database Name  Tablespace
   ----------------------------------
     16392         hogedb  pg_default
     13310       postgres  pg_default
     13309      template0  pg_default
         1      template1  pg_default

   $ oid2name -d postgres
   From database "postgres":
     Filenode  Table Name
   ----------------------
        16388          t1


>>     Code
>>       - Revive handling "-H host" option silently
>>
>> I didn't add "-H" to the help message because it's a deprecated
>> option.
> 
> It seems to me that this should be still documented in both --help and
> in the docs, and that we should just mark it as deprecated in both.
> 
>> I guess that that means "silently" as you said on previous email.
>> Should I add it?  For example,
>>
>> [...]
> 
> Let's use a different line for that.  See for example how pg_standby -k
> is treated.

I will do that.


> I can see why you have reordered the options, this is more consistent
> with things in src/bin/scripts.
> 
> We could have as well some basic TAP tests for both utilities while on
> it.  Something as simple as the first tests in 050_dropdb.pl would do a
> fine first shot.

For now, I'm not sure about TAP test but I knew both utilities have no
regression tests. It would be better to use something tests.


Regards,
Tatsuro Yamada