From: | Tatsuro Yamada <yamada(dot)tatsuro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Fix help option of contrib/oid2name |
Date: | 2018-08-20 06:51:07 |
Message-ID: | 287ac54f-d8fe-4d2a-ef76-a28282cdf555@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Gierth | 2018-08-20 07:38:52 | A really subtle lexer bug |
Previous Message | Surafel Temesgen | 2018-08-20 06:37:05 | Re: Conflict handling for COPY FROM |