Re: Fw: patch for pg_ctl.c to add windows service start-type

Sorry.
I forget to attach the patch file.

Begin forwarded message:

Date: Mon, 16 Aug 2010 19:49:20 +0800
From: Quan Zongliang <quanzongliang(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: patch for pg_ctl.c to add windows service start-type

Hi, all

I modified pg_ctl.c to add a new option for Windows service start-type.
new option is -S [auto|demand]

For example, the command can be used under Windows:
pg_ctl register -N "s-name" -S auto
or
pg_ctl register -N "s-name" -S demand

The created service will be SERVICE_AUTO_START or SERVICE_DEMAND_START respectively.

regards

--
Quan Zongliang <quanzongliang(at)gmail(dot)com>

--
Quan Zongliang <quanzongliang(at)gmail(dot)com>

On Tue, Aug 17, 2010 at 2:58 PM, Quan Zongliang <quanzongliang(at)gmail(dot)com> wrote:
>
> Sorry.
> I forget to attach the patch file.

Without looking at the details of this patch, it looks reasonable - so
please put it on the commitfest page, if you haven't already.

It does, however, lack documentation updates - that needs to be done
before it can get applied.

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

documents attached. html and man-page

On Tue, 17 Aug 2010 16:18:42 +0200
Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> On Tue, Aug 17, 2010 at 2:58 PM, Quan Zongliang <quanzongliang(at)gmail(dot)com> wrote:
> >
> > Sorry.
> > I forget to attach the patch file.
>
> Without looking at the details of this patch, it looks reasonable - so
> please put it on the commitfest page, if you haven't already.
>
> It does, however, lack documentation updates - that needs to be done
> before it can get applied.
>
> --
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.com/

--
Quan Zongliang <quanzongliang(at)gmail(dot)com>

On Thu, Aug 19, 2010 at 10:24:54PM +0800, Quan Zongliang wrote:
> documents attached. html and man-page

Thanks!

For future reference, the way to patch docs is by patching the SGML
source. Please find enclosed a patch which incorporates the code
patch you sent with these docs.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:

> + <varlistentry>
> + <term><option>-S <replaceable class="parameter"></replaceable></option></term>

You omitted the start-type inside the <replaceable> tag. Also, the "a"
and "d" values seem to be accepted but not documented.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
>
> > + <varlistentry>
> > + <term><option>-S <replaceable class="parameter"></replaceable></option></term>
>
> You omitted the start-type inside the <replaceable> tag. Also, the "a"
> and "d" values seem to be accepted but not documented.

D'oh! Changed patch enclosed. Now in context format :)

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Excerpts from David Fetter's message of jue ago 19 16:40:18 -0400 2010:
> On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> > Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
> >
> > > + <varlistentry>
> > > + <term><option>-S <replaceable class="parameter"></replaceable></option></term>
> >
> > You omitted the start-type inside the <replaceable> tag. Also, the "a"
> > and "d" values seem to be accepted but not documented.
>
> D'oh! Changed patch enclosed. Now in context format :)

Thanks.

Another thing -- why is there an enum at all? Seems it'd be
simpler to assign the right value to the variable in the getopt() code
to start with.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Thu, Aug 19, 2010 at 04:48:53PM -0400, Alvaro Herrera wrote:
> Excerpts from David Fetter's message of jue ago 19 16:40:18 -0400 2010:
> > On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> > > Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
> > >
> > > > + <varlistentry>
> > > > + <term><option>-S <replaceable class="parameter"></replaceable></option></term>
> > >
> > > You omitted the start-type inside the <replaceable> tag. Also,
> > > the "a" and "d" values seem to be accepted but not documented.
> >
> > D'oh! Changed patch enclosed. Now in context format :)
>
> Thanks.
>
> Another thing -- why is there an enum at all? Seems it'd be simpler
> to assign the right value to the variable in the getopt() code to
> start with.

That's a question for the patch author. I was just cleaning up the
docs :)

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Because Windows's CreateService has serial start-type:
SERVICE_AUTO_START
SERVICE_BOOT_START
SERVICE_DEMAND_START
SERVICE_DISABLED
SERVICE_SYSTEM_START

Although all of them are not useful for pg service.
I think it is better to use enum.

On Thu, 19 Aug 2010 16:48:53 -0400
Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:

> Excerpts from David Fetter's message of jue ago 19 16:40:18 -0400 2010:
> > On Thu, Aug 19, 2010 at 03:47:43PM -0400, Alvaro Herrera wrote:
> > > Excerpts from David Fetter's message of jue ago 19 11:48:53 -0400 2010:
> > >
> > > > + <varlistentry>
> > > > + <term><option>-S <replaceable class="parameter"></replaceable></option></term>
> > >
> > > You omitted the start-type inside the <replaceable> tag. Also, the "a"
> > > and "d" values seem to be accepted but not documented.
> >
> > D'oh! Changed patch enclosed. Now in context format :)
>
> Thanks.
>
> Another thing -- why is there an enum at all? Seems it'd be
> simpler to assign the right value to the variable in the getopt() code
> to start with.
>
> --
> Álvaro Herrera <alvherre(at)commandprompt(dot)com>
> The PostgreSQL Company - Command Prompt, Inc.
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Quan Zongliang <quanzongliang(at)gmail(dot)com>

I don't know how to edit documents exactly before.

Thanks.

On Thu, 19 Aug 2010 08:48:53 -0700
David Fetter <david(at)fetter(dot)org> wrote:

