From bd36c10d4413d0e2be4f3bab3a1de1a9736886c6 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 10 Sep 2021 11:50:05 -0400
Subject: [PATCH v5 1/8] 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.

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.

Discussion: http://postgr.es/m/CA+TgmobAczXDRO_Gr2euo_TxgzaH1JxbNxvFx=HYvBinefNH8Q@mail.gmail.com
---
 doc/src/sgml/protocol.sgml            | 68 ++++++++++++--------
 src/backend/replication/basebackup.c  | 33 +++++-----
 src/backend/replication/repl_gram.y   | 93 +++++++++++++++++++++++----
 src/bin/pg_basebackup/pg_basebackup.c | 65 ++++++++++++-------
 src/bin/pg_basebackup/streamutil.c    | 61 ++++++++++++++++++
 src/bin/pg_basebackup/streamutil.h    | 12 ++++
 6 files changed, 254 insertions(+), 78 deletions(-)

diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index a232546b1d..32d1eeabdc 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2517,8 +2517,7 @@ The commands accepted in replication mode are:
   </varlistentry>
 
   <varlistentry id="protocol-replication-base-backup" xreflabel="BASE_BACKUP">
-    <term><literal>BASE_BACKUP</literal> [ <literal>LABEL</literal> <replaceable>'label'</replaceable> ] [ <literal>PROGRESS</literal> ] [ <literal>FAST</literal> ] [ <literal>WAL</literal> ] [ <literal>NOWAIT</literal> ] [ <literal>MAX_RATE</literal> <replaceable>rate</replaceable> ] [ <literal>TABLESPACE_MAP</literal> ] [ <literal>NOVERIFY_CHECKSUMS</literal> ] [ <literal>MANIFEST</literal> <replaceable>manifest_option</replaceable> ] [ <literal>MANIFEST_CHECKSUMS</literal> <replaceable>checksum_algorithm</replaceable> ]
-     <indexterm><primary>BASE_BACKUP</primary></indexterm>
+    <term><literal>BASE_BACKUP</literal> [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ]
     </term>
     <listitem>
      <para>
@@ -2540,52 +2539,55 @@ The commands accepted in replication mode are:
        </varlistentry>
 
        <varlistentry>
-        <term><literal>PROGRESS</literal></term>
+        <term><literal>PROGRESS [ <replaceable class="parameter">boolean</replaceable> ]</literal></term>
         <listitem>
          <para>
-          Request information required to generate a progress report. This will
-          send back an approximate size in the header of each tablespace, which
-          can be used to calculate how far along the stream is done. This is
-          calculated by enumerating all the file sizes once before the transfer
-          is even started, and might as such have a negative impact on the
-          performance.  In particular, it might take longer before the first data
+          If set to true, request information required to generate a progress
+          report. This will send back an approximate size in the header of each
+          tablespace, which can be used to calculate how far along the stream
+          is done. This is calculated by enumerating all the file sizes once
+          before the transfer is even started, and might as such have a
+          negative impact on the performance.  In particular, it might take
+          longer before the first data
           is streamed. Since the database files can change during the backup,
           the size is only approximate and might both grow and shrink between
           the time of approximation and the sending of the actual files.
+          The default is false.
          </para>
         </listitem>
        </varlistentry>
 
        <varlistentry>
-        <term><literal>FAST</literal></term>
+        <term><literal>FAST [ <replaceable class="parameter">boolean</replaceable> ]</literal></term>
         <listitem>
          <para>
-          Request a fast checkpoint.
+          If set to true, a fast checkpoint is requested.
+          The default is false.
          </para>
         </listitem>
        </varlistentry>
 
        <varlistentry>
-        <term><literal>WAL</literal></term>
+        <term><literal>WAL [ <replaceable class="parameter">boolean</replaceable> ]</literal></term>
         <listitem>
          <para>
-          Include the necessary WAL segments in the backup. This will include
-          all the files between start and stop backup in the
+          If set to true, include the necessary WAL segments in the backup.
+          This will include all the files between start and stop backup in the
           <filename>pg_wal</filename> directory of the base directory tar
-          file.
+          file. The default is false.
          </para>
         </listitem>
        </varlistentry>
 
        <varlistentry>
-        <term><literal>NOWAIT</literal></term>
+        <term><literal>WAIT [ <replaceable class="parameter">boolean</replaceable> ]</literal></term>
         <listitem>
          <para>
-          By default, the backup will wait until the last required WAL
+          If set to true, the backup will wait until the last required WAL
           segment has been archived, or emit a warning if log archiving is
