v3-0001-Flexible-options-for-BASE_BACKUP-and-CREATE_REPLI.patch

From 51e9982701eee197a9c6ae54830e4d6683f8a032 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Thu, 1 Jul 2021 11:33:32 -0400
Subject: [PATCH v3 1/7] Flexible options for BASE_BACKUP and
 CREATE_REPLICATION_SLOT.

Previously, these replication commands 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.

v2: Fix compile error.
v3: Fix inverted test, as reported by Tushar Ahuja.
v4: Adjustments for v15.
---
 src/backend/replication/basebackup.c          |  33 ++---
 .../libpqwalreceiver/libpqwalreceiver.c       |   8 +-
 src/backend/replication/repl_gram.y           | 116 +++++++++++++++---
 src/backend/replication/walsender.c           |  17 +--
 src/bin/pg_basebackup/pg_basebackup.c         |  65 ++++++----
 src/bin/pg_basebackup/streamutil.c            | 102 +++++++++++++--
 src/bin/pg_basebackup/streamutil.h            |  12 ++
 7 files changed, 273 insertions(+), 80 deletions(-)

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/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 6eaa84a031..37a0d1c79a 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -871,19 +871,19 @@ libpqrcv_create_slot(WalReceiverConn *conn, const char *slotname,
 		switch (snapshot_action)
 		{
 			case CRS_EXPORT_SNAPSHOT:
-				appendStringInfoString(&cmd, " EXPORT_SNAPSHOT");
+				appendStringInfoString(&cmd, " (EXPORT_SNAPSHOT TRUE)");
 				break;
 			case CRS_NOEXPORT_SNAPSHOT:
-				appendStringInfoString(&cmd, " NOEXPORT_SNAPSHOT");
+				appendStringInfoString(&cmd, " (EXPORT_SNAPSHOT FALSE)");
 				break;
 			case CRS_USE_SNAPSHOT:
-				appendStringInfoString(&cmd, " USE_SNAPSHOT");
+				appendStringInfoString(&cmd, " (USE_SNAPSHOT)");
 				break;
 		}
 	}
 	else
 	{
-		appendStringInfoString(&cmd, " PHYSICAL RESERVE_WAL");
+		appendStringInfoString(&cmd, " PHYSICAL (RESERVE_WAL)");
 	}
 
 	res = libpqrcv_PQexec(conn->streamConn, cmd.data);
diff --git a/src/backend/replication/repl_gram.y b/src/backend/replication/repl_gram.y
index e1e8ec29cc..69e990cda3 100644
--- a/src/backend/replication/repl_gram.y
+++ b/src/backend/replication/repl_gram.y
@@ -95,16 +95,16 @@ 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
+%type <list>	create_slot_options create_slot_legacy_opt_list
+%type <defelt>	create_slot_legacy_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
 				{
@@ -231,8 +243,8 @@ base_backup_opt:
 			;
 
 create_replication_slot:
-			/* CREATE_REPLICATION_SLOT slot TEMPORARY PHYSICAL RESERVE_WAL */
-			K_CREATE_REPLICATION_SLOT IDENT opt_temporary K_PHYSICAL create_slot_opt_list
+			/* CREATE_REPLICATION_SLOT slot TEMPORARY PHYSICAL [options] */
+			K_CREATE_REPLICATION_SLOT IDENT opt_temporary K_PHYSICAL create_slot_options
 				{
 					CreateReplicationSlotCmd *cmd;
 					cmd = makeNode(CreateReplicationSlotCmd);
@@ -242,8 +254,8 @@ create_replication_slot:
 					cmd->options = $5;
 					$$ = (Node *) cmd;
 				}
-			/* CREATE_REPLICATION_SLOT slot TEMPORARY LOGICAL plugin */
-			| K_CREATE_REPLICATION_SLOT IDENT opt_temporary K_LOGICAL IDENT create_slot_opt_list
+			/* CREATE_REPLICATION_SLOT slot TEMPORARY LOGICAL plugin [options] */
+			| K_CREATE_REPLICATION_SLOT IDENT opt_temporary K_LOGICAL IDENT create_slot_options
 				{
 					CreateReplicationSlotCmd *cmd;
 					cmd = makeNode(CreateReplicationSlotCmd);
@@ -256,14 +268,19 @@ create_replication_slot:
 				}
 			;
 
-create_slot_opt_list:
-			create_slot_opt_list create_slot_opt
+create_slot_options:
+			'(' generic_option_list ')'			{ $$ = $2; }
+			| create_slot_legacy_opt_list		{ $$ = $1; }
+			;
+
+create_slot_legacy_opt_list:
+			create_slot_legacy_opt_list create_slot_legacy_opt
 				{ $$ = lappend($1, $2); }
 			| /* EMPTY */
 				{ $$ = NIL; }
 			;
 
-create_slot_opt:
+create_slot_legacy_opt:
 			K_EXPORT_SNAPSHOT
 				{
 				  $$ = makeDefElem("export_snapshot",
@@ -422,6 +439,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/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 92c755f346..93f2d0ece4 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -895,7 +895,8 @@ parseCreateReplSlotOptions(CreateReplicationSlotCmd *cmd,
 						 errmsg("conflicting or redundant options")));
 
 			snapshot_action_given = true;
-			*snapshot_action = CRS_USE_SNAPSHOT;
+			if (defGetBoolean(defel))
+				*snapshot_action = CRS_USE_SNAPSHOT;
 		}
 		else if (strcmp(defel->defname, "reserve_wal") == 0)
 		{
@@ -905,7 +906,7 @@ parseCreateReplSlotOptions(CreateReplicationSlotCmd *cmd,
 						 errmsg("conflicting or redundant options")));
 
 			reserve_wal_given = true;
-			*reserve_wal = true;
+			*reserve_wal = defGetBoolean(defel);
 		}
 		else if (strcmp(defel->defname, "two_phase") == 0)
 		{
@@ -914,7 +915,7 @@ parseCreateReplSlotOptions(CreateReplicationSlotCmd *cmd,
 						(errcode(ERRCODE_SYNTAX_ERROR),
 						 errmsg("conflicting or redundant options")));
 			two_phase_given = true;
-			*two_phase = true;
+			*two_phase = defGetBoolean(defel);
 		}
 		else
 			elog(ERROR, "unrecognized option: %s", defel->defname);