> On Thu, Aug 19, 2010 at 10:24:54PM +0800, Quan Zongliang wrote:
> > documents attached. html and man-page
>
> Thanks!
>
> For future reference, the way to patch docs is by patching the SGML
> source. Please find enclosed a patch which incorporates the code
> patch you sent with these docs.
>
> Cheers,
> David.
> --
> David Fetter <david(at)fetter(dot)org> http://fetter.org/
> Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
> Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
> iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Quan Zongliang <quanzongliang(at)gmail(dot)com>

On Fri, Aug 20, 2010 at 01:01, Quan Zongliang <quanzongliang(at)gmail(dot)com> wrote:
> Because Windows's CreateService has serial start-type:
> SERVICE_AUTO_START
> SERVICE_BOOT_START
> SERVICE_DEMAND_START
> SERVICE_DISABLED
> SERVICE_SYSTEM_START
>
> Although all of them are not useful for pg service.
> I think it is better to use enum.

I don't see us ever using anything other than auto or demand. The
others aren't for "regular services", except for "disabled". And
adding a disabled service makes no sense :-) So I'm with Alvaro, I
think it's a good idea to simplify that.

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

Sure, I agree.
New patch attached. How about this?

On Fri, 20 Aug 2010 11:21:18 +0200
Magnus Hagander <magnus(at)hagander(dot)net> wrote:

> On Fri, Aug 20, 2010 at 01:01, Quan Zongliang <quanzongliang(at)gmail(dot)com> wrote:
> > Because Windows's CreateService has serial start-type:
> > SERVICE_AUTO_START
> > SERVICE_BOOT_START
> > SERVICE_DEMAND_START
> > SERVICE_DISABLED
> > SERVICE_SYSTEM_START
> >
> > Although all of them are not useful for pg service.
> > I think it is better to use enum.
>
> I don't see us ever using anything other than auto or demand. The
> others aren't for "regular services", except for "disabled". And
> adding a disabled service makes no sense :-) So I'm with Alvaro, I
> think it's a good idea to simplify that.
>
>
> --
>  Magnus Hagander
>  Me: http://www.hagander.net/
>  Work: http://www.redpill-linpro.com/

--
Quan Zongliang <quanzongliang(at)gmail(dot)com>

On Sun, Aug 22, 2010 at 10:03:32PM +0800, Quan Zongliang wrote:
> Sure, I agree.
> New patch attached. How about this?

Docs re-added. Please not to leave these out in future patches. :)

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Which files need be modified?
sgml, html, and man-page or only sgml?
I am not familiar with this.

On Sun, 22 Aug 2010 08:25:59 -0700
David Fetter <david(at)fetter(dot)org> wrote:

> On Sun, Aug 22, 2010 at 10:03:32PM +0800, Quan Zongliang wrote:
> > Sure, I agree.
> > New patch attached. How about this?
>
> Docs re-added. Please not to leave these out in future patches. :)
>
> Cheers,
> David.
> --
> David Fetter <david(at)fetter(dot)org> http://fetter.org/
> Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
> Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
> iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics
>
> Remember to vote!
> Consider donating to Postgres: http://www.postgresql.org/about/donate

--
Quan Zongliang <quanzongliang(at)gmail(dot)com>

On Tue, Aug 24, 2010 at 2:05 PM, Quan Zongliang <quanzongliang(at)gmail(dot)com> wrote:
> Which files need be modified?
> sgml, html, and man-page or only sgml?
> I am not familiar with this.

Only SGML. HTML and man pages are generated from the SGML.

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

Hi, I have a question about the latest patch.

On Sun, Aug 22, 2010 at 11:03 PM, Quan Zongliang
<quanzongliang(at)gmail(dot)com> wrote:
> New patch attached. How about this?
>> I don't see us ever using anything other than auto or demand. The
>> others aren't for "regular services"

+ set_starttype(char *starttypeopt)
+ {
+ if (strcmp(starttypeopt, "a") == 0 || strcmp(starttypeopt, "auto") == 0)
+ pgctl_start_type = SERVICE_AUTO_START;
+ else if (strcmp(starttypeopt, "d") == 0 || strcmp(starttypeopt,
"demand") == 0)
+ pgctl_start_type = SERVICE_DEMAND_START;

It accepts only "a" and "auto" for auto, but "au" or "aut" are rejected.
Is is an intended behavior? I think we can use prefix match here because
we use the logic in some places. For example,

postgres=# SELECT 'tru'::bool, 'fal'::bool;
bool | bool
------+------
t | f
(1 row)

--
Itagaki Takahiro

On Thu, Sep 30, 2010 at 04:40, Itagaki Takahiro
<itagaki(dot)takahiro(at)gmail(dot)com> wrote:
> Hi, I have a question about the latest patch.
>
> On Sun, Aug 22, 2010 at 11:03 PM, Quan Zongliang
> <quanzongliang(at)gmail(dot)com> wrote:
>> New patch attached. How about this?
>>> I don't see us ever using anything other than auto or demand. The
>>> others aren't for "regular services"
>
> + set_starttype(char *starttypeopt)
> + {
> +       if (strcmp(starttypeopt, "a") == 0 || strcmp(starttypeopt, "auto") == 0)
> +               pgctl_start_type = SERVICE_AUTO_START;
> +       else if (strcmp(starttypeopt, "d") == 0 || strcmp(starttypeopt,
> "demand") == 0)
> +               pgctl_start_type = SERVICE_DEMAND_START;
>
> It accepts only "a" and "auto" for auto, but "au" or "aut" are rejected.
> Is is an intended behavior?  I think we can use prefix match here because
> we use the logic in some places. For example,

I think it's modeled on what we allow for the "-m" parameter, and ISTM
it's good to keep those consistent.

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