public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v2 1/4] Add a parser_hook hook.
7+ messages / 2 participants
[nested] [flat]

* [PATCH v3 1/4] Add a parser_hook hook.
@ 2021-04-21 14:47 Julien Rouhaud <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Julien Rouhaud @ 2021-04-21 14:47 UTC (permalink / raw)

This does nothing but allow third-party plugins to implement a different
syntax, and fallback on the core parser if they don't implement a superset of
the supported core syntax.
---
 src/backend/tcop/postgres.c | 16 ++++++++++++++--
 src/include/tcop/tcopprot.h |  5 +++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8cea10c901..e941b59b85 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -99,6 +99,9 @@ int			log_statement = LOGSTMT_NONE;
 /* GUC variable for maximum stack depth (measured in kilobytes) */
 int			max_stack_depth = 100;
 
+/* Hook for plugins to get control in pg_parse_query() */
+parser_hook_type parser_hook = NULL;
+
 /* wait N seconds to allow attach from a debugger */
 int			PostAuthDelay = 0;
 
@@ -589,18 +592,27 @@ ProcessClientWriteInterrupt(bool blocked)
  * database tables.  So, we rely on the raw parser to determine whether
  * we've seen a COMMIT or ABORT command; when we are in abort state, other
  * commands are not processed any further than the raw parse stage.