-          not enabled. Specifying <literal>NOWAIT</literal> disables both
-          the waiting and the warning, leaving the client responsible for
-          ensuring the required log is available.
+          not enabled. If false, the backup will neither wait nor warn,
+          leaving the client responsible for ensuring the required log is
+          available. The default is true.
          </para>
         </listitem>
        </varlistentry>
@@ -2605,25 +2607,25 @@ The commands accepted in replication mode are:
        </varlistentry>
 
        <varlistentry>
-        <term><literal>TABLESPACE_MAP</literal></term>
+        <term><literal>TABLESPACE_MAP [ <replaceable class="parameter">boolean</replaceable> ]</literal></term>
         <listitem>
          <para>
-          Include information about symbolic links present in the directory
-          <filename>pg_tblspc</filename> in a file named
+          If true, include information about symbolic links present in the
+          directory <filename>pg_tblspc</filename> in a file named
           <filename>tablespace_map</filename>. The tablespace map file includes
           each symbolic link name as it exists in the directory
           <filename>pg_tblspc/</filename> and the full path of that symbolic link.
+          The default is false.
          </para>
         </listitem>
        </varlistentry>
 
        <varlistentry>
-        <term><literal>NOVERIFY_CHECKSUMS</literal></term>
+        <term><literal>VERIFY_CHECKSUMS [ <replaceable class="parameter">boolean</replaceable> ]</literal></term>
         <listitem>
          <para>
-          By default, checksums are verified during a base backup if they are
-          enabled. Specifying <literal>NOVERIFY_CHECKSUMS</literal> disables
-          this verification.
+          If true, checksums are verified during a base backup if they are
+          enabled. If false, this is skipped. The default is true.
          </para>
         </listitem>
        </varlistentry>
@@ -2708,6 +2710,7 @@ The commands accepted in replication mode are:
        </varlistentry>
       </variablelist>
      </para>
+
      <para>
       After the second regular result set, one or more CopyOutResponse results
       will be sent, one for the main data directory and one for each additional tablespace other
@@ -2788,6 +2791,17 @@ The commands accepted in replication mode are:
      </para>
     </listitem>
   </varlistentry>
+
+  <varlistentry>
+    <term><literal>BASE_BACKUP</literal> [ <literal>LABEL</literal> <replaceable>'label'</replaceable> ] [ <literal>PROGRESS</literal> ] [ <literal>FAST</literal> ] [ <literal>WAL</literal> ] [ <literal>NOWAIT</literal> ] [ <literal>MAX_RATE</literal> <replaceable>rate</replaceable> ] [ <literal>TABLESPACE_MAP</literal> ] [ <literal>NOVERIFY_CHECKSUMS</literal> ] [ <literal>MANIFEST</literal> <replaceable>manifest_option</replaceable> ] [ <literal>MANIFEST_CHECKSUMS</literal> <replaceable>checksum_algorithm</replaceable> ]
+    </term>
+    <listitem>
+     <para>
+      For compatibility with older releases, this alternative syntax for
+      the <literal>BASE_BACKUP</literal> command is still supported.
+     </para>
+    </listitem>
+  </varlistentry>
 </variablelist>
 
 </para>
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index e09108d0ec..b0b52d3b1a 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -19,6 +19,7 @@
 #include "access/xlog_internal.h"	/* for pg_start/stop_backup */
 #include "catalog/pg_type.h"
 #include "common/file_perm.h"
+#include "commands/defrem.h"
 #include "commands/progress.h"
 #include "lib/stringinfo.h"
 #include "libpq/libpq.h"
@@ -787,7 +788,7 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->label = strVal(defel->arg);
+			opt->label = defGetString(defel);
 			o_label = true;
 		}
 		else if (strcmp(defel->defname, "progress") == 0)
@@ -796,7 +797,7 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->progress = true;
+			opt->progress = defGetBoolean(defel);
 			o_progress = true;
 		}
 		else if (strcmp(defel->defname, "fast") == 0)
@@ -805,16 +806,16 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->fastcheckpoint = true;
+			opt->fastcheckpoint = defGetBoolean(defel);
 			o_fast = true;
 		}
-		else if (strcmp(defel->defname, "nowait") == 0)
+		else if (strcmp(defel->defname, "wait") == 0)
 		{
 			if (o_nowait)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->nowait = true;
+			opt->nowait = !defGetBoolean(defel);
 			o_nowait = true;
 		}
 		else if (strcmp(defel->defname, "wal") == 0)
