Re: make pg_controldata accept "-D dirname"

Hi.

I can never remember that pg_controldata takes only a dirname rather
than "-D dirname", and I gather I'm not the only one. Here's a tiny
patch to make it work either way. As far as I can see, it doesn't
break anything, not even if you have a data directory named "-D".

-- Abhijit

On 09/24/2014 01:59 PM, Abhijit Menon-Sen wrote:
> Hi.
>
> I can never remember that pg_controldata takes only a dirname rather
> than "-D dirname", and I gather I'm not the only one. Here's a tiny
> patch to make it work either way. As far as I can see, it doesn't
> break anything, not even if you have a data directory named "-D".

Ah, I frequently run into that too; but with pg_resetxlog.

- Heikki

On Wed, Sep 24, 2014 at 12:59 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> Hi.
>
> I can never remember that pg_controldata takes only a dirname rather
> than "-D dirname", and I gather I'm not the only one. Here's a tiny
> patch to make it work either way. As far as I can see, it doesn't
> break anything, not even if you have a data directory named "-D".

I haven't looked at the code, but definitely +1 for the feature.
That's really quite annoying.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

At 2014-09-24 14:03:41 +0300, hlinnakangas(at)vmware(dot)com wrote:
>
> Ah, I frequently run into that too; but with pg_resetxlog.

Well, that's no fun. Patch attached.

-- Abhijit

On Wed, Sep 24, 2014 at 7:59 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> I can never remember that pg_controldata takes only a dirname rather
> than "-D dirname", and I gather I'm not the only one. Here's a tiny
> patch to make it work either way. As far as I can see, it doesn't
> break anything, not even if you have a data directory named "-D".
+1. I forget it all the time as well...
--
Michael

On 24 September 2014 17:15, Michael Paquier Wrote:

> On Wed, Sep 24, 2014 at 7:59 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>
> wrote:
> > I can never remember that pg_controldata takes only a dirname rather
> > than "-D dirname", and I gather I'm not the only one. Here's a tiny
> > patch to make it work either way. As far as I can see, it doesn't
> > break anything, not even if you have a data directory named "-D".
> +1. I forget it all the time as well...

+1, always I get confused once pg_controldata does not work with -D.

Magnus Hagander wrote:
> On Wed, Sep 24, 2014 at 12:59 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com> wrote:
> > Hi.
> >
> > I can never remember that pg_controldata takes only a dirname rather
> > than "-D dirname", and I gather I'm not the only one. Here's a tiny
> > patch to make it work either way. As far as I can see, it doesn't
> > break anything, not even if you have a data directory named "-D".
>
> I haven't looked at the code, but definitely +1 for the feature.
> That's really quite annoying.

+1

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 24 September 2014 12:04, Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> On Wed, Sep 24, 2014 at 12:59 PM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>
> wrote:
> > Hi.
> >
> > I can never remember that pg_controldata takes only a dirname rather
> > than "-D dirname", and I gather I'm not the only one. Here's a tiny
> > patch to make it work either way. As far as I can see, it doesn't
> > break anything, not even if you have a data directory named "-D".
>
> I haven't looked at the code, but definitely +1 for the feature.
> That's really quite annoying.
>

And here I was thinking it was just me.

+1

Thom

On 09/24/2014 02:21 PM, Abhijit Menon-Sen wrote:
> At 2014-09-24 14:03:41 +0300, hlinnakangas(at)vmware(dot)com wrote:
>>
>> Ah, I frequently run into that too; but with pg_resetxlog.
>
> Well, that's no fun. Patch attached.

Thanks. Please update the docs and usage(), too.

- Heikki

At 2014-09-24 16:02:29 +0300, hlinnakangas(at)vmware(dot)com wrote:
>
> Thanks. Please update the docs and usage(), too.

I'm sorry, but I don't think it would be an improvement to make the docs
explain that it's valid to use either "-D datadir" or specify it without
an option. If both commands were changed to *require* "-D datadir", that
would be worth documenting. But of course I didn't want to change any
behaviour for people who have a better memory than I do.

I think it's all right as a quick hack to remove an inconsistency that
has clearly annoyed many people, but not much more.

Would you be OK with my just sticking a "[-D]" before DATADIR in the
usage() message for both programs, with no further explanation? (But
to be honest, I don't think even that is necessary or desirable.)

-- Abhijit

To put it another way, I doubt any of the people who were surprised by
it looked at either the usage message or the docs. ;-)

-- Abhijit

Abhijit Menon-Sen <ams(at)2ndQuadrant(dot)com> writes:
> At 2014-09-24 16:02:29 +0300, hlinnakangas(at)vmware(dot)com wrote:
>> Thanks. Please update the docs and usage(), too.

> I'm sorry, but I don't think it would be an improvement to make the docs
> explain that it's valid to use either "-D datadir" or specify it without
> an option.

