0004-syncrep-parser-Return-parse-result-not-via-global-va.patch

text/plain

Filename: 0004-syncrep-parser-Return-parse-result-not-via-global-va.patch
Type: text/plain
Part: 3
Message: Re: pure parsers and reentrant scanners

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch 0004
Subject: syncrep parser: Return parse result not via global variable
File+
src/backend/nls.mk 1 1
src/backend/replication/syncrep.c 4 4
src/backend/replication/syncrep_gram.y 4 5
src/backend/replication/syncrep_scanner.l 19 3
src/include/replication/syncrep.h 3 7
From 4d8bc0e8ad06b1e9edecdd682ddf17dc6800a4dd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 17 Jan 2025 15:41:11 +0100
Subject: [PATCH 4/4] syncrep parser: Return parse result not via global
 variable

Instead of passing the parse result from yyparse() via a global
variable, pass it via a function output argument.
---
 src/backend/nls.mk                        |  2 +-
 src/backend/replication/syncrep.c         |  8 ++++----
 src/backend/replication/syncrep_gram.y    |  9 ++++-----
 src/backend/replication/syncrep_scanner.l | 22 +++++++++++++++++++---
 src/include/replication/syncrep.h         | 10 +++-------
 5 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/src/backend/nls.mk b/src/backend/nls.mk
index 210893c7b72..b7d5dd46e45 100644
--- a/src/backend/nls.mk
+++ b/src/backend/nls.mk
@@ -11,7 +11,7 @@ GETTEXT_TRIGGERS = $(BACKEND_COMMON_GETTEXT_TRIGGERS) \
                    parser_yyerror \
                    replication_yyerror:3 \
                    scanner_yyerror \
-                   syncrep_yyerror:2 \
+                   syncrep_yyerror:4 \
                    report_invalid_record:2 \
                    ereport_startup_progress \
                    json_token_error:2 \
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 1ce8bc7533f..d75e3968035 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -996,13 +996,13 @@ check_synchronous_standby_names(char **newval, void **extra, GucSource source)
 		int			parse_rc;
 		SyncRepConfigData *pconf;
 
-		/* Reset communication variables to ensure a fresh start */
-		syncrep_parse_result = NULL;
-		syncrep_parse_error_msg = NULL;
+		/* Result of parsing is returned in one of these two variables */
+		SyncRepConfigData *syncrep_parse_result = NULL;
+		char	   *syncrep_parse_error_msg = NULL;
 
 		/* Parse the synchronous_standby_names string */
 		syncrep_scanner_init(*newval, &scanner);
-		parse_rc = syncrep_yyparse(scanner);
+		parse_rc = syncrep_yyparse(&syncrep_parse_result, &syncrep_parse_error_msg, scanner);
 		syncrep_scanner_finish(scanner);
 
 		if (parse_rc != 0 || syncrep_parse_result == NULL)
diff --git a/src/backend/replication/syncrep_gram.y b/src/backend/replication/syncrep_gram.y
index 1a3b89ca674..22297bb151a 100644
--- a/src/backend/replication/syncrep_gram.y
+++ b/src/backend/replication/syncrep_gram.y
@@ -19,10 +19,6 @@
 
 #include "syncrep_gram.h"
 
-/* Result of parsing is returned in one of these two variables */
-SyncRepConfigData *syncrep_parse_result;
-char	   *syncrep_parse_error_msg;
-
 static SyncRepConfigData *create_syncrep_config(const char *num_sync,
 					List *members, uint8 syncrep_method);
 
@@ -36,7 +32,10 @@ static SyncRepConfigData *create_syncrep_config(const char *num_sync,
 
 %}
 
+%parse-param {SyncRepConfigData **syncrep_parse_result_p}
+%parse-param {char **syncrep_parse_error_msg_p}
 %parse-param {yyscan_t yyscanner}
+%lex-param   {char **syncrep_parse_error_msg_p}
 %lex-param   {yyscan_t yyscanner}
 %pure-parser
 %expect 0
