Make walsender options order-independent

While doing this, also move base backup options into
a struct instead of increasing the number of parameters
to multiple functions for each new option.

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 943d80470bf1ff5a7adf609126a66e13ed0475d6..73edcf2dc3e57017b850931bd21b7a64e643ae73 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -32,6 +32,14 @@
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 
+typedef struct
+{
+   const char *label;
+   bool        progress;
+   bool        fastcheckpoint;
+}  basebackup_options;
+
+
 static int64 sendDir(char *path, int basepathlen, bool sizeonly);
 static void sendFile(char *path, int basepathlen, struct stat * statbuf);
 static void _tarWriteHeader(char *filename, char *linktarget,
@@ -40,7 +48,8 @@ static void send_int8_string(StringInfoData *buf, int64 intval);
 static void SendBackupHeader(List *tablespaces);
 static void SendBackupDirectory(char *location, char *spcoid);
 static void base_backup_cleanup(int code, Datum arg);
-static void perform_base_backup(const char *backup_label, bool progress, DIR *tblspcdir, bool fastcheckpoint);
+static void perform_base_backup(basebackup_options *opt, DIR *tblspcdir);
+static void parse_basebackup_options(List *options, basebackup_options *opt);
 
 typedef struct
 {
@@ -67,9 +76,9 @@ base_backup_cleanup(int code, Datum arg)
  * clobbered by longjmp" from stupider versions of gcc.
  */
 static void
-perform_base_backup(const char *backup_label, bool progress, DIR *tblspcdir, bool fastcheckpoint)
+perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
 {
-   do_pg_start_backup(backup_label, fastcheckpoint);
+   do_pg_start_backup(opt->label, opt->fastcheckpoint);
 
    PG_ENSURE_ERROR_CLEANUP(base_backup_cleanup, (Datum) 0);
    {
@@ -81,7 +90,7 @@ perform_base_backup(const char *backup_label, bool progress, DIR *tblspcdir, boo
 
        /* Add a node for the base directory */
        ti = palloc0(sizeof(tablespaceinfo));
-       ti->size = progress ? sendDir(".", 1, true) : -1;
+       ti->size = opt->progress ? sendDir(".", 1, true) : -1;
        tablespaces = lappend(tablespaces, ti);
 
        /* Collect information about all tablespaces */
@@ -107,7 +116,7 @@ perform_base_backup(const char *backup_label, bool progress, DIR *tblspcdir, boo
            ti = palloc(sizeof(tablespaceinfo));
            ti->oid = pstrdup(de->d_name);
            ti->path = pstrdup(linkpath);
-           ti->size = progress ? sendDir(linkpath, strlen(linkpath), true) : -1;
+           ti->size = opt->progress ? sendDir(linkpath, strlen(linkpath), true) : -1;
            tablespaces = lappend(tablespaces, ti);
        }
 
@@ -128,6 +137,58 @@ perform_base_backup(const char *backup_label, bool progress, DIR *tblspcdir, boo
    do_pg_stop_backup();
 }
 
+/*
+ * Parse the base backup options passed down by the parser
+ */
+static void
+parse_basebackup_options(List *options, basebackup_options *opt)
+{
+   ListCell   *lopt;
+   bool        o_label = false;
+   bool        o_progress = false;
+   bool        o_fast = false;
+
+   MemSet(opt, 0, sizeof(opt));
+   foreach(lopt, options)
+   {
+       DefElem    *defel = (DefElem *) lfirst(lopt);
+
+       if (strcmp(defel->defname, "label") == 0)
+       {
+           if (o_label)
+               ereport(ERROR,
+                       (errcode(ERRCODE_SYNTAX_ERROR),
+                        errmsg("duplicate option \"%s\"", defel->defname)));
+           opt->label = strVal(defel->arg);
+           o_label = true;
+       }
+       else if (strcmp(defel->defname, "progress") == 0)
+       {
+           if (o_progress)
+               ereport(ERROR,
+                       (errcode(ERRCODE_SYNTAX_ERROR),
+                        errmsg("duplicate option \"%s\"", defel->defname)));
+           opt->progress = true;
+           o_progress = true;
+       }
+       else if (strcmp(defel->defname, "fast") == 0)
+       {
+           if (o_fast)
+               ereport(ERROR,
+                       (errcode(ERRCODE_SYNTAX_ERROR),
+                        errmsg("duplicate option \"%s\"", defel->defname)));
+           opt->fastcheckpoint = true;
+           o_fast = true;
+       }
+       else
+           elog(ERROR, "option \"%s\" not recognized",
+                defel->defname);
+   }
+   if (opt->label == NULL)
+       opt->label = "base backup";
+}
+
+
 /*
  * SendBaseBackup() - send a complete base backup.
  *
@@ -135,11 +196,14 @@ perform_base_backup(const char *backup_label, bool progress, DIR *tblspcdir, boo
  * pg_stop_backup() for the user.
  */
 void
-SendBaseBackup(const char *backup_label, bool progress, bool fastcheckpoint)
+SendBaseBackup(BaseBackupCmd *cmd)
 {
    DIR        *dir;
    MemoryContext backup_context;
    MemoryContext old_context;
+   basebackup_options opt;
+
+   parse_basebackup_options(cmd->options, &opt);
 
    backup_context = AllocSetContextCreate(CurrentMemoryContext,
                                           "Streaming base backup context",
@@ -150,15 +214,12 @@ SendBaseBackup(const char *backup_label, bool progress, bool fastcheckpoint)
 
    WalSndSetState(WALSNDSTATE_BACKUP);
 
-   if (backup_label == NULL)
-       backup_label = "base backup";
-
    if (update_process_title)
    {
        char        activitymsg[50];
 
        snprintf(activitymsg, sizeof(activitymsg), "sending backup \"%s\"",
-                backup_label);
+                opt.label);
        set_ps_display(activitymsg, false);
    }
 
@@ -168,7 +229,7 @@ SendBaseBackup(const char *backup_label, bool progress, bool fastcheckpoint)
        ereport(ERROR,
                (errmsg("unable to open directory pg_tblspc: %m")));
 
-   perform_base_backup(backup_label, progress, dir, fastcheckpoint);
+   perform_base_backup(&opt, dir);
 
    FreeDir(dir);
 

diff --git a/src/backend/replication/repl_gram.y b/src/backend/replication/repl_gram.y
index e4f4c4742f68783c964c0da8f8450819935ca9ac..879a0bd7db0d9248027cd5869ccc34567185301f 100644 (file)
--- a/src/backend/replication/repl_gram.y
+++ b/src/backend/replication/repl_gram.y
@@ -15,6 +15,8 @@
 
 #include "postgres.h"
 
+#include "nodes/makefuncs.h"
+#include "nodes/parsenodes.h"
 #include "replication/replnodes.h"
 #include "replication/walsender.h"
 
@@ -55,6 +57,8 @@ Node *replication_parse_result;
 
        XLogRecPtr              recptr;
        Node                    *node;
+       List                    *list;
+       DefElem                 *defelt;
 }
 
 /* Non-keyword tokens */
@@ -71,9 +75,8 @@ Node *replication_parse_result;
 
 %type <node>   command
 %type <node>   base_backup start_replication identify_system
-%type <boolval>    opt_progress opt_fast
-%type <str>     opt_label
-
+%type <list>   base_backup_opt_list
+%type <defelt> base_backup_opt
 %%
 
 firstcmd: command opt_semicolon
@@ -106,28 +109,34 @@ identify_system:
  * BASE_BACKUP [LABEL <label>] [PROGRESS] [FAST]
  */
 base_backup:
-           K_BASE_BACKUP opt_label opt_progress opt_fast
+           K_BASE_BACKUP base_backup_opt_list
                {
                    BaseBackupCmd *cmd = (BaseBackupCmd *) makeNode(BaseBackupCmd);
-
-                   cmd->label = $2;
-                   cmd->progress = $3;
-                   cmd->fastcheckpoint = $4;
-
+                   cmd->options = $2;
                    $$ = (Node *) cmd;
                }
            ;
 
-opt_label: K_LABEL SCONST { $$ = $2; }
-           | /* EMPTY */       { $$ = NULL; }
-           ;
+base_backup_opt_list: base_backup_opt_list base_backup_opt { $$ = lappend($1, $2); }
+           | /* EMPTY */           { $$ = NIL; }
+
+base_backup_opt:
+           K_LABEL SCONST
+               {
+                 $$ = makeDefElem("label",
+                          (Node *)makeString($2));
+               }
+           | K_PROGRESS
+               {
+                 $$ = makeDefElem("progress",
+                          (Node *)makeInteger(TRUE));
+               }
+           | K_FAST
+               {
+                 $$ = makeDefElem("fast",
+                          (Node *)makeInteger(TRUE));
+               }
 
-opt_progress: K_PROGRESS       { $$ = true; }
-           | /* EMPTY */       { $$ = false; }
-           ;
-opt_fast: K_FAST       { $$ = true; }
-           | /* EMPTY */       { $$ = false; }
-           ;
 
 /*
  * START_REPLICATION %X/%X

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 8a6392345697035028ba2c08165e050ae3d5330b..44efa9fc25e8d37a94e2d7e72264c0794a96a211 100644 (file)
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -399,17 +399,13 @@ HandleReplicationCommand(const char *cmd_string)
            break;
 
        case T_BaseBackupCmd:
-           {
-               BaseBackupCmd *cmd = (BaseBackupCmd *) cmd_node;
-
-               SendBaseBackup(cmd->label, cmd->progress, cmd->fastcheckpoint);
+           SendBaseBackup((BaseBackupCmd *) cmd_node);
 
-               /* Send CommandComplete and ReadyForQuery messages */
-               EndCommand("SELECT", DestRemote);
-               ReadyForQuery(DestRemote);
-               /* ReadyForQuery did pq_flush for us */
-               break;
-           }
+           /* Send CommandComplete and ReadyForQuery messages */
+           EndCommand("SELECT", DestRemote);
+           ReadyForQuery(DestRemote);
+           /* ReadyForQuery did pq_flush for us */
+           break;
 
        default:
            ereport(FATAL,

diff --git a/src/include/replication/basebackup.h b/src/include/replication/basebackup.h
index 80f814b2e7c6e6b11d4c5a1ddf815cb831fc3ed5..af6a36621f247942d3915b7f0de358ce493137f8 100644 (file)
--- a/src/include/replication/basebackup.h
+++ b/src/include/replication/basebackup.h
@@ -12,6 +12,8 @@
 #ifndef _BASEBACKUP_H
 #define _BASEBACKUP_H
 
-extern void SendBaseBackup(const char *backup_label, bool progress, bool fastcheckpoint);
+#include "replication/replnodes.h"
+
+extern void SendBaseBackup(BaseBackupCmd *cmd);
 
 #endif   /* _BASEBACKUP_H */

diff --git a/src/include/replication/replnodes.h b/src/include/replication/replnodes.h
index fc814146baf895e7674cb51996143b4fc4b46085..6fc037580fa92a7ea455be2433378fda49b5cef4 100644 (file)
--- a/src/include/replication/replnodes.h
+++ b/src/include/replication/replnodes.h
@@ -45,9 +45,7 @@ typedef struct IdentifySystemCmd
 typedef struct BaseBackupCmd
 {
    NodeTag     type;
-   char       *label;
-   bool        progress;
-   bool        fastcheckpoint;
+   List       *options;
 }  BaseBackupCmd;