What's so hard about [ -D ] before the datadir argument?

regards, tom lane

At 2014-09-24 09:25:12 -0400, tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>
> What's so hard about [ -D ] before the datadir argument?

I'm sorry to have given you the impression that I objected to it because
it was hard. Since I proposed the same thing a few lines after what you
quoted, I guess I have to agree it's not hard.

I think it's pointless, because if you're going to look at the usage
message to begin with, why not do what it already says? But I don't
care enough to argue about it any further.

Patches attached.

-- Abhijit

P.S. Trivia: I can't find any other options in Postgres where the
argument is mandatory but the option is optional.

On 09/24/2014 04:49 PM, Abhijit Menon-Sen wrote:
> At 2014-09-24 09:25:12 -0400, tgl(at)sss(dot)pgh(dot)pa(dot)us wrote:
>>
>> What's so hard about [ -D ] before the datadir argument?
>
> I'm sorry to have given you the impression that I objected to it because
> it was hard. Since I proposed the same thing a few lines after what you
> quoted, I guess I have to agree it's not hard.
>
> I think it's pointless, because if you're going to look at the usage
> message to begin with, why not do what it already says? But I don't
> care enough to argue about it any further.

There's also the reverse situation: you see that a script contains a
line like "pg_controldata -D foo". You're accustomed to doing just
"pg_controldata foo", and you wonder what the -D option does. So you
look it up in the docs or "pg_controldata --help".

- Heikki

Abhijit Menon-Sen wrote:

> P.S. Trivia: I can't find any other options in Postgres where the
> argument is mandatory but the option is optional.

psql behaves similarly with its -d and -U switches also; note --help:

Usage:
psql [OPTION]... [DBNAME [USERNAME]]
...
-d, --dbname=DBNAME database name to connect to (default: "alvherre")
...
-U, --username=USERNAME database user name (default: "alvherre")

Both are optional and have defaults. Datadir for pg_controldata is also
optional and --help contains this blurb:

If no data directory (DATADIR) is specified, the environment variable PGDATA
is used.

I think you could replace those lines with
-D data directory blah (default: "value of $PGDATA")

But psql --help doesn't use $PGUSER to expand the default value for -U;
I'm not clear if this means we shouldn't expand PGDATA in the help line
for pg_controldata, or need to fix psql to expand PGUSER in -U.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Updated patches attached.

-- Abhijit

On 09/24/2014 05:48 PM, Abhijit Menon-Sen wrote:
> Updated patches attached.

Thanks, applied some version of these.

- Heikki

On Thu, Sep 25, 2014 at 12:38 PM, Heikki Linnakangas <
hlinnakangas(at)vmware(dot)com> wrote:

> On 09/24/2014 05:48 PM, Abhijit Menon-Sen wrote:
>
>> Updated patches attached.
>>
>
> Thanks, applied some version of these.
>
This set of patches has been applied as b0d81ad but patch 0001 did not make
it in, so pg_controldata is not yet able to accept -D:
$ pg_controldata -D /foo/path
pg_controldata: could not open file "-D/global/pg_control" for reading: No
such file or directory
$ pg_controldata /foo/path
pg_controldata: could not open file "/foo/path/global/pg_control" for
reading: No such file or directory
--
Michael

On 10/24/2014 11:36 AM, Michael Paquier wrote:
> On Thu, Sep 25, 2014 at 12:38 PM, Heikki Linnakangas <
> hlinnakangas(at)vmware(dot)com> wrote:
>
>> On 09/24/2014 05:48 PM, Abhijit Menon-Sen wrote:
>>
>>> Updated patches attached.
>>>
>>
>> Thanks, applied some version of these.
>>
> This set of patches has been applied as b0d81ad but patch 0001 did not make
> it in, so pg_controldata is not yet able to accept -D:
> $ pg_controldata -D /foo/path
> pg_controldata: could not open file "-D/global/pg_control" for reading: No
> such file or directory
> $ pg_controldata /foo/path
> pg_controldata: could not open file "/foo/path/global/pg_control" for
> reading: No such file or directory

Argh, looks like I forgot the actual code changes required.

I just noticed that pg_controldata and pg_resetxlog don't check for
extra arguments:

$ pg_resetxlog data fds sdf sdf
Transaction log reset

Fixed that too.

- Heikki

On Sat, Oct 25, 2014 at 1:20 AM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> Argh, looks like I forgot the actual code changes required.
> I just noticed that pg_controldata and pg_resetxlog don't check for extra
> arguments:
> $ pg_resetxlog data fds sdf sdf
> Transaction log reset
I think that it would be good to add as well a set of TAP tests for
all those things. So attached are two patches, one to complete the
tests of pg_controldata, and a second one to add TAP tests for
pg_resetxlog.
--
Michael