pg_upgrade: allow multiple -o/-O options
Hello,
RFE: Consider that you want to run pg_upgrade via some script with some
default '-o' option. But then you also want to give the script's user a
chance to specify the old-server's options according user's needs.
Then something like the following is not possible:
$ cat script
...
pg_upgrade ... -o 'sth' $PG_UPGRADE_OPT ...
...
I know that this problem is still script-able, but the fix should be
innocent and it would simplify things. Thanks for considering,
Pavel
Attachments:
0001-pg_upgrade-allow-passing-multiple-o-O-options.patchtext/x-patch; charset=utf-8; name=0001-pg_upgrade-allow-passing-multiple-o-O-options.patchDownload
>From 44ac4867a6fb67ab086ba22db8d0ad2788e9860e Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Tue, 4 Mar 2014 15:58:16 +0100
Subject: [PATCH] pg_upgrade: allow passing multiple -o/-O options
The final options passed to subsequent servers are concatenated
from particular option arguments.
---
contrib/pg_upgrade/option.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index cd9f66d..d3d2460
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
***************
*** 25,30 ****
--- 25,31 ----
static void usage(void);
static void check_required_directory(char **dirpath, char **configpath,
char *envVarName, char *cmdLineOption, char *description);
+ static void append_opt(char **str, const char *str2);
#define FIX_DEFAULT_READ_ONLY "-c default_transaction_read_only=false"
*************** parseCommandLine(int argc, char *argv[])
*** 138,148 ****
break;
case 'o':
! old_cluster.pgopts = pg_strdup(optarg);
break;
case 'O':
! new_cluster.pgopts = pg_strdup(optarg);
break;
/*
--- 139,149 ----
break;
case 'o':
! append_opt(&old_cluster.pgopts, optarg);
break;
case 'O':
! append_opt(&new_cluster.pgopts, optarg);
break;
/*
*************** parseCommandLine(int argc, char *argv[])
*** 232,237 ****
--- 233,252 ----
}
+ static void
+ append_opt(char **str, const char *str2)
+ {
+ if (!*str)
+ *str = pg_strdup(str2);
+ else
+ {
+ *str = pg_realloc(*str, strlen(*str) + strlen(str2) + 2);
+ strcat(*str, " ");
+ strcat(*str, str2);
+ }
+ }
+
+
static void
usage(void)
{
--
1.8.5.3
On Tue, Mar 4, 2014 at 04:52:56PM +0100, Pavel Raiskup wrote:
Hello,
RFE: Consider that you want to run pg_upgrade via some script with some
default '-o' option. But then you also want to give the script's user a
chance to specify the old-server's options according user's needs.
Then something like the following is not possible:$ cat script
...
pg_upgrade ... -o 'sth' $PG_UPGRADE_OPT ...
...I know that this problem is still script-able, but the fix should be
innocent and it would simplify things. Thanks for considering,
Attached is a patch that makes multiple -o options append their
arguments for pg_upgrade and pg_ctl, and documents this and the append
behavior of postmaster/postgres. This covers all the -o behaviors.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
Attachments:
dash-o.difftext/x-diff; charset=us-asciiDownload
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
new file mode 100644
index bb594dd..cfc88ec
*** a/contrib/pg_upgrade/option.c
--- b/contrib/pg_upgrade/option.c
*************** parseCommandLine(int argc, char *argv[])
*** 137,153 ****
break;
case 'o':
! old_cluster.pgopts = pg_strdup(optarg);
break;
case 'O':
! new_cluster.pgopts = pg_strdup(optarg);
break;
/*
* Someday, the port number option could be removed and passed
* using -o/-O, but that requires postmaster -C to be
! * supported on all old/new versions.
*/
case 'p':
if ((old_cluster.port = atoi(optarg)) <= 0)
--- 137,171 ----
break;
case 'o':
! /* append option? */
! if (!old_cluster.pgopts)
! old_cluster.pgopts = pg_strdup(optarg);
! else
! {
! char *old_pgopts = old_cluster.pgopts;
!
! old_cluster.pgopts = psprintf("%s %s", old_pgopts, optarg);
! free(old_pgopts);
! }
break;
case 'O':
! /* append option? */
! if (!new_cluster.pgopts)
! new_cluster.pgopts = pg_strdup(optarg);
! else
! {
! char *new_pgopts = new_cluster.pgopts;
!
! new_cluster.pgopts = psprintf("%s %s", new_pgopts, optarg);
! free(new_pgopts);
! }
break;
/*
* Someday, the port number option could be removed and passed
* using -o/-O, but that requires postmaster -C to be
! * supported on all old/new versions (added in PG 9.2).
*/
case 'p':
if ((old_cluster.port = atoi(optarg)) <= 0)
diff --git a/doc/src/sgml/pgupgrade.sgml b/doc/src/sgml/pgupgrade.sgml
new file mode 100644
index b79f0db..dd57c5c
*** a/doc/src/sgml/pgupgrade.sgml
--- b/doc/src/sgml/pgupgrade.sgml
***************
*** 130,143 ****
<term><option>-o</option> <replaceable class="parameter">options</replaceable></term>
<term><option>--old-options</option> <replaceable class="parameter">options</replaceable></term>
<listitem><para>options to be passed directly to the
! old <command>postgres</command> command</para></listitem>
</varlistentry>
<varlistentry>
<term><option>-O</option> <replaceable class="parameter">options</replaceable></term>
<term><option>--new-options</option> <replaceable class="parameter">options</replaceable></term>
<listitem><para>options to be passed directly to the
! new <command>postgres</command> command</para></listitem>
</varlistentry>
<varlistentry>
--- 130,145 ----
<term><option>-o</option> <replaceable class="parameter">options</replaceable></term>
<term><option>--old-options</option> <replaceable class="parameter">options</replaceable></term>
<listitem><para>options to be passed directly to the
! old <command>postgres</command> command; multiple
! option invocations are appended</para></listitem>
</varlistentry>
<varlistentry>
<term><option>-O</option> <replaceable class="parameter">options</replaceable></term>
<term><option>--new-options</option> <replaceable class="parameter">options</replaceable></term>
<listitem><para>options to be passed directly to the
! new <command>postgres</command> command; multiple
! option invocations are appended</para></listitem>
</varlistentry>
<varlistentry>
diff --git a/doc/src/sgml/ref/pg_ctl-ref.sgml b/doc/src/sgml/ref/pg_ctl-ref.sgml
new file mode 100644
index 2368129..29f882b
*** a/doc/src/sgml/ref/pg_ctl-ref.sgml
--- b/doc/src/sgml/ref/pg_ctl-ref.sgml
*************** PostgreSQL documentation
*** 302,308 ****
<listitem>
<para>
Specifies options to be passed directly to the
! <command>postgres</command> command.
</para>
<para>
The options should usually be surrounded by single or double
--- 302,309 ----
<listitem>
<para>
Specifies options to be passed directly to the
! <command>postgres</command> command; multiple
! option invocations are appended.
</para>
<para>
The options should usually be surrounded by single or double
diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml
new file mode 100644
index cdfdaa0..845d969
*** a/doc/src/sgml/ref/postgres-ref.sgml
--- b/doc/src/sgml/ref/postgres-ref.sgml
*************** PostgreSQL documentation
*** 288,294 ****
class="parameter">extra-options</replaceable> are passed to
all server processes started by this
<command>postgres</command> process. If the option string contains
! any spaces, the entire string must be quoted.
</para>
<para>
--- 288,295 ----
class="parameter">extra-options</replaceable> are passed to
all server processes started by this
<command>postgres</command> process. If the option string contains
! any spaces, the entire string must be quoted; multiple
! option invocations are appended.
</para>
<para>
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
new file mode 100644
index ad7f36c..a46ca53
*** a/src/bin/pg_ctl/pg_ctl.c
--- b/src/bin/pg_ctl/pg_ctl.c
*************** main(int argc, char **argv)
*** 2184,2190 ****
register_servicename = pg_strdup(optarg);
break;
case 'o':
! post_opts = pg_strdup(optarg);
break;
case 'p':
exec_path = pg_strdup(optarg);
--- 2184,2199 ----
register_servicename = pg_strdup(optarg);
break;
case 'o':
! /* append option? */
! if (!post_opts)
! post_opts = pg_strdup(optarg);
! else
! {
! char *old_post_opts = post_opts;
!
! post_opts = psprintf("%s %s", old_post_opts, optarg);
! free(old_post_opts);
! }
break;
case 'p':
exec_path = pg_strdup(optarg);
On Thursday 21 of August 2014 18:26:37 Bruce Momjian wrote:
On Tue, Mar 4, 2014 at 04:52:56PM +0100, Pavel Raiskup wrote:
RFE: Consider that you want to run pg_upgrade via some script with some
default '-o' option. But then you also want to give the script's user a
chance to specify the old-server's options according user's needs.
Then something like the following is not possible:$ cat script
...
pg_upgrade ... -o 'sth' $PG_UPGRADE_OPT ...
...I know that this problem is still script-able, but the fix should be
innocent and it would simplify things. Thanks for considering,Attached is a patch that makes multiple -o options append their
arguments for pg_upgrade and pg_ctl, and documents this and the append
behavior of postmaster/postgres. This covers all the -o behaviors.
Thanks! Seems to be OK to me, one nit - why you did not go the
append_optiton way (there could be probably better name like arg_cat)?
Because this is just about few lines, it is probably OK from PostgreSQL
policy POV, so "review? ~> review+", thanks again!
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 22, 2014 at 10:52:12AM +0200, Pavel Raiskup wrote:
On Thursday 21 of August 2014 18:26:37 Bruce Momjian wrote:
On Tue, Mar 4, 2014 at 04:52:56PM +0100, Pavel Raiskup wrote:
RFE: Consider that you want to run pg_upgrade via some script with some
default '-o' option. But then you also want to give the script's user a
chance to specify the old-server's options according user's needs.
Then something like the following is not possible:$ cat script
...
pg_upgrade ... -o 'sth' $PG_UPGRADE_OPT ...
...I know that this problem is still script-able, but the fix should be
innocent and it would simplify things. Thanks for considering,Attached is a patch that makes multiple -o options append their
arguments for pg_upgrade and pg_ctl, and documents this and the append
behavior of postmaster/postgres. This covers all the -o behaviors.Thanks! Seems to be OK to me, one nit - why you did not go the
append_optiton way (there could be probably better name like arg_cat)?
Because this is just about few lines, it is probably OK from PostgreSQL
policy POV, so "review? ~> review+", thanks again!
Well, I found append_optiton() to be an extra function that wasn't
necessary --- the psprintf() use was short enough not to need a separate
function.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 22, 2014 at 10:02:11AM -0400, Bruce Momjian wrote:
On Fri, Aug 22, 2014 at 10:52:12AM +0200, Pavel Raiskup wrote:
On Thursday 21 of August 2014 18:26:37 Bruce Momjian wrote:
On Tue, Mar 4, 2014 at 04:52:56PM +0100, Pavel Raiskup wrote:
RFE: Consider that you want to run pg_upgrade via some script with some
default '-o' option. But then you also want to give the script's user a
chance to specify the old-server's options according user's needs.
Then something like the following is not possible:$ cat script
...
pg_upgrade ... -o 'sth' $PG_UPGRADE_OPT ...
...I know that this problem is still script-able, but the fix should be
innocent and it would simplify things. Thanks for considering,Attached is a patch that makes multiple -o options append their
arguments for pg_upgrade and pg_ctl, and documents this and the append
behavior of postmaster/postgres. This covers all the -o behaviors.Thanks! Seems to be OK to me, one nit - why you did not go the
append_optiton way (there could be probably better name like arg_cat)?
Because this is just about few lines, it is probably OK from PostgreSQL
policy POV, so "review? ~> review+", thanks again!Well, I found append_optiton() to be an extra function that wasn't
necessary --- the psprintf() use was short enough not to need a separate
function.
Patch applied; this will appear in PG 9.5.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers