Flexible options for BASE_BACKUP.

Previously, BASE_BACKUP used an entirely hard-coded syntax, but that's
hard to extend. Instead, adopt the same kind of syntax we've used for
SQL commands such as VACUUM, ANALYZE, COPY, and EXPLAIN, where it's
not necessary for all of the option names to be parser keywords.

In the new syntax, most of the options now take an optional Boolean
argument. To match our practice in other in places, the options which
the old syntax called NOWAIT and NOVERIFY_CHECKSUMS options are in the
new syntax called WAIT and VERIFY_CHECKUMS, and the default value is
false. In the new syntax, the FAST option has been replaced by a
CHECKSUM option whose value may be 'fast' or 'spread'.

This commit does not remove support for the old syntax. It just adds
the new one as an additional option, and makes pg_basebackup prefer
the new syntax when the server is new enough to support it.

Patch by me, reviewed and tested by Fabien Coelho, Sergei Kornilov,
Fujii Masao, and Tushar Ahuja.

Discussion: http://postgr.es/m/CA+TgmobAczXDRO_Gr2euo_TxgzaH1JxbNxvFx=HYvBinefNH8Q@mail.gmail.com
Discussion: http://postgr.es/m/CA+TgmoZGwR=ZVWFeecncubEyPdwghnvfkkdBe9BLccLSiqdf9Q@mail.gmail.com

3 files changed, 119 insertions, 25 deletions

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 669aa207a3c..27ee6394cfa 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1804,10 +1804,6 @@ BaseBackup(void)
 	TimeLineID	latesttli;
 	TimeLineID	starttli;
 	char	   *basebkp;
-	char		escaped_label[MAXPGPATH];
-	char	   *maxrate_clause = NULL;
-	char	   *manifest_clause = NULL;
-	char	   *manifest_checksums_clause = "";
 	int			i;
 	char		xlogstart[64];
 	char		xlogend[64];
@@ -1816,8 +1812,11 @@ BaseBackup(void)
 	int			serverVersion,
 				serverMajor;
 	int			writing_to_stdout;
+	bool		use_new_option_syntax = false;
+	PQExpBufferData buf;
 
 	Assert(conn != NULL);
+	initPQExpBuffer(&buf);
 
 	/*
 	 * Check server version. BASE_BACKUP command was introduced in 9.1, so we
@@ -1835,6 +1834,8 @@ BaseBackup(void)
 					 serverver ? serverver : "'unknown'");
 		exit(1);
 	}
+	if (serverMajor >= 1500)
+		use_new_option_syntax = true;
 
 	/*
 	 * If WAL streaming was requested, also check that the server is new
@@ -1865,20 +1866,48 @@ BaseBackup(void)
 	/*
 	 * Start the actual backup
 	 */
-	PQescapeStringConn(conn, escaped_label, label, sizeof(escaped_label), &i);
-
+	AppendStringCommandOption(&buf, use_new_option_syntax, "LABEL", label);
+	if (estimatesize)
+		AppendPlainCommandOption(&buf, use_new_option_syntax, "PROGRESS");
+	if (includewal == FETCH_WAL)
+		AppendPlainCommandOption(&buf, use_new_option_syntax, "WAL");
+	if (fastcheckpoint)
+	{
+		if (use_new_option_syntax)
+			AppendStringCommandOption(&buf, use_new_option_syntax,
+									  "CHECKPOINT", "fast");
+		else
+			AppendPlainCommandOption(&buf, use_new_option_syntax, "FAST");
+	}
+	if (includewal != NO_WAL)
+	{
+		if (use_new_option_syntax)
+			AppendIntegerCommandOption(&buf, use_new_option_syntax, "WAIT", 0);
+		else
+			AppendPlainCommandOption(&buf, use_new_option_syntax, "NOWAIT");
+	}
 	if (maxrate > 0)
-		maxrate_clause = psprintf("MAX_RATE %u", maxrate);
+		AppendIntegerCommandOption(&buf, use_new_option_syntax, "MAX_RATE",
+									  maxrate);
+	if (format == 't')
+		AppendPlainCommandOption(&buf, use_new_option_syntax, "TABLESPACE_MAP");
+	if (!verify_checksums)
+	{
+		if (use_new_option_syntax)
+			AppendIntegerCommandOption(&buf, use_new_option_syntax,
+										  "VERIFY_CHECKSUMS", 0);
+		else
+			AppendPlainCommandOption(&buf, use_new_option_syntax,
+										"NOVERIFY_CHECKSUMS");
+	}
 
 	if (manifest)
 	{
-		if (manifest_force_encode)
-			manifest_clause = "MANIFEST 'force-encode'";
-		else
-			manifest_clause = "MANIFEST 'yes'";
+		AppendStringCommandOption(&buf, use_new_option_syntax, "MANIFEST",
+									 manifest_force_encode ? "force-encode" : "yes");
 		if (manifest_checksums != NULL)
-			manifest_checksums_clause = psprintf("MANIFEST_CHECKSUMS '%s'",
-												 manifest_checksums);
+			AppendStringCommandOption(&buf, use_new_option_syntax,
+										 "MANIFEST_CHECKSUMS", manifest_checksums);
 	}
 
 	if (verbose)
