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

On Tue, Aug 17, 2010 at 2:58 PM, Quan Zongliang <quanzongliang@gmail.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@hagander.net> wrote:

On Tue, Aug 17, 2010 at 2:58 PM, Quan Zongliang <quanzongliang@gmail.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@gmail.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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@commandprompt.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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@commandprompt.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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@commandprompt.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@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Quan Zongliang <quanzongliang@gmail.com>

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

Thanks.

On Thu, 19 Aug 2010 08:48:53 -0700
David Fetter <david@fetter.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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@gmail.com>

On Fri, Aug 20, 2010 at 01:01, Quan Zongliang <quanzongliang@gmail.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@hagander.net> wrote:

On Fri, Aug 20, 2010 at 01:01, Quan Zongliang <quanzongliang@gmail.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@gmail.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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@fetter.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.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@gmail.com>

On Tue, Aug 24, 2010 at 2:05 PM, Quan Zongliang <quanzongliang@gmail.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@gmail.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.takahiro@gmail.com> wrote:

Hi, I have a question about the latest patch.

On Sun, Aug 22, 2010 at 11:03 PM, Quan Zongliang
<quanzongliang@gmail.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/