make pg_controldata accept "-D dirname"

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Sep 24, 2014 at 12:59 PM, Abhijit Menon-Sen <ams@2ndquadrant.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/

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

At 2014-09-24 14:03:41 +0300, hlinnakangas@vmware.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@2ndquadrant.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Wed, Sep 24, 2014 at 7:59 PM, Abhijit Menon-Sen <ams@2ndquadrant.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.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Magnus Hagander wrote:

On Wed, Sep 24, 2014 at 12:59 PM, Abhijit Menon-Sen <ams@2ndquadrant.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 24 September 2014 12:04, Magnus Hagander <magnus@hagander.net> wrote:

On Wed, Sep 24, 2014 at 12:59 PM, Abhijit Menon-Sen <ams@2ndquadrant.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@vmware.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

At 2014-09-24 16:02:29 +0300, hlinnakangas@vmware.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Abhijit Menon-Sen <ams@2ndQuadrant.com> writes:

At 2014-09-24 16:02:29 +0300, hlinnakangas@vmware.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

At 2014-09-24 09:25:12 -0400, tgl@sss.pgh.pa.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@sss.pgh.pa.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Sep 25, 2014 at 12:38 PM, Heikki Linnakangas <
hlinnakangas@vmware.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@vmware.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Oct 25, 2014 at 1:20 AM, Heikki Linnakangas
<hlinnakangas@vmware.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