@@ -823,19 +824,19 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->includewal = true;
+			opt->includewal = defGetBoolean(defel);
 			o_wal = true;
 		}
 		else if (strcmp(defel->defname, "max_rate") == 0)
 		{
-			long		maxrate;
+			int64		maxrate;
 
 			if (o_maxrate)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
 
-			maxrate = intVal(defel->arg);
+			maxrate = defGetInt64(defel);
 			if (maxrate < MAX_RATE_LOWER || maxrate > MAX_RATE_UPPER)
 				ereport(ERROR,
 						(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
@@ -851,21 +852,21 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			opt->sendtblspcmapfile = true;
+			opt->sendtblspcmapfile = defGetBoolean(defel);
 			o_tablespace_map = true;
 		}
-		else if (strcmp(defel->defname, "noverify_checksums") == 0)
+		else if (strcmp(defel->defname, "verify_checksums") == 0)
 		{
 			if (o_noverify_checksums)
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("duplicate option \"%s\"", defel->defname)));
-			noverify_checksums = true;
+			noverify_checksums = !defGetBoolean(defel);
 			o_noverify_checksums = true;
 		}
 		else if (strcmp(defel->defname, "manifest") == 0)
 		{
-			char	   *optval = strVal(defel->arg);
+			char	   *optval = defGetString(defel);
 			bool		manifest_bool;
 
 			if (o_manifest)
@@ -890,7 +891,7 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 		}
 		else if (strcmp(defel->defname, "manifest_checksums") == 0)
 		{
-			char	   *optval = strVal(defel->arg);
+			char	   *optval = defGetString(defel);
 
 			if (o_manifest_checksums)
 				ereport(ERROR,
@@ -905,8 +906,10 @@ parse_basebackup_options(List *options, basebackup_options *opt)
 			o_manifest_checksums = true;
 		}
 		else
-			elog(ERROR, "option \"%s\" not recognized",
-				 defel->defname);
+			ereport(ERROR,
+					errcode(ERRCODE_SYNTAX_ERROR),
+					errmsg("option \"%s\" not recognized",
+						   defel->defname));
 	}
 	if (opt->label == NULL)
 		opt->label = "base backup";
diff --git a/src/backend/replication/repl_gram.y b/src/backend/replication/repl_gram.y
index e1e8ec29cc..ce51a5e322 100644
--- a/src/backend/replication/repl_gram.y
+++ b/src/backend/replication/repl_gram.y
@@ -95,13 +95,13 @@ static SQLCmd *make_sqlcmd(void);
 %type <node>	base_backup start_replication start_logical_replication
 				create_replication_slot drop_replication_slot identify_system
 				timeline_history show sql_cmd
-%type <list>	base_backup_opt_list
-%type <defelt>	base_backup_opt
+%type <list>	base_backup_legacy_opt_list generic_option_list
+%type <defelt>	base_backup_legacy_opt generic_option
 %type <uintval>	opt_timeline
 %type <list>	plugin_options plugin_opt_list
 %type <defelt>	plugin_opt_elem
 %type <node>	plugin_opt_arg
-%type <str>		opt_slot var_name
+%type <str>		opt_slot var_name ident_or_keyword
 %type <boolval>	opt_temporary
 %type <list>	create_slot_opt_list
 %type <defelt>	create_slot_opt
@@ -157,12 +157,24 @@ var_name:	IDENT	{ $$ = $1; }
 		;
 
 /*
+ * BASE_BACKUP ( option [ 'value' ] [, ...] )
+ *
+ * We also still support the legacy syntax:
+ *
  * BASE_BACKUP [LABEL '<label>'] [PROGRESS] [FAST] [WAL] [NOWAIT]
  * [MAX_RATE %d] [TABLESPACE_MAP] [NOVERIFY_CHECKSUMS]
  * [MANIFEST %s] [MANIFEST_CHECKSUMS %s]
+ *
+ * Future options should be supported only using the new syntax.
  */
 base_backup:
-			K_BASE_BACKUP base_backup_opt_list
+			K_BASE_BACKUP '(' generic_option_list ')'
+				{
+					BaseBackupCmd *cmd = makeNode(BaseBackupCmd);
+					cmd->options = $3;
+					$$ = (Node *) cmd;
+				}
+			| K_BASE_BACKUP base_backup_legacy_opt_list
 				{
 					BaseBackupCmd *cmd = makeNode(BaseBackupCmd);
 					cmd->options = $2;
@@ -170,14 +182,14 @@ base_backup:
 				}
 			;
 
-base_backup_opt_list:
-			base_backup_opt_list base_backup_opt
+base_backup_legacy_opt_list:
+			base_backup_legacy_opt_list base_backup_legacy_opt
 				{ $$ = lappend($1, $2); }
 			| /* EMPTY */
 				{ $$ = NIL; }
 			;
 
