pg_ctl options checking

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Simon Riggs wrote:

pg_ctl: Minor patch to implement option checking to prevent silently
ignoring options and continuing with defaults.

All of the following now cause errors:

pg_ctl -D test -m logfile start
pg_ctl -D test -U foo start
pg_ctl -D test -W start
pg_ctl -D test -w stop
pg_ctl -D test -l fast stop
etc.. (16 possible error combinations in total now checked)

The following also fails, following the --help, though I think we might
want to change this so that it does actually work:

pg_ctl -D test -l logfile restart

Patch against cvstip, make check, tested on long and short options.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Simon Riggs wrote:

pg_ctl -D test -U foo start
pg_ctl -D test -W start
pg_ctl -D test -w stop
pg_ctl -D test -l fast stop
etc.. (16 possible error combinations in total now checked)

I object to making these throw an error. It is very convenient to be
able to alter the command line from start to stop to reload etc.
without having to remove or rewrite all the options that are not really
relevant for that particular operation.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut wrote:

Simon Riggs wrote:

pg_ctl -D test -U foo start
pg_ctl -D test -W start
pg_ctl -D test -w stop
pg_ctl -D test -l fast stop
etc.. (16 possible error combinations in total now checked)

I object to making these throw an error. It is very convenient to be
able to alter the command line from start to stop to reload etc.
without having to remove or rewrite all the options that are not really
relevant for that particular operation.

For a command as significant as pg_ctl, I can't see how making it
_convenient_ is a good argument.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

Peter Eisentraut wrote:

Simon Riggs wrote:

pg_ctl -D test -U foo start
pg_ctl -D test -W start
pg_ctl -D test -w stop
pg_ctl -D test -l fast stop
etc.. (16 possible error combinations in total now checked)

I object to making these throw an error. It is very convenient to
be able to alter the command line from start to stop to reload etc.
without having to remove or rewrite all the options that are not
really relevant for that particular operation.

For a command as significant as pg_ctl, I can't see how making it
_convenient_ is a good argument.

Well, loss of convenience is one argument in opposition to this change
but I don't see any argument in _favor_ of this change other than
"let's reject these option combinations", some of which seem perfectly
valid.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut wrote:

Bruce Momjian wrote:

Peter Eisentraut wrote:

Simon Riggs wrote:

pg_ctl -D test -U foo start
pg_ctl -D test -W start
pg_ctl -D test -w stop
pg_ctl -D test -l fast stop
etc.. (16 possible error combinations in total now checked)

I object to making these throw an error. It is very convenient to
be able to alter the command line from start to stop to reload etc.
without having to remove or rewrite all the options that are not
really relevant for that particular operation.

For a command as significant as pg_ctl, I can't see how making it
_convenient_ is a good argument.

Well, loss of convenience is one argument in opposition to this change
but I don't see any argument in _favor_ of this change other than
"let's reject these option combinations", some of which seem perfectly
valid.

Well, if the option combination makes no sense, it seems we should
reject it, no? Isn't that part of what we do with SQL too.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Peter Eisentraut <peter_e@gmx.net> writes:

Bruce Momjian wrote:

For a command as significant as pg_ctl, I can't see how making it
_convenient_ is a good argument.

Well, loss of convenience is one argument in opposition to this change
but I don't see any argument in _favor_ of this change other than
"let's reject these option combinations", some of which seem perfectly
valid.

Ignoring irrelevant arguments is a time-honored Unix tradition that
contributes significantly to the usefulness of cc, for example.
Would you be happy if cc rejected -D when being used only to link, say?

I hadn't thought about this when Simon submitted the patch, but I'm
with Peter: we should not reject arguments just because they're not
relevant. If you can make a case that particular combinations strongly
suggest user error, then let's reject those cases ... but not a blanket
prohibition.

regards, tom lane

Patch rejected due to user complaints.

---------------------------------------------------------------------------

Simon Riggs wrote:

pg_ctl: Minor patch to implement option checking to prevent silently
ignoring options and continuing with defaults.

All of the following now cause errors:

pg_ctl -D test -m logfile start
pg_ctl -D test -U foo start
pg_ctl -D test -W start
pg_ctl -D test -w stop
pg_ctl -D test -l fast stop
etc.. (16 possible error combinations in total now checked)

The following also fails, following the --help, though I think we might
want to change this so that it does actually work:

pg_ctl -D test -l logfile restart

Patch against cvstip, make check, tested on long and short options.

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Mon, 2006-04-17 at 15:12 -0400, Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Bruce Momjian wrote:

For a command as significant as pg_ctl, I can't see how making it
_convenient_ is a good argument.

Well, loss of convenience is one argument in opposition to this change
but I don't see any argument in _favor_ of this change other than
"let's reject these option combinations", some of which seem perfectly
valid.

Ignoring irrelevant arguments is a time-honored Unix tradition that
contributes significantly to the usefulness of cc, for example.
Would you be happy if cc rejected -D when being used only to link, say?

I hadn't thought about this when Simon submitted the patch, but I'm
with Peter: we should not reject arguments just because they're not
relevant. If you can make a case that particular combinations strongly
suggest user error, then let's reject those cases ... but not a blanket
prohibition.

AFAICS -l -o on stop and -m on start could be ignored

Mixing options between register and non-registration commands definitely
indicates user error. So does mixing up -w and -W

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com/

Simon Riggs wrote:

AFAICS -l -o on stop and -m on start could be ignored

Right.

Mixing options between register and non-registration commands
definitely indicates user error.

That I agree on.

So does mixing up -w and -W

I don't follow that. These options are perfectly good for all modes
(except registration).

--
Peter Eisentraut
http://developer.postgresql.org/~petere/