Thread: 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
Attachment
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 HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
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
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
Attachment
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
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.
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
On 24 September 2014 12:04, Magnus Hagander <magnus@hagander.net> wrote:
I haven't looked at the code, but definitely +1 for the feature.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".
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
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
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@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
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.
Attachment
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
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
Attachment
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@vmware.com> wrote:
--
Michael
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
$ 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
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