-base_backup_opt:
+base_backup_legacy_opt:
 			K_LABEL SCONST
 				{
 				  $$ = makeDefElem("label",
@@ -200,8 +212,8 @@ base_backup_opt:
 				}
 			| K_NOWAIT
 				{
-				  $$ = makeDefElem("nowait",
-								   (Node *)makeInteger(true), -1);
+				  $$ = makeDefElem("wait",
+								   (Node *)makeInteger(false), -1);
 				}
 			| K_MAX_RATE UCONST
 				{
@@ -215,8 +227,8 @@ base_backup_opt:
 				}
 			| K_NOVERIFY_CHECKSUMS
 				{
-				  $$ = makeDefElem("noverify_checksums",
-								   (Node *)makeInteger(true), -1);
+				  $$ = makeDefElem("verify_checksums",
+								   (Node *)makeInteger(false), -1);
 				}
 			| K_MANIFEST SCONST
 				{
@@ -422,6 +434,65 @@ plugin_opt_arg:
 sql_cmd:
 			IDENT							{ $$ = (Node *) make_sqlcmd(); }
 		;
+
+generic_option_list:
+			generic_option_list ',' generic_option
+				{ $$ = lappend($1, $3); }
+			| generic_option
+				{ $$ = list_make1($1); }
+			;
+
+generic_option:
+			ident_or_keyword
+				{
+					$$ = makeDefElem($1, NULL, -1);
+				}
+			| ident_or_keyword IDENT
+				{
+					$$ = makeDefElem($1, (Node *) makeString($2), -1);
+				}
+			| ident_or_keyword SCONST
+				{
+					$$ = makeDefElem($1, (Node *) makeString($2), -1);
+				}
+			| ident_or_keyword UCONST
+				{
+					$$ = makeDefElem($1, (Node *) makeInteger($2), -1);
+				}
+			;
+
+ident_or_keyword:
+			IDENT							{ $$ = $1; }
+			| K_BASE_BACKUP					{ $$ = "base_backup"; }
+			| K_IDENTIFY_SYSTEM				{ $$ = "identify_system"; }
+			| K_SHOW						{ $$ = "show"; }
+			| K_START_REPLICATION			{ $$ = "start_replication"; }
+			| K_CREATE_REPLICATION_SLOT	{ $$ = "create_replication_slot"; }
+			| K_DROP_REPLICATION_SLOT		{ $$ = "drop_replication_slot"; }
+			| K_TIMELINE_HISTORY			{ $$ = "timeline_history"; }
+			| K_LABEL						{ $$ = "label"; }
+			| K_PROGRESS					{ $$ = "progress"; }
+			| K_FAST						{ $$ = "fast"; }
+			| K_WAIT						{ $$ = "wait"; }
+			| K_NOWAIT						{ $$ = "nowait"; }
+			| K_MAX_RATE					{ $$ = "max_rate"; }
+			| K_WAL							{ $$ = "wal"; }
+			| K_TABLESPACE_MAP				{ $$ = "tablespace_map"; }
+			| K_NOVERIFY_CHECKSUMS			{ $$ = "noverify_checksums"; }
+			| K_TIMELINE					{ $$ = "timeline"; }
+			| K_PHYSICAL					{ $$ = "physical"; }
+			| K_LOGICAL						{ $$ = "logical"; }
+			| K_SLOT						{ $$ = "slot"; }
+			| K_RESERVE_WAL					{ $$ = "reserve_wal"; }
+			| K_TEMPORARY					{ $$ = "temporary"; }
+			| K_TWO_PHASE					{ $$ = "two_phase"; }
+			| K_EXPORT_SNAPSHOT				{ $$ = "export_snapshot"; }
+			| K_NOEXPORT_SNAPSHOT			{ $$ = "noexport_snapshot"; }
+			| K_USE_SNAPSHOT				{ $$ = "use_snapshot"; }
+			| K_MANIFEST					{ $$ = "manifest"; }
+			| K_MANIFEST_CHECKSUMS			{ $$ = "manifest_checksums"; }
+		;
+
 %%
 
 static SQLCmd *
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 7296eb97d0..0c8be22558 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1809,10 +1809,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];
@@ -1821,8 +1817,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
@@ -1840,6 +1839,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
@@ -1870,20 +1871,42 @@ 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)
+		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)
@@ -1898,18 +1921,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 f5b3b476e5..d782b81adc 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 504803b976..65135c79e0 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,
-- 
2.24.3 (Apple Git-128)

