Thread: pg_ctl options checking
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
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/