@@ -1893,18 +1922,10 @@ BaseBackup(void)
 			fprintf(stderr, "\n");
 	}
 
-	basebkp =
-		psprintf("BASE_BACKUP LABEL '%s' %s %s %s %s %s %s %s %s %s",
-				 escaped_label,
-				 estimatesize ? "PROGRESS" : "",
-				 includewal == FETCH_WAL ? "WAL" : "",
-				 fastcheckpoint ? "FAST" : "",
-				 includewal == NO_WAL ? "" : "NOWAIT",
-				 maxrate_clause ? maxrate_clause : "",
-				 format == 't' ? "TABLESPACE_MAP" : "",
-				 verify_checksums ? "" : "NOVERIFY_CHECKSUMS",
-				 manifest_clause ? manifest_clause : "",
-				 manifest_checksums_clause);
+	if (use_new_option_syntax && buf.len > 0)
+		basebkp = psprintf("BASE_BACKUP (%s)", buf.data);
+	else
+		basebkp = psprintf("BASE_BACKUP %s", buf.data);
 
 	if (PQsendQuery(conn, basebkp) == 0)
 	{
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index f5b3b476e52..d782b81adc6 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -603,6 +603,67 @@ DropReplicationSlot(PGconn *conn, const char *slot_name)
 	return true;
 }
 
+/*
+ * Append a "plain" option - one with no value - to a server command that
+ * is being constructed.
+ *
+ * In the old syntax, all options were parser keywords, so you could just
+ * write things like SOME_COMMAND OPTION1 OPTION2 'opt2value' OPTION3 42. The
+ * new syntax uses a comma-separated list surrounded by parentheses, so the
+ * equivalent is SOME_COMMAND (OPTION1, OPTION2 'optvalue', OPTION3 42).
+ */
+void
+AppendPlainCommandOption(PQExpBuffer buf, bool use_new_option_syntax,
+						 char *option_name)
+{
+	if (buf->len > 0 && buf->data[buf->len - 1] != '(')
+	{
+		if (use_new_option_syntax)
+			appendPQExpBufferStr(buf, ", ");
+		else
+			appendPQExpBufferChar(buf, ' ');
+	}
+
+	appendPQExpBuffer(buf, " %s", option_name);
+}
+
+/*
+ * Append an option with an associated string value to a server command that
+ * is being constructed.
+ *
+ * See comments for AppendPlainCommandOption, above.
+ */
+void
+AppendStringCommandOption(PQExpBuffer buf, bool use_new_option_syntax,
+						  char *option_name, char *option_value)
+{
+	AppendPlainCommandOption(buf, use_new_option_syntax, option_name);
+
+	if (option_value != NULL)
+	{
+		size_t		length = strlen(option_value);
+		char	   *escaped_value = palloc(1 + 2 * length);
+
+		PQescapeStringConn(conn, escaped_value, option_value, length, NULL);
+		appendPQExpBuffer(buf, " '%s'", escaped_value);
+		pfree(escaped_value);
+	}
+}
+
+/*
+ * Append an option with an associated integer value to a server command
+ * is being constructed.
+ *
+ * See comments for AppendPlainCommandOption, above.
+ */
+void
+AppendIntegerCommandOption(PQExpBuffer buf, bool use_new_option_syntax,
+						   char *option_name, int32 option_value)
+{
+	AppendPlainCommandOption(buf, use_new_option_syntax, option_name);
+
+	appendPQExpBuffer(buf, " %d", option_value);
+}
 
 /*
  * Frontend version of GetCurrentTimestamp(), since we are not linked with
diff --git a/src/bin/pg_basebackup/streamutil.h b/src/bin/pg_basebackup/streamutil.h
index 504803b9763..65135c79e08 100644
--- a/src/bin/pg_basebackup/streamutil.h
+++ b/src/bin/pg_basebackup/streamutil.h
@@ -15,6 +15,7 @@
 #include "access/xlogdefs.h"
 #include "datatype/timestamp.h"
 #include "libpq-fe.h"
+#include "pqexpbuffer.h"
 
 extern const char *progname;
 extern char *connection_string;
@@ -40,6 +41,17 @@ extern bool RunIdentifySystem(PGconn *conn, char **sysid,
 							  TimeLineID *starttli,
 							  XLogRecPtr *startpos,
 							  char **db_name);
+
+extern void AppendPlainCommandOption(PQExpBuffer buf,
+									 bool use_new_option_syntax,
+									 char *option_value);
+extern void AppendStringCommandOption(PQExpBuffer buf,
+									  bool use_new_option_syntax,
+									  char *option_name, char *option_value);
+extern void AppendIntegerCommandOption(PQExpBuffer buf,
+									   bool use_new_option_syntax,
+									   char *option_name, int32 option_value);
+
 extern bool RetrieveWalSegSize(PGconn *conn);
 extern TimestampTz feGetCurrentTimestamp(void);
 extern void feTimestampDifference(TimestampTz start_time, TimestampTz stop_time,