@@ -984,7 +985,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 				ereport(ERROR,
 				/*- translator: %s is a CREATE_REPLICATION_SLOT statement */
 						(errmsg("%s must not be called inside a transaction",
-								"CREATE_REPLICATION_SLOT ... EXPORT_SNAPSHOT")));
+								"CREATE_REPLICATION_SLOT ... (EXPORT_SNAPSHOT)")));
 
 			need_full_snapshot = true;
 		}
@@ -994,25 +995,25 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 				ereport(ERROR,
 				/*- translator: %s is a CREATE_REPLICATION_SLOT statement */
 						(errmsg("%s must be called inside a transaction",
-								"CREATE_REPLICATION_SLOT ... USE_SNAPSHOT")));
+								"CREATE_REPLICATION_SLOT ... (USE_SNAPSHOT)")));
 
 			if (XactIsoLevel != XACT_REPEATABLE_READ)
 				ereport(ERROR,
 				/*- translator: %s is a CREATE_REPLICATION_SLOT statement */
 						(errmsg("%s must be called in REPEATABLE READ isolation mode transaction",
-								"CREATE_REPLICATION_SLOT ... USE_SNAPSHOT")));
+								"CREATE_REPLICATION_SLOT ... (USE_SNAPSHOT)")));
 
 			if (FirstSnapshotSet)
 				ereport(ERROR,
 				/*- translator: %s is a CREATE_REPLICATION_SLOT statement */
 						(errmsg("%s must be called before any query",
-								"CREATE_REPLICATION_SLOT ... USE_SNAPSHOT")));
+								"CREATE_REPLICATION_SLOT ... (USE_SNAPSHOT)")));
 
 			if (IsSubTransaction())
 				ereport(ERROR,
 				/*- translator: %s is a CREATE_REPLICATION_SLOT statement */
 						(errmsg("%s must not be called in a subtransaction",
-								"CREATE_REPLICATION_SLOT ... USE_SNAPSHOT")));
+								"CREATE_REPLICATION_SLOT ... (USE_SNAPSHOT)")));
 
 			need_full_snapshot = true;
 		}
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8bb0acf498..80912a0ea6 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1801,10 +1801,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];
@@ -1813,8 +1809,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
@@ -1832,6 +1831,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
@@ -1862,20 +1863,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)
@@ -1890,18 +1913,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..9232ef77e2 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -490,6 +490,7 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 {
 	PQExpBuffer query;
 	PGresult   *res;
+	bool		use_new_option_syntax = (PQserverVersion(conn) >= 150000);
 
 	query = createPQExpBuffer();
 
@@ -498,27 +499,51 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 	Assert(!(two_phase && is_physical));
 	Assert(slot_name != NULL);
 
-	/* Build query */
+	/* Build base portion of query */
 	appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\"", slot_name);
 	if (is_temporary)
 		appendPQExpBufferStr(query, " TEMPORARY");
 	if (is_physical)
-	{
 		appendPQExpBufferStr(query, " PHYSICAL");
+	else
+		appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
+
+	/* Add any requested options */
+	if (use_new_option_syntax)
+		appendPQExpBufferStr(query, " (");
+	if (is_physical)
+	{
 		if (reserve_wal)
-			appendPQExpBufferStr(query, " RESERVE_WAL");
+			AppendPlainCommandOption(query, use_new_option_syntax,
+									 "RESERVE_WAL");
 	}
 	else
 	{
-		appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
 		if (two_phase && PQserverVersion(conn) >= 150000)
-			appendPQExpBufferStr(query, " TWO_PHASE");
+			AppendPlainCommandOption(query, use_new_option_syntax,
+									 "TWO_PHASE");
 
-		if (PQserverVersion(conn) >= 100000)
-			/* pg_recvlogical doesn't use an exported snapshot, so suppress */
-			appendPQExpBufferStr(query, " NOEXPORT_SNAPSHOT");
+		/* pg_recvlogical doesn't use an exported snapshot, so suppress */
+		if (use_new_option_syntax)
+			AppendIntegerCommandOption(query, use_new_option_syntax,
+									   "EXPORT_SNAPSHOT", 0);
+		else
+			AppendPlainCommandOption(query, use_new_option_syntax,
+									 "NOEXPORT_SNAPSHOT");
+	}
+	if (use_new_option_syntax)
+	{
+		/* Suppress option list if it would be empty, otherwise terminate */
+		if (query->data[query->len - 1] == '(')
+		{
+			query->len -= 2;
+			query->data[query->len] = '\0';
+		}
+		else
+			appendPQExpBufferChar(query, ')');
 	}
 
+	/* Now run the query */
 	res = PQexec(conn, query->data);
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
@@ -603,6 +628,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)