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.

Author: mhagander

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,18 +137,73 @@ 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.
  *
  * The function will take care of running pg_start_backup() and
  * 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);
 

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

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,

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 */

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;