+ *
+ * To support loadable plugins that monitor the parsing or implements SQL
+ * syntactic sugar we provide a hook variable that lets a plugin get control
+ * before and after the standard parsing process.  If the plugin only implement
+ * a subset of postgres supported syntax, it's its duty to call raw_parser (or
+ * the previous hook if any) for the statements it doesn't understand.
  */
 List *
 pg_parse_query(const char *query_string)
 {
-	List	   *raw_parsetree_list;
+	List	   *raw_parsetree_list = NIL;
 
 	TRACE_POSTGRESQL_QUERY_PARSE_START(query_string);
 
 	if (log_parser_stats)
 		ResetUsage();
 
-	raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
+	if (parser_hook)
+		raw_parsetree_list = (*parser_hook) (query_string, RAW_PARSE_DEFAULT);
+	else
+		raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
 
 	if (log_parser_stats)
 		ShowUsage("PARSER STATISTICS");
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index 968345404e..131dc2b22e 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -17,6 +17,7 @@
 #include "nodes/params.h"
 #include "nodes/parsenodes.h"
 #include "nodes/plannodes.h"
+#include "parser/parser.h"
 #include "storage/procsignal.h"
 #include "utils/guc.h"
 #include "utils/queryenvironment.h"
@@ -43,6 +44,10 @@ typedef enum
 
 extern PGDLLIMPORT int log_statement;
 
+/* Hook for plugins to get control in pg_parse_query() */
+typedef List *(*parser_hook_type) (const char *str, RawParseMode mode);
+extern PGDLLIMPORT parser_hook_type parser_hook;
+
 extern List *pg_parse_query(const char *query_string);
 extern List *pg_rewrite_query(Query *query);
 extern List *pg_analyze_and_rewrite(RawStmt *parsetree,
-- 
2.31.1


--v6yspzozp6k6x4hy
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v3-0002-Add-a-sqlol-parser.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v2 1/4] Add a parser_hook hook.
@ 2021-04-21 14:47 Julien Rouhaud <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Julien Rouhaud @ 2021-04-21 14:47 UTC (permalink / raw)

This does nothing but allow third-party plugins to implement a different
syntax, and fallback on the core parser if they don't implement a superset of
the supported core syntax.
---
 src/backend/tcop/postgres.c | 16 ++++++++++++++--
 src/include/tcop/tcopprot.h |  5 +++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8cea10c901..e941b59b85 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -99,6 +99,9 @@ int			log_statement = LOGSTMT_NONE;
 /* GUC variable for maximum stack depth (measured in kilobytes) */
 int			max_stack_depth = 100;
 
+/* Hook for plugins to get control in pg_parse_query() */
+parser_hook_type parser_hook = NULL;
+
 /* wait N seconds to allow attach from a debugger */
 int			PostAuthDelay = 0;
 
@@ -589,18 +592,27 @@ ProcessClientWriteInterrupt(bool blocked)
  * database tables.  So, we rely on the raw parser to determine whether
  * we've seen a COMMIT or ABORT command; when we are in abort state, other
  * commands are not processed any further than the raw parse stage.
+ *
+ * To support loadable plugins that monitor the parsing or implements SQL
+ * syntactic sugar we provide a hook variable that lets a plugin get control
+ * before and after the standard parsing process.  If the plugin only implement
+ * a subset of postgres supported syntax, it's its duty to call raw_parser (or
+ * the previous hook if any) for the statements it doesn't understand.
  */
 List *
 pg_parse_query(const char *query_string)
 {
-	List	   *raw_parsetree_list;
+	List	   *raw_parsetree_list = NIL;
 
 	TRACE_POSTGRESQL_QUERY_PARSE_START(query_string);
 
 	if (log_parser_stats)
 		ResetUsage();
 
-	raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
+	if (parser_hook)
+		raw_parsetree_list = (*parser_hook) (query_string, RAW_PARSE_DEFAULT);
+	else
+		raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
 
 	if (log_parser_stats)
 		ShowUsage("PARSER STATISTICS");
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index 968345404e..131dc2b22e 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -17,6 +17,7 @@
 #include "nodes/params.h"
 #include "nodes/parsenodes.h"
 #include "nodes/plannodes.h"
+#include "parser/parser.h"
 #include "storage/procsignal.h"
 #include "utils/guc.h"
 #include "utils/queryenvironment.h"
@@ -43,6 +44,10 @@ typedef enum
 
 extern PGDLLIMPORT int log_statement;
 
+/* Hook for plugins to get control in pg_parse_query() */
+typedef List *(*parser_hook_type) (const char *str, RawParseMode mode);
+extern PGDLLIMPORT parser_hook_type parser_hook;
+
 extern List *pg_parse_query(const char *query_string);
 extern List *pg_rewrite_query(Query *query);
 extern List *pg_analyze_and_rewrite(RawStmt *parsetree,
-- 
2.31.1


--p53pz3r7l3llx7gx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v2-0002-Add-a-sqlol-parser.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v4 1/4] Add a parser_hook hook.
@ 2021-04-21 14:47 Julien Rouhaud <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Julien Rouhaud @ 2021-04-21 14:47 UTC (permalink / raw)

This does nothing but allow third-party plugins to implement a different
syntax, and fallback on the core parser if they don't implement a superset of
the supported core syntax.
---
 src/backend/tcop/postgres.c | 16 ++++++++++++++--
 src/include/tcop/tcopprot.h |  5 +++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8cea10c901..e941b59b85 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -99,6 +99,9 @@ int			log_statement = LOGSTMT_NONE;
 /* GUC variable for maximum stack depth (measured in kilobytes) */
 int			max_stack_depth = 100;
 
+/* Hook for plugins to get control in pg_parse_query() */
+parser_hook_type parser_hook = NULL;
+
 /* wait N seconds to allow attach from a debugger */
 int			PostAuthDelay = 0;
 
@@ -589,18 +592,27 @@ ProcessClientWriteInterrupt(bool blocked)
  * database tables.  So, we rely on the raw parser to determine whether
  * we've seen a COMMIT or ABORT command; when we are in abort state, other
  * commands are not processed any further than the raw parse stage.
+ *
+ * To support loadable plugins that monitor the parsing or implements SQL
+ * syntactic sugar we provide a hook variable that lets a plugin get control
+ * before and after the standard parsing process.  If the plugin only implement
+ * a subset of postgres supported syntax, it's its duty to call raw_parser (or
+ * the previous hook if any) for the statements it doesn't understand.
  */
 List *
 pg_parse_query(const char *query_string)
 {
-	List	   *raw_parsetree_list;
+	List	   *raw_parsetree_list = NIL;
 
 	TRACE_POSTGRESQL_QUERY_PARSE_START(query_string);
 
 	if (log_parser_stats)
 		ResetUsage();
 
-	raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
+	if (parser_hook)
+		raw_parsetree_list = (*parser_hook) (query_string, RAW_PARSE_DEFAULT);
+	else
+		raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
 
 	if (log_parser_stats)
 		ShowUsage("PARSER STATISTICS");
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index 968345404e..131dc2b22e 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -17,6 +17,7 @@
 #include "nodes/params.h"
 #include "nodes/parsenodes.h"
 #include "nodes/plannodes.h"
+#include "parser/parser.h"
 #include "storage/procsignal.h"
 #include "utils/guc.h"
 #include "utils/queryenvironment.h"
@@ -43,6 +44,10 @@ typedef enum
 
 extern PGDLLIMPORT int log_statement;
 
+/* Hook for plugins to get control in pg_parse_query() */
+typedef List *(*parser_hook_type) (const char *str, RawParseMode mode);
+extern PGDLLIMPORT parser_hook_type parser_hook;
+
 extern List *pg_parse_query(const char *query_string);
 extern List *pg_rewrite_query(Query *query);
 extern List *pg_analyze_and_rewrite(RawStmt *parsetree,
-- 
2.31.1


--j3ft3uyq57t2xkwr
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v4-0002-Add-a-sqlol-parser.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v1 1/4] Add a parser_hook hook.
@ 2021-04-21 14:47 Julien Rouhaud <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Julien Rouhaud @ 2021-04-21 14:47 UTC (permalink / raw)

This does nothing but allow third-party plugins to implement a different
syntax, and fallback on the core parser if they don't implement a superset of
the supported core syntax.
---
 src/backend/tcop/postgres.c | 16 ++++++++++++++--
 src/include/tcop/tcopprot.h |  5 +++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2d6d145ecc..e91db69830 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -99,6 +99,9 @@ int			log_statement = LOGSTMT_NONE;
 /* GUC variable for maximum stack depth (measured in kilobytes) */
 int			max_stack_depth = 100;
 
+/* Hook for plugins to get control in pg_parse_query() */
+parser_hook_type parser_hook = NULL;
+
 /* wait N seconds to allow attach from a debugger */
 int			PostAuthDelay = 0;
 
@@ -589,18 +592,27 @@ ProcessClientWriteInterrupt(bool blocked)
  * database tables.  So, we rely on the raw parser to determine whether
  * we've seen a COMMIT or ABORT command; when we are in abort state, other
  * commands are not processed any further than the raw parse stage.
+ *
+ * To support loadable plugins that monitor the parsing or implements SQL
+ * syntactic sugar we provide a hook variable that lets a plugin get control
+ * before and after the standard parsing process.  If the plugin only implement
+ * a subset of postgres supported syntax, it's its duty to call raw_parser (or
+ * the previous hook if any) for the statements it doesn't understand.
  */
 List *
 pg_parse_query(const char *query_string)
 {
-	List	   *raw_parsetree_list;
+	List	   *raw_parsetree_list = NIL;
 
 	TRACE_POSTGRESQL_QUERY_PARSE_START(query_string);
 
 	if (log_parser_stats)
 		ResetUsage();
 
-	raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
+	if (parser_hook)
+		raw_parsetree_list = (*parser_hook) (query_string, RAW_PARSE_DEFAULT);
+	else
+		raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
 
 	if (log_parser_stats)
 		ShowUsage("PARSER STATISTICS");
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index 968345404e..131dc2b22e 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -17,6 +17,7 @@
 #include "nodes/params.h"
 #include "nodes/parsenodes.h"
 #include "nodes/plannodes.h"
+#include "parser/parser.h"
 #include "storage/procsignal.h"
 #include "utils/guc.h"
 #include "utils/queryenvironment.h"
@@ -43,6 +44,10 @@ typedef enum
 
 extern PGDLLIMPORT int log_statement;
 
+/* Hook for plugins to get control in pg_parse_query() */
+typedef List *(*parser_hook_type) (const char *str, RawParseMode mode);
+extern PGDLLIMPORT parser_hook_type parser_hook;
+
 extern List *pg_parse_query(const char *query_string);
 extern List *pg_rewrite_query(Query *query);
 extern List *pg_analyze_and_rewrite(RawStmt *parsetree,
-- 
2.30.1


--ymfuxy2befenx45f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v1-0002-Add-a-sqlol-parser.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v5 1/4] Add a parser_hook hook.
@ 2021-04-21 14:47 Julien Rouhaud <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Julien Rouhaud @ 2021-04-21 14:47 UTC (permalink / raw)

This does nothing but allow third-party plugins to implement a different
syntax, and fallback on the core parser if they don't implement a superset of
the supported core syntax.
---
 src/backend/tcop/postgres.c | 16 ++++++++++++++--
 src/include/tcop/tcopprot.h |  5 +++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 3f9ed549f9..66ee58a4b1 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -99,6 +99,9 @@ int			log_statement = LOGSTMT_NONE;
 /* GUC variable for maximum stack depth (measured in kilobytes) */
 int			max_stack_depth = 100;
 
+/* Hook for plugins to get control in pg_parse_query() */
+parser_hook_type parser_hook = NULL;
+
 /* wait N seconds to allow attach from a debugger */
 int			PostAuthDelay = 0;
 
@@ -589,18 +592,27 @@ ProcessClientWriteInterrupt(bool blocked)
  * database tables.  So, we rely on the raw parser to determine whether
  * we've seen a COMMIT or ABORT command; when we are in abort state, other
  * commands are not processed any further than the raw parse stage.
+ *
+ * To support loadable plugins that monitor the parsing or implements SQL
+ * syntactic sugar we provide a hook variable that lets a plugin get control
+ * before and after the standard parsing process.  If the plugin only implement
+ * a subset of postgres supported syntax, it's its duty to call raw_parser (or
+ * the previous hook if any) for the statements it doesn't understand.
  */
 List *
 pg_parse_query(const char *query_string)
 {
-	List	   *raw_parsetree_list;
+	List	   *raw_parsetree_list = NIL;
 
 	TRACE_POSTGRESQL_QUERY_PARSE_START(query_string);
 
 	if (log_parser_stats)
 		ResetUsage();
 
-	raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
+	if (parser_hook)
+		raw_parsetree_list = (*parser_hook) (query_string, RAW_PARSE_DEFAULT);
+	else
+		raw_parsetree_list = raw_parser(query_string, RAW_PARSE_DEFAULT);
 
 	if (log_parser_stats)
 		ShowUsage("PARSER STATISTICS");
diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h
index 968345404e..131dc2b22e 100644
--- a/src/include/tcop/tcopprot.h
+++ b/src/include/tcop/tcopprot.h
@@ -17,6 +17,7 @@
 #include "nodes/params.h"
 #include "nodes/parsenodes.h"
 #include "nodes/plannodes.h"
+#include "parser/parser.h"
 #include "storage/procsignal.h"
 #include "utils/guc.h"
 #include "utils/queryenvironment.h"
@@ -43,6 +44,10 @@ typedef enum
 
 extern PGDLLIMPORT int log_statement;
 
+/* Hook for plugins to get control in pg_parse_query() */
+typedef List *(*parser_hook_type) (const char *str, RawParseMode mode);
+extern PGDLLIMPORT parser_hook_type parser_hook;
+
 extern List *pg_parse_query(const char *query_string);
 extern List *pg_rewrite_query(Query *query);
 extern List *pg_analyze_and_rewrite(RawStmt *parsetree,
-- 
2.32.0


--n4pdl6wolj3mjhvd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v5-0002-Add-a-sqlol-parser.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Fixup some more appendStringInfo misusages
@ 2023-10-02 22:24 David Rowley <[email protected]>
  2023-10-03 04:13 ` Re: Fixup some more appendStringInfo misusages David Rowley <[email protected]>
  0 siblings, 1 reply; 7+ messages in thread

From: David Rowley @ 2023-10-02 22:24 UTC (permalink / raw)
  To: PostgreSQL Developers <[email protected]>

The attached v1-0001 patch adjusts some code in stringinfo.h to find
misusages of the appendStringInfo functions.  I don't intend to commit
that, but I do intend to commit the patch for the new misusages that
it found, which is also attached.

This is along the same lines as 8b26769bc, f736e188c, 110d81728 and 8abc13a88.

David


Attachments:

  [application/octet-stream] fixup_some_appendStringInfo_misuages_2023.patch (4.4K, ../../CAApHDvpXKQmL+r=VDNS98upqhr9yGBhv2Jw3GBFFk_wKHcB39A@mail.gmail.com/2-fixup_some_appendStringInfo_misuages_2023.patch)
  download | inline diff:
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index d73248abdd..f382c0f623 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -47,7 +47,7 @@ infobits_desc(StringInfo buf, uint8 infobits, const char *keyname)
 		buf->data[buf->len] = '\0';
 	}
 
-	appendStringInfoString(buf, "]");
+	appendStringInfoChar(buf, ']');
 }
 
 static void
@@ -68,7 +68,7 @@ truncate_flags_desc(StringInfo buf, uint8 flags)
 		buf->data[buf->len] = '\0';
 	}
 
-	appendStringInfoString(buf, "]");
+	appendStringInfoChar(buf, ']');
 }
 
 static void
@@ -88,7 +88,7 @@ plan_elem_desc(StringInfo buf, void *plan, void *data)
 
 	*offsets += new_plan->ntuples;
 
-	appendStringInfo(buf, " }");
+	appendStringInfoString(buf, " }");
 }
 
 void
diff --git a/src/backend/access/rmgrdesc/nbtdesc.c b/src/backend/access/rmgrdesc/nbtdesc.c
index e4fbaa4d5d..f3d725a274 100644
--- a/src/backend/access/rmgrdesc/nbtdesc.c
+++ b/src/backend/access/rmgrdesc/nbtdesc.c
@@ -248,5 +248,5 @@ delvacuum_desc(StringInfo buf, char *block_data,
 			((char *) updates + SizeOfBtreeUpdate +
 			 updates->ndeletedtids * sizeof(uint16));
 	}
-	appendStringInfoString(buf, "]");
+	appendStringInfoChar(buf, ']');
 }
diff --git a/src/backend/access/rmgrdesc/rmgrdesc_utils.c b/src/backend/access/rmgrdesc/rmgrdesc_utils.c
index 808770524d..ef89179bb4 100644
--- a/src/backend/access/rmgrdesc/rmgrdesc_utils.c
+++ b/src/backend/access/rmgrdesc/rmgrdesc_utils.c
@@ -37,7 +37,7 @@ array_desc(StringInfo buf, void *array, size_t elem_size, int count,
 		if (i < count - 1)
 			appendStringInfoString(buf, ", ");
 	}
-	appendStringInfoString(buf, "]");
+	appendStringInfoChar(buf, ']');
 }
 
 void
diff --git a/src/backend/access/transam/xlogbackup.c b/src/backend/access/transam/xlogbackup.c
index 23461c9d2c..21d68133ae 100644
--- a/src/backend/access/transam/xlogbackup.c
+++ b/src/backend/access/transam/xlogbackup.c
@@ -58,7 +58,7 @@ build_backup_content(BackupState *state, bool ishistoryfile)
 
 	appendStringInfo(result, "CHECKPOINT LOCATION: %X/%X\n",
 					 LSN_FORMAT_ARGS(state->checkpointloc));
-	appendStringInfo(result, "BACKUP METHOD: streamed\n");
+	appendStringInfoString(result, "BACKUP METHOD: streamed\n");
 	appendStringInfo(result, "BACKUP FROM: %s\n",
 					 state->started_in_recovery ? "standby" : "primary");
 	appendStringInfo(result, "START TIME: %s\n", startstrbuf);
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 3ded3c1473..7e5ec500d8 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1282,7 +1282,7 @@ ReportSlotInvalidation(ReplicationSlotInvalidationCause cause,
 			break;
 
 		case RS_INVAL_WAL_LEVEL:
-			appendStringInfo(&err_detail, _("Logical decoding on standby requires wal_level >= logical on the primary server."));
+			appendStringInfoString(&err_detail, _("Logical decoding on standby requires wal_level >= logical on the primary server."));
 			break;
 		case RS_INVAL_NONE:
 			pg_unreachable();
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 8d5eac4791..442205382e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10395,7 +10395,7 @@ get_func_sql_syntax(FuncExpr *expr, deparse_context *context)
 
 		case F_IS_NORMALIZED:
 			/* IS xxx NORMALIZED */
-			appendStringInfoString(buf, "(");
+			appendStringInfoChar(buf, '(');
 			get_rule_expr_paren((Node *) linitial(expr->args), context, false,
 								(Node *) expr);
 			appendStringInfoString(buf, " IS");
@@ -10884,7 +10884,7 @@ get_json_constructor(JsonConstructorExpr *ctor, deparse_context *context,
 	}
 
 	get_json_constructor_options(ctor, buf);
-	appendStringInfo(buf, ")");
+	appendStringInfoChar(buf, ')');
 }
 
 /*
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index ec67588cf5..7f704da730 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -2339,9 +2339,9 @@ regression_main(int argc, char *argv[],
 							 bindir ? "/" : "",
 							 temp_instance);
 			if (debug)
-				appendStringInfo(&cmd, " --debug");
+				appendStringInfoString(&cmd, " --debug");
 			if (nolocale)
-				appendStringInfo(&cmd, " --no-locale");
+				appendStringInfoString(&cmd, " --no-locale");
 			appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir);
 			fflush(NULL);
 			if (system(cmd.data))


  [application/octet-stream] v1-0001-Adjust-code-to-highlight-appendStringInfo-misusag.patch (9.3K, ../../CAApHDvpXKQmL+r=VDNS98upqhr9yGBhv2Jw3GBFFk_wKHcB39A@mail.gmail.com/3-v1-0001-Adjust-code-to-highlight-appendStringInfo-misusag.patch)
  download | inline diff:
From cf13f964c0e3b4f424c73f8bc6df603de69c8269 Mon Sep 17 00:00:00 2001
From: David Rowley <[email protected]>
Date: Thu, 15 Sep 2022 21:27:00 +1200
Subject: [PATCH v1] Adjust code to highlight appendStringInfo misusages

Not intended for commit
---
 src/backend/nodes/outfuncs.c | 40 ++++++++++++++++++------------------
 src/backend/utils/adt/xml.c  |  2 +-
 src/common/stringinfo.c      |  6 +++---
 src/include/lib/stringinfo.h | 30 +++++++++++++++++++++++----
 4 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index e66a99247e..a236ec9e77 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -42,92 +42,92 @@ static void outDouble(StringInfo str, double d);
 
 /* Write an integer field (anything written as ":fldname %d") */
 #define WRITE_INT_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %d", node->fldname)
 
 /* Write an unsigned integer field (anything written as ":fldname %u") */
 #define WRITE_UINT_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %u", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %u", node->fldname)
 
 /* Write an unsigned integer field (anything written with UINT64_FORMAT) */
 #define WRITE_UINT64_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " " UINT64_FORMAT, \
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " " UINT64_FORMAT, \
 					 node->fldname)
 
 /* Write an OID field (don't hard-wire assumption that OID is same as uint) */
 #define WRITE_OID_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %u", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %u", node->fldname)
 
 /* Write a long-integer field */
 #define WRITE_LONG_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %ld", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %ld", node->fldname)
 
 /* Write a char field (ie, one ascii character) */
 #define WRITE_CHAR_FIELD(fldname) \
-	(appendStringInfo(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outChar(str, node->fldname))
 
 /* Write an enumerated-type field as an integer code */
 #define WRITE_ENUM_FIELD(fldname, enumtype) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", \
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %d", \
 					 (int) node->fldname)
 
 /* Write a float field (actually, they're double) */
 #define WRITE_FLOAT_FIELD(fldname) \
-	(appendStringInfo(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoInternal(str, " :" CppAsString(fldname) " "), \
 	 outDouble(str, node->fldname))
 
 /* Write a boolean field */
 #define WRITE_BOOL_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %s", \
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %s", \
 					 booltostr(node->fldname))
 
 /* Write a character-string (possibly NULL) field */
 #define WRITE_STRING_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outToken(str, node->fldname))
 
 /* Write a parse location field (actually same as INT case) */
 #define WRITE_LOCATION_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", node->fldname)
+	appendStringInfoInternal(str, " :" CppAsString(fldname) " %d", node->fldname)
 
 /* Write a Node field */
 #define WRITE_NODE_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outNode(str, node->fldname))
 
 /* Write a bitmapset field */
 #define WRITE_BITMAPSET_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 outBitmapset(str, node->fldname))
 
 /* Write a variable-length array (not a List) of Node pointers */
 #define WRITE_NODE_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeNodeArray(str, (const Node * const *) node->fldname, len))
 
 /* Write a variable-length array of AttrNumber */
 #define WRITE_ATTRNUMBER_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeAttrNumberCols(str, node->fldname, len))
 
 /* Write a variable-length array of Oid */
 #define WRITE_OID_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeOidCols(str, node->fldname, len))
 
 /* Write a variable-length array of Index */
 #define WRITE_INDEX_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeIndexCols(str, node->fldname, len))
 
 /* Write a variable-length array of int */
 #define WRITE_INT_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeIntCols(str, node->fldname, len))
 
 /* Write a variable-length array of bool */
 #define WRITE_BOOL_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
+	(appendStringInfoStringInternal(str, " :" CppAsString(fldname) " "), \
 	 writeBoolCols(str, node->fldname, len))
 
 #define booltostr(x)  ((x) ? "true" : "false")
@@ -233,7 +233,7 @@ fnname(StringInfo str, const datatype *arr, int len) \
 		appendStringInfoChar(str, ')'); \
 	} \
 	else \
-		appendStringInfoString(str, "<>"); \
+		appendStringInfoStringInternal(str, "<>"); \
 }
 
 WRITE_SCALAR_ARRAY(writeAttrNumberCols, AttrNumber, " %d",)
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 2300c7ebf3..9df1a5b72c 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -2104,7 +2104,7 @@ xml_errorHandler(void *data, xmlErrorPtr error)
 		void	   *errCtxSaved = xmlGenericErrorContext;
 
 		xmlSetGenericErrorFunc((void *) errorBuf,
-							   (xmlGenericErrorFunc) appendStringInfo);
+							   (xmlGenericErrorFunc) appendStringInfoInternal);
 
 		/* Add context information to errorBuf */
 		appendStringInfoLineSeparator(errorBuf);
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index 05b22b5c53..a275185113 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -88,7 +88,7 @@ resetStringInfo(StringInfo str)
  * strcat.
  */
 void
-appendStringInfo(StringInfo str, const char *fmt,...)
+appendStringInfoInternal(StringInfo str, const char *fmt, ...)
 {
 	int			save_errno = errno;
 
@@ -167,13 +167,13 @@ appendStringInfoVA(StringInfo str, const char *fmt, va_list args)
 }
 
 /*
- * appendStringInfoString
+ * appendStringInfoStringInternal
  *
  * Append a null-terminated string to str.
  * Like appendStringInfo(str, "%s", s) but faster.
  */
 void
-appendStringInfoString(StringInfo str, const char *s)
+appendStringInfoStringInternal(StringInfo str, const char *s)
 {
 	appendBinaryStringInfo(str, s, strlen(s));
 }
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 36a416f8e0..4fa1b1ea66 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -93,9 +93,20 @@ extern void resetStringInfo(StringInfo str);
  * to str if necessary.  This is sort of like a combination of sprintf and
  * strcat.
  */
-extern void appendStringInfo(StringInfo str, const char *fmt,...) pg_attribute_printf(2, 3);
-
-/*------------------------
+#if defined(HAVE__BUILTIN_CONSTANT_P) && defined(USE_ASSERT_CHECKING)
+#define appendStringInfo(str, fmt, ...) \
+	do { \
+		StaticAssertStmt(!__builtin_constant_p(fmt) || strcmp(fmt, "%s") != 0,   \
+						 "use appendStringInfoString instead of appendStringInfo with %s");   \
+		appendStringInfoInternal(str, fmt, __VA_ARGS__); \
+	} while (0)
+#else
+#define appendStringInfo(str, fmt, ...) appendStringInfoInternal(str, fmt, __VA_ARGS__)
+#endif
+
+extern void appendStringInfoInternal(StringInfo str, const char *fmt,...) pg_attribute_printf(2, 3);
+
+ /*------------------------
  * appendStringInfoVA
  * Attempt to format text data under the control of fmt (an sprintf-style
  * format string) and append it to whatever is already in str.  If successful
@@ -111,7 +122,18 @@ extern int	appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_
  * Append a null-terminated string to str.
  * Like appendStringInfo(str, "%s", s) but faster.
  */
-extern void appendStringInfoString(StringInfo str, const char *s);
+#if defined(HAVE__BUILTIN_CONSTANT_P) && defined(USE_ASSERT_CHECKING)
+#define appendStringInfoString(str, s) \
+	do { \
+		StaticAssertStmt(!__builtin_constant_p(s) || strlen(s) != 1, \
+						 "use appendStringInfoChar to append single characters to a string"); \
+		appendStringInfoStringInternal(str, s); \
+	 } while (0)
+#else
+#define appendStringInfoString(str, s) appendStringInfoStringInternal(str, s)
+#endif
+
+extern void appendStringInfoStringInternal(StringInfo str, const char *s);
 
 /*------------------------
  * appendStringInfoChar
-- 
2.40.1.windows.1



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* Re: Fixup some more appendStringInfo misusages
  2023-10-02 22:24 Fixup some more appendStringInfo misusages David Rowley <[email protected]>
@ 2023-10-03 04:13 ` David Rowley <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: David Rowley @ 2023-10-03 04:13 UTC (permalink / raw)
  To: PostgreSQL Developers <[email protected]>

On Tue, 3 Oct 2023 at 11:24, David Rowley <[email protected]> wrote:
> This is along the same lines as 8b26769bc, f736e188c, 110d81728 and 8abc13a88.

I've pushed the patch to fix the misusages of the functions.

David






^ permalink  raw  reply  [nested|flat] 7+ messages in thread

end of thread, other threads:[~2023-10-03 04:13 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 14:47 [PATCH v3 1/4] Add a parser_hook hook. Julien Rouhaud <[email protected]>
2021-04-21 14:47 [PATCH v2 1/4] Add a parser_hook hook. Julien Rouhaud <[email protected]>
2021-04-21 14:47 [PATCH v4 1/4] Add a parser_hook hook. Julien Rouhaud <[email protected]>
2021-04-21 14:47 [PATCH v1 1/4] Add a parser_hook hook. Julien Rouhaud <[email protected]>
2021-04-21 14:47 [PATCH v5 1/4] Add a parser_hook hook. Julien Rouhaud <[email protected]>
2023-10-02 22:24 Fixup some more appendStringInfo misusages David Rowley <[email protected]>
2023-10-03 04:13 ` Re: Fixup some more appendStringInfo misusages David Rowley <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox