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

From 787068be3c23308b6fe46fc8c731c6d6ef82f485 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Thu, 11 Jun 2020 15:28:39 -0400
Subject: [PATCH v2 01/11] Flexible options for BASE_BACKUP and
 CREATE_REPLICATION_SLOT.

---
 src/backend/replication/basebackup.c          |  33 ++---
 .../libpqwalreceiver/libpqwalreceiver.c       |   8 +-
 src/backend/replication/repl_gram.y           | 115 +++++++++++++++---
 src/backend/replication/walsender.c           |  15 +--
 src/bin/pg_basebackup/pg_basebackup.c         | 108 ++++++++++++----
 src/bin/pg_basebackup/streamutil.c            |  14 ++-
 6 files changed, 220 insertions(+), 73 deletions(-)

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 6064384e32..d43c34e8e9 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"
@@ -777,7 +778,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)
@@ -786,7 +787,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)
@@ -795,16 +796,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)
@@ -813,19 +814,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),
@@ -841,21 +842,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)
@@ -880,7 +881,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,
@@ -895,8 +896,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 e9057230e4..c381e26143 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -828,19 +828,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 f93a0de218..8b2109855d 100644
--- a/src/backend/replication/repl_gram.y
+++ b/src/backend/replication/repl_gram.y
@@ -94,16 +94,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
 
 %%
 
@@ -156,12 +156,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;
@@ -169,14 +181,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",
@@ -199,8 +211,8 @@ base_backup_opt:
 				}
 			| K_NOWAIT
 				{
-				  $$ = makeDefElem("nowait",
-								   (Node *)makeInteger(true), -1);
+				  $$ = makeDefElem("wait",
+								   (Node *)makeInteger(false), -1);
 				}
 			| K_MAX_RATE UCONST
 				{
@@ -214,8 +226,8 @@ base_backup_opt:
 				}
 			| K_NOVERIFY_CHECKSUMS
 				{
-				  $$ = makeDefElem("noverify_checksums",
-								   (Node *)makeInteger(true), -1);
+				  $$ = makeDefElem("verify_checksums",
+								   (Node *)makeInteger(false), -1);
 				}
 			| K_MANIFEST SCONST
 				{
@@ -230,8 +242,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);
@@ -241,8 +253,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);
@@ -255,14 +267,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",
@@ -416,6 +433,64 @@ 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_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 5e2210dd7b..5da45ecca0 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -897,7 +897,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)
 		{
@@ -907,7 +908,7 @@ parseCreateReplSlotOptions(CreateReplicationSlotCmd *cmd,
 						 errmsg("conflicting or redundant options")));
 
 			reserve_wal_given = true;
-			*reserve_wal = true;
+			*reserve_wal = defGetBoolean(defel);
 		}
 		else
 			elog(ERROR, "unrecognized option: %s", defel->defname);
@@ -974,7 +975,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;
 		}
@@ -984,25 +985,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 4f29671d0c..0645e983c6 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1787,6 +1787,49 @@ ReceiveBackupManifestInMemoryChunk(size_t r, char *copybuf,
 	appendPQExpBuffer(buf, copybuf, r);
 }
 
+static void
+AppendBaseBackupPlainOption(StringInfo buf, bool use_new_option_syntax,
+							char *option_name)
+{
+	if (buf->len > 0)
+	{
+		if (use_new_option_syntax)
+			appendStringInfoString(buf, ", ");
+		else
+			appendStringInfoChar(buf, ' ');
+	}
+
+	appendStringInfoString(buf, option_name);
+}
+
+static void
+AppendBaseBackupStringOption(StringInfo buf, bool use_new_option_syntax,
+							 char *option_name, char *option_value)
+{
+	AppendBaseBackupPlainOption(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);
+		appendStringInfoString(buf, " '");
+		appendStringInfoString(buf, escaped_value);
+		appendStringInfoChar(buf, '\'');
+		pfree(escaped_value);
+	}
+}
+
+static void
+AppendBaseBackupIntegerOption(StringInfo buf, bool use_new_option_syntax,
+							  char *option_name, int32 option_value)
+{
+	AppendBaseBackupPlainOption(buf, use_new_option_syntax, option_name);
+
+	appendStringInfo(buf, " %d", option_value);
+}
+
 static void
 BaseBackup(void)
 {
@@ -1795,10 +1838,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];
@@ -1807,8 +1846,11 @@ BaseBackup(void)
 	int			serverVersion,
 				serverMajor;
 	int			writing_to_stdout;
