Clean up parsing of synchronous_standby_names GUC variable.

Commit 989be0810dffd08b added a flex/bison lexer/parser to interpret
synchronous_standby_names.  It was done in a pretty crufty way, though,
making assorted end-use sites responsible for calling the parser at the
right times.  That was not only vulnerable to errors of omission, but made
it possible that lexer/parser errors occur at very undesirable times,
and created memory leakages even if there was no error.

Instead, perform the parsing once during check_synchronous_standby_names
and let guc.c manage the resulting data.  To do that, we have to flatten
the parsed representation into a single hunk of malloc'd memory, but that
is not very hard.

While at it, work a little harder on making useful error reports for
parsing problems; the previous code felt that "synchronous_standby_names
parser returned 1" was an appropriate user-facing error message.  (To
be fair, it did also log a syntax error message, but separately from the
GUC problem report, which is at best confusing.)  It had some outright
bugs in the face of invalid input, too.

I (tgl) also concluded that we need to restrict unquoted names in
synchronous_standby_names to be just SQL identifiers.  The previous coding
would accept darn near anything, which (1) makes the quoting convention
both nearly-unnecessary and formally ambiguous, (2) makes it very hard to
understand what is a syntax error and what is a creative interpretation of
the input as a standby name, and (3) makes it impossible to further extend
the syntax in future without a compatibility break.  I presume that we're
intending future extensions of the syntax, else this parsing infrastructure
is massive overkill, so (3) is an important objection.  Since we've taken
a compatibility hit for non-identifier names with this change anyway, we
might as well lock things down now and insist that users use double quotes
for standby names that aren't identifiers.

Kyotaro Horiguchi and Tom Lane

1 files changed, 17 insertions, 8 deletions

diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h
index 14b56649da..e4e0e27371 100644
--- a/src/include/replication/syncrep.h
+++ b/src/include/replication/syncrep.h
@@ -20,7 +20,7 @@
 	(max_wal_senders > 0 && synchronous_commit > SYNCHRONOUS_COMMIT_LOCAL_FLUSH)
 
 /* SyncRepWaitMode */
-#define SYNC_REP_NO_WAIT		-1
+#define SYNC_REP_NO_WAIT		(-1)
 #define SYNC_REP_WAIT_WRITE		0
 #define SYNC_REP_WAIT_FLUSH		1
 #define SYNC_REP_WAIT_APPLY		2
@@ -34,15 +34,24 @@
 
 /*
  * Struct for the configuration of synchronous replication.
+ *
+ * Note: this must be a flat representation that can be held in a single
+ * chunk of malloc'd memory, so that it can be stored as the "extra" data
+ * for the synchronous_standby_names GUC.
  */
 typedef struct SyncRepConfigData
 {
-	int	num_sync;	/* number of sync standbys that we need to wait for */
-	List	*members;	/* list of names of potential sync standbys */
+	int			config_size;	/* total size of this struct, in bytes */
+	int			num_sync;		/* number of sync standbys that we need to
+								 * wait for */
+	int			nmembers;		/* number of members in the following list */
+	/* member_names contains nmembers consecutive nul-terminated C strings */
+	char		member_names[FLEXIBLE_ARRAY_MEMBER];
 } SyncRepConfigData;
 
+/* communication variables for parsing synchronous_standby_names GUC */
 extern SyncRepConfigData *syncrep_parse_result;
-extern SyncRepConfigData *SyncRepConfig;
+extern char *syncrep_parse_error_msg;
 
 /* user-settable parameters for synchronous replication */
 extern char *SyncRepStandbyNames;
@@ -59,21 +68,21 @@ extern void SyncRepReleaseWaiters(void);
 
 /* called by wal sender and user backend */
 extern List *SyncRepGetSyncStandbys(bool *am_sync);
-extern void SyncRepUpdateConfig(void);
-extern void SyncRepFreeConfig(SyncRepConfigData *config);
 
 /* called by checkpointer */
 extern void SyncRepUpdateSyncStandbysDefined(void);
 
+/* GUC infrastructure */
 extern bool check_synchronous_standby_names(char **newval, void **extra, GucSource source);
+extern void assign_synchronous_standby_names(const char *newval, void *extra);
 extern void assign_synchronous_commit(int newval, void *extra);
 
 /*
  * Internal functions for parsing synchronous_standby_names grammar,
  * in syncrep_gram.y and syncrep_scanner.l
  */
-extern int  syncrep_yyparse(void);
-extern int  syncrep_yylex(void);
+extern int	syncrep_yyparse(void);
+extern int	syncrep_yylex(void);
 extern void syncrep_yyerror(const char *str);
 extern void syncrep_scanner_init(const char *query_string);
 extern void syncrep_scanner_finish(void);