Do not output actual value of location fields in node serialization by default

This changes nodeToString() to not output the actual value of location
fields in nodes, but instead it writes -1.  This mirrors the fact that
stringToNode() also does not read location field values but always
stores -1.

For most uses of nodeToString(), which is to store nodes in catalog
fields, this is more useful.  We don't store original query texts in
catalogs, so any lingering query location values are not meaningful.

For debugging purposes, there is a new nodeToStringWithLocations(),
which mirrors the existing stringToNodeWithLocations().  This is used
for WRITE_READ_PARSE_PLAN_TREES and nodes/print.c functions, which
covers all the debugging uses.

Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAEze2WgrCiR3JZmWyB0YTc8HV7ewRdx13j0CqD6mVkYAW+SFGQ@mail.gmail.com

4 files changed, 41 insertions, 9 deletions

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index c55375e7f91..3337b77ae6d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -25,6 +25,9 @@
 #include "nodes/pg_list.h"
 #include "utils/datum.h"
 
+/* State flag that determines how nodeToStringInternal() should treat location fields */
+static bool write_location_fields = false;
+
 static void outChar(StringInfo str, char c);
 static void outDouble(StringInfo str, double d);
 
@@ -88,7 +91,7 @@ static void outDouble(StringInfo str, double d);
 
 /* Write a parse location field (actually same as INT case) */
 #define WRITE_LOCATION_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", node->fldname)
+	appendStringInfo(str, " :" CppAsString(fldname) " %d", write_location_fields ? node->fldname : -1)
 
 /* Write a Node field */
 #define WRITE_NODE_FIELD(fldname) \
@@ -757,19 +760,47 @@ outNode(StringInfo str, const void *obj)
 /*
  * nodeToString -
  *	   returns the ascii representation of the Node as a palloc'd string
+ *
+ * write_loc_fields determines whether location fields are output with their
+ * actual value rather than -1.  The actual value can be useful for debugging,
+ * but for most uses, the actual value is not useful, since the original query
+ * string is no longer available.
  */
-char *
-nodeToString(const void *obj)
+static char *
+nodeToStringInternal(const void *obj, bool write_loc_fields)
 {
 	StringInfoData str;
+	bool		save_write_location_fields;
+
+	save_write_location_fields = write_location_fields;
+	write_location_fields = write_loc_fields;
 
 	/* see stringinfo.h for an explanation of this maneuver */
 	initStringInfo(&str);
 	outNode(&str, obj);
+
+	write_location_fields = save_write_location_fields;
+
 	return str.data;
 }
 
 /*
+ * Externally visible entry points
+ */
+char *
+nodeToString(const void *obj)
+{
+	return nodeToStringInternal(obj, false);
+}
+
+char *
+nodeToStringWithLocations(const void *obj)
+{
+	return nodeToStringInternal(obj, true);
+}
+
+
+/*
  * bmsToString -
  *	   returns the ascii representation of the Bitmapset as a palloc'd string
  */
diff --git a/src/backend/nodes/print.c b/src/backend/nodes/print.c
index d2a58a5956a..02798f4482d 100644
--- a/src/backend/nodes/print.c
+++ b/src/backend/nodes/print.c
@@ -38,7 +38,7 @@ print(const void *obj)
 	char	   *s;
 	char	   *f;
 
-	s = nodeToString(obj);
+	s = nodeToStringWithLocations(obj);
 	f = format_node_dump(s);
 	pfree(s);
 	printf("%s\n", f);
@@ -56,7 +56,7 @@ pprint(const void *obj)
 	char	   *s;
 	char	   *f;
 
-	s = nodeToString(obj);
+	s = nodeToStringWithLocations(obj);
 	f = pretty_format_node_dump(s);
 	pfree(s);
 	printf("%s\n", f);
@@ -74,7 +74,7 @@ elog_node_display(int lev, const char *title, const void *obj, bool pretty)
 	char	   *s;
 	char	   *f;
 
-	s = nodeToString(obj);
+	s = nodeToStringWithLocations(obj);
 	if (pretty)
 		f = pretty_format_node_dump(s);
 	else
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index fd4199a0983..76f48b13d20 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -641,7 +641,7 @@ pg_parse_query(const char *query_string)
 	 */
 #ifdef WRITE_READ_PARSE_PLAN_TREES
 	{
-		char	   *str = nodeToString(raw_parsetree_list);
+		char	   *str = nodeToStringWithLocations(raw_parsetree_list);
 		List	   *new_list = stringToNodeWithLocations(str);
 
 		pfree(str);
@@ -849,7 +849,7 @@ pg_rewrite_query(Query *query)
 		foreach(lc, querytree_list)
 		{
 			Query	   *curr_query = lfirst_node(Query, lc);
-			char	   *str = nodeToString(curr_query);
+			char	   *str = nodeToStringWithLocations(curr_query);
 			Query	   *new_query = stringToNodeWithLocations(str);
 
 			/*
@@ -931,7 +931,7 @@ pg_plan_query(Query *querytree, const char *query_string, int cursorOptions,
 		char	   *str;
 		PlannedStmt *new_plan;
 
-		str = nodeToString(plan);
+		str = nodeToStringWithLocations(plan);
 		new_plan = stringToNodeWithLocations(str);
 		pfree(str);
 
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index f7a532ea0a6..855009fd6e2 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -195,6 +195,7 @@ extern void outBitmapset(struct StringInfoData *str,
 extern void outDatum(struct StringInfoData *str, uintptr_t value,
 					 int typlen, bool typbyval);
 extern char *nodeToString(const void *obj);
+extern char *nodeToStringWithLocations(const void *obj);
 extern char *bmsToString(const struct Bitmapset *bms);
 
 /*