+	bool		use_new_option_syntax = false;
+	StringInfoData	buf;
 
 	Assert(conn != NULL);
+	initStringInfo(&buf);
 
 	/*
 	 * Check server version. BASE_BACKUP command was introduced in 9.1, so we
@@ -1826,6 +1868,8 @@ BaseBackup(void)
 					 serverver ? serverver : "'unknown'");
 		exit(1);
 	}
+	if (serverMajor >= 1400)
+		use_new_option_syntax = true;
 
 	/*
 	 * If WAL streaming was requested, also check that the server is new
@@ -1856,20 +1900,42 @@ BaseBackup(void)
 	/*
 	 * Start the actual backup
 	 */
-	PQescapeStringConn(conn, escaped_label, label, sizeof(escaped_label), &i);
-
+	AppendBaseBackupStringOption(&buf, use_new_option_syntax, "LABEL", label);
+	if (estimatesize)
+		AppendBaseBackupPlainOption(&buf, use_new_option_syntax, "PROGRESS");
+	if (includewal == FETCH_WAL)
+		AppendBaseBackupPlainOption(&buf, use_new_option_syntax, "WAL");
+	if (fastcheckpoint)
+		AppendBaseBackupPlainOption(&buf, use_new_option_syntax, "FAST");
+	if (includewal == NO_WAL)
+	{
+		if (use_new_option_syntax)
+			AppendBaseBackupIntegerOption(&buf, use_new_option_syntax, "WAIT", 0);
+		else
+			AppendBaseBackupPlainOption(&buf, use_new_option_syntax, "NOWAIT");
+	}
 	if (maxrate > 0)
-		maxrate_clause = psprintf("MAX_RATE %u", maxrate);
+		AppendBaseBackupIntegerOption(&buf, use_new_option_syntax, "MAX_RATE",
+									  maxrate);
+	if (format == 't')
+		AppendBaseBackupPlainOption(&buf, use_new_option_syntax, "TABLESPACE_MAP");
+	if (!verify_checksums)
+	{
+		if (use_new_option_syntax)
+			AppendBaseBackupIntegerOption(&buf, use_new_option_syntax,
+										  "VERIFY_CHECKSUMS", 0);
+		else
+			AppendBaseBackupPlainOption(&buf, use_new_option_syntax,
+										"NOVERIFY_CHECKSUMS");
+	}
 
 	if (manifest)
 	{
-		if (manifest_force_encode)
-			manifest_clause = "MANIFEST 'force-encode'";
-		else
-			manifest_clause = "MANIFEST 'yes'";
+		AppendBaseBackupStringOption(&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);
+			AppendBaseBackupStringOption(&buf, use_new_option_syntax,
+										 "MANIFEST_CHECKSUMS", manifest_checksums);
 	}
 
 	if (verbose)
@@ -1884,18 +1950,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 410116492e..8ed628d8fa 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -505,14 +505,24 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 	{
 		appendPQExpBufferStr(query, " PHYSICAL");
 		if (reserve_wal)
-			appendPQExpBufferStr(query, " RESERVE_WAL");
+		{
+			if (PQserverVersion(conn) >= 140000)
+				appendPQExpBufferStr(query, " (RESERVE_WAL)");
+			else
+				appendPQExpBufferStr(query, " RESERVE_WAL");
+		}
 	}
 	else
 	{
 		appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
 		if (PQserverVersion(conn) >= 100000)
+		{
 			/* pg_recvlogical doesn't use an exported snapshot, so suppress */
-			appendPQExpBufferStr(query, " NOEXPORT_SNAPSHOT");
+			if (PQserverVersion(conn) >= 140000)
+				appendPQExpBufferStr(query, " (EXPORT_SNAPSHOT 0)");
+			else
+				appendPQExpBufferStr(query, " NOEXPORT_SNAPSHOT");
+		}
 	}
 
 	res = PQexec(conn, query->data);
-- 
2.24.3 (Apple Git-128)