@@ -60,7 +59,7 @@ static SyncRepConfigData *create_syncrep_config(const char *num_sync,
 %%
 result:
 		standby_config				{
-										syncrep_parse_result = $1;
+										*syncrep_parse_result_p = $1;
 										(void) yynerrs; /* suppress compiler warning */
 									}
 	;
diff --git a/src/backend/replication/syncrep_scanner.l b/src/backend/replication/syncrep_scanner.l
index 05e5fdecf1b..7dec1f869c7 100644
--- a/src/backend/replication/syncrep_scanner.l
+++ b/src/backend/replication/syncrep_scanner.l
@@ -42,6 +42,13 @@ struct syncrep_yy_extra_type
 	StringInfoData xdbuf;
 };
 
+/*
+ * Better keep this definition here than put it in replication/syncrep.h and
+ * save a bit of duplication.  Putting it in replication/syncrep.h would leak
+ * the definition to other parts and possibly affect other scanners.
+*/
+#define YY_DECL extern int	syncrep_yylex(union YYSTYPE *yylval_param, char **syncrep_parse_error_msg_p, yyscan_t yyscanner)
+
 /* LCOV_EXCL_START */
 
 %}
@@ -104,7 +111,7 @@ xdinside		[^"]+
 				return NAME;
 		}
 <xd><<EOF>> {
-				syncrep_yyerror(yyscanner, "unterminated quoted identifier");
+				syncrep_yyerror(NULL, syncrep_parse_error_msg_p, yyscanner, "unterminated quoted identifier");
 				return JUNK;
 		}
 
@@ -136,12 +143,21 @@ xdinside		[^"]+
 #undef yyextra
 #define yyextra (((struct yyguts_t *) yyscanner)->yyextra_r)
 
-/* Needs to be here for access to yytext */
+/*
+ * This yyerror() function does not raise an error (elog or similar), it just
+ * collects the error message in *syncrep_parse_error_msg_p and leaves it to
+ * the ultimate caller of the syncrep parser to raise the error.  (The
+ * ultimate caller will do that with special GUC error functions.)
+ *
+ * (The first argument is enforced by Bison to match the first argument of
+ * yyparse(), but it is not used here.)
+ */
 void
-syncrep_yyerror(yyscan_t yyscanner, const char *message)
+syncrep_yyerror(SyncRepConfigData **syncrep_parse_result_p, char **syncrep_parse_error_msg_p, yyscan_t yyscanner, const char *message)
 {
 	struct yyguts_t *yyg = (struct yyguts_t *) yyscanner;	/* needed for yytext
 															 * macro */
+	char *syncrep_parse_error_msg = *syncrep_parse_error_msg_p;
 
 	/* report only the first error in a parse operation */
 	if (syncrep_parse_error_msg)
diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h
index 33b9cdb18f7..675669a79f7 100644
--- a/src/include/replication/syncrep.h
+++ b/src/include/replication/syncrep.h
@@ -73,10 +73,6 @@ typedef struct SyncRepConfigData
 
 extern PGDLLIMPORT SyncRepConfigData *SyncRepConfig;
 
-/* communication variables for parsing synchronous_standby_names GUC */
-extern PGDLLIMPORT SyncRepConfigData *syncrep_parse_result;
-extern PGDLLIMPORT char *syncrep_parse_error_msg;
-
 /* user-settable parameters for synchronous replication */
 extern PGDLLIMPORT char *SyncRepStandbyNames;
 
@@ -105,9 +101,9 @@ union YYSTYPE;
 #define YY_TYPEDEF_YY_SCANNER_T
 typedef void *yyscan_t;
 #endif
-extern int	syncrep_yyparse(yyscan_t yyscanner);
-extern int	syncrep_yylex(union YYSTYPE *yylval_param, yyscan_t yyscanner);
-extern void syncrep_yyerror(yyscan_t yyscanner, const char *str);
+extern int	syncrep_yyparse(SyncRepConfigData **syncrep_parse_result_p, char **syncrep_parse_error_msg_p, yyscan_t yyscanner);
+extern int	syncrep_yylex(union YYSTYPE *yylval_param, char **syncrep_parse_error_msg_p, yyscan_t yyscanner);
+extern void syncrep_yyerror(SyncRepConfigData **syncrep_parse_result_p, char **syncrep_parse_error_msg_p, yyscan_t yyscanner, const char *str);
 extern void syncrep_scanner_init(const char *str, yyscan_t *yyscannerp);
 extern void syncrep_scanner_finish(yyscan_t yyscanner);
 
-- 
2.47.1