From 36be38efe8e710a745b1f2a211555e56b57cb04c Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Wed, 3 Jan 2024 01:39:42 +0100
Subject: [PATCH v8 3/8] pg_node_tree: Don't store query text locations in
 pg_node_tree fields.

We don't store original query texts, so any lingering query location
value can only be useful in forensic debugging. In normal operation,
however, a non-default value will show up as measurable overhead in
serialization, so by omitting serialization we save several 10s of
kBs in (amongst others) pg_rewrite.
---
 src/backend/catalog/heap.c           | 10 ++++++----
 src/backend/catalog/index.c          |  5 +++--
 src/backend/catalog/pg_attrdef.c     |  6 ++++--
 src/backend/catalog/pg_proc.c        | 11 +++++++++--
 src/backend/catalog/pg_publication.c |  6 +++++-
 src/backend/commands/policy.c        | 11 +++++++++--
 src/backend/commands/statscmds.c     |  3 ++-
 src/backend/commands/trigger.c       |  3 ++-
 src/backend/commands/typecmds.c      |  8 +++++---
 src/backend/nodes/outfuncs.c         | 20 +++++++++++++++++++-
 src/backend/rewrite/rewriteDefine.c  |  5 +++--
 src/include/nodes/nodes.h            |  1 +
 12 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 5e773740f4..c729196538 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -58,6 +58,7 @@
 #include "commands/typecmds.h"
 #include "common/int.h"
 #include "miscadmin.h"
+#include "nodes/nodes.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
 #include "parser/parse_coerce.h"
@@ -2064,9 +2065,9 @@ StoreRelCheck(Relation rel, const char *ccname, Node *expr,
 	Oid			constrOid;
 
 	/*
-	 * Flatten expression to string form for storage.
+	 * Flatten expression to string form for storage, without query refs.
 	 */
-	ccbin = nodeToString(expr);
+	ccbin = nodeToStringNoParseLocs(expr);
 
 	/*
 	 * Find columns of rel that are used in expr
@@ -3676,7 +3677,7 @@ StorePartitionKey(Relation rel,
 	{
 		char	   *exprString;
 
-		exprString = nodeToString(partexprs);
+		exprString = nodeToStringNoParseLocs(partexprs);
 		partexprDatum = CStringGetTextDatum(exprString);
 		pfree(exprString);
 	}
@@ -3834,7 +3835,8 @@ StorePartitionBound(Relation rel, Relation parent, PartitionBoundSpec *bound)
 	memset(new_val, 0, sizeof(new_val));
 	memset(new_null, false, sizeof(new_null));
 	memset(new_repl, false, sizeof(new_repl));
-	new_val[Anum_pg_class_relpartbound - 1] = CStringGetTextDatum(nodeToString(bound));
+	new_val[Anum_pg_class_relpartbound - 1] = 
+		CStringGetTextDatum(nodeToStringNoParseLocs(bound));
 	new_null[Anum_pg_class_relpartbound - 1] = false;
 	new_repl[Anum_pg_class_relpartbound - 1] = true;
 	newtuple = heap_modify_tuple(tuple, RelationGetDescr(classRel),
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index e6140853c0..a796d4e8b1 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -59,6 +59,7 @@
 #include "executor/executor.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
+#include "nodes/nodes.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
 #include "parser/parser.h"
@@ -582,7 +583,7 @@ UpdateIndexRelation(Oid indexoid,
 	{
 		char	   *exprsString;
 
-		exprsString = nodeToString(indexInfo->ii_Expressions);
+		exprsString = nodeToStringNoParseLocs(indexInfo->ii_Expressions);
 		exprsDatum = CStringGetTextDatum(exprsString);
 		pfree(exprsString);
 	}
@@ -597,7 +598,7 @@ UpdateIndexRelation(Oid indexoid,
 	{
 		char	   *predString;
 
-		predString = nodeToString(make_ands_explicit(indexInfo->ii_Predicate));
+		predString = nodeToStringNoParseLocs(make_ands_explicit(indexInfo->ii_Predicate));
 		predDatum = CStringGetTextDatum(predString);
 		pfree(predString);
 	}
diff --git a/src/backend/catalog/pg_attrdef.c b/src/backend/catalog/pg_attrdef.c
index 003ae70b4d..685399b2be 100644
--- a/src/backend/catalog/pg_attrdef.c
+++ b/src/backend/catalog/pg_attrdef.c
@@ -23,6 +23,7 @@
 #include "catalog/objectaccess.h"
 #include "catalog/pg_attrdef.h"
 #include "executor/executor.h"
+#include "nodes/nodes.h"
 #include "optimizer/optimizer.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
@@ -62,9 +63,10 @@ StoreAttrDefault(Relation rel, AttrNumber attnum,
 	adrel = table_open(AttrDefaultRelationId, RowExclusiveLock);
 
 	/*
-	 * Flatten expression to string form for storage.
+	 * Flatten expression to string form for storage, without references to
+	 * the original query string.
 	 */
-	adbin = nodeToString(expr);
+	adbin = nodeToStringNoParseLocs(expr);
 
 	/*
 	 * Make the pg_attrdef entry.
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index ab2b6ca148..9317aa3c0c 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -31,6 +31,7 @@
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
+#include "nodes/nodes.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parse_coerce.h"
 #include "pgstat.h"
@@ -329,7 +330,10 @@ ProcedureCreate(const char *procedureName,
 	else
 		nulls[Anum_pg_proc_proargnames - 1] = true;
 	if (parameterDefaults != NIL)
-		values[Anum_pg_proc_proargdefaults - 1] = CStringGetTextDatum(nodeToString(parameterDefaults));
+	{
+		values[Anum_pg_proc_proargdefaults - 1] =
+			CStringGetTextDatum(nodeToStringNoParseLocs(parameterDefaults));
+	}
 	else
 		nulls[Anum_pg_proc_proargdefaults - 1] = true;
 	if (trftypes != PointerGetDatum(NULL))
@@ -342,7 +346,10 @@ ProcedureCreate(const char *procedureName,
 	else
 		nulls[Anum_pg_proc_probin - 1] = true;
 	if (prosqlbody)
-		values[Anum_pg_proc_prosqlbody - 1] = CStringGetTextDatum(nodeToString(prosqlbody));
+	{
+		values[Anum_pg_proc_prosqlbody - 1] =
+			CStringGetTextDatum(nodeToStringNoParseLocs(prosqlbody));
+	}
 	else
 		nulls[Anum_pg_proc_prosqlbody - 1] = true;
 	if (proconfig != PointerGetDatum(NULL))
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index ac05dc057f..a5647d5cd2 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -32,6 +32,7 @@
 #include "catalog/pg_type.h"
 #include "commands/publicationcmds.h"
 #include "funcapi.h"
+#include "nodes/nodes.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/catcache.h"
@@ -417,7 +418,10 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
 
 	/* Add qualifications, if available */
 	if (pri->whereClause != NULL)
-		values[Anum_pg_publication_rel_prqual - 1] = CStringGetTextDatum(nodeToString(pri->whereClause));
+	{
+		values[Anum_pg_publication_rel_prqual - 1] =
+			CStringGetTextDatum(nodeToStringNoParseLocs(pri->whereClause));
+	}
 	else
 		nulls[Anum_pg_publication_rel_prqual - 1] = true;
 
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 6ff3eba824..43bf0bd829 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -28,6 +28,7 @@
 #include "catalog/pg_type.h"
 #include "commands/policy.h"
 #include "miscadmin.h"
+#include "nodes/nodes.h"
 #include "nodes/pg_list.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_collate.h"
@@ -698,13 +699,19 @@ CreatePolicy(CreatePolicyStmt *stmt)
 
 	/* Add qual if present. */
 	if (qual)
-		values[Anum_pg_policy_polqual - 1] = CStringGetTextDatum(nodeToString(qual));
+	{
+		values[Anum_pg_policy_polqual - 1] =
+			CStringGetTextDatum(nodeToStringNoParseLocs(qual));
+	}
 	else
 		isnull[Anum_pg_policy_polqual - 1] = true;
 
 	/* Add WITH CHECK qual if present */
 	if (with_check_qual)
-		values[Anum_pg_policy_polwithcheck - 1] = CStringGetTextDatum(nodeToString(with_check_qual));
+	{
+		values[Anum_pg_policy_polwithcheck - 1] =
+			CStringGetTextDatum(nodeToStringNoParseLocs(with_check_qual));
+	}
 	else
 		isnull[Anum_pg_policy_polwithcheck - 1] = true;
 
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 6fa840fada..bdf0fda82b 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -27,6 +27,7 @@
 #include "commands/comment.h"
 #include "commands/defrem.h"
 #include "miscadmin.h"
+#include "nodes/nodes.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
 #include "statistics/statistics.h"
@@ -474,7 +475,7 @@ CreateStatistics(CreateStatsStmt *stmt)
 	{
 		char	   *exprsString;
 
-		exprsString = nodeToString(stxexprs);
+		exprsString = nodeToStringNoParseLocs(stxexprs);
 		exprsDatum = CStringGetTextDatum(exprsString);
 		pfree(exprsString);
 	}
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 35eb7180f7..07be4f1994 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -36,6 +36,7 @@
 #include "miscadmin.h"
 #include "nodes/bitmapset.h"
 #include "nodes/makefuncs.h"
+#include "nodes/nodes.h"
 #include "optimizer/optimizer.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_collate.h"
@@ -667,7 +668,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 		/* we'll need the rtable for recordDependencyOnExpr */
 		whenRtable = pstate->p_rtable;
 
-		qual = nodeToString(whenClause);
+		qual = nodeToStringNoParseLocs(whenClause);
 
 		free_parsestate(pstate);
 	}
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index f4cdec3bf2..04fa5587c6 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -59,6 +59,7 @@
 #include "executor/executor.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
+#include "nodes/nodes.h"
 #include "optimizer/optimizer.h"
 #include "parser/parse_coerce.h"
 #include "parser/parse_collate.h"
@@ -924,7 +925,7 @@ DefineDomain(CreateDomainStmt *stmt)
 						defaultValue =
 							deparse_expression(defaultExpr,
 											   NIL, false, false);
-						defaultValueBin = nodeToString(defaultExpr);
+						defaultValueBin = nodeToStringNoParseLocs(defaultExpr);
 					}
 				}
 				else
@@ -3506,9 +3507,10 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
 				 errmsg("cannot use table references in domain check constraint")));
 
 	/*
-	 * Convert to string form for storage.
+	 * Convert to string form for storage, without references to the original
+	 * query text.
 	 */
-	ccbin = nodeToString(expr);
+	ccbin = nodeToStringNoParseLocs(expr);
 
 	/*
 	 * Store the constraint in pg_constraint
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 70123ea39c..b03773b7fe 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -29,6 +29,7 @@
 static void outChar(StringInfo str, char c);
 static void outDouble(StringInfo str, double d);
 
+static bool output_parse_locations = true;
 
 /*
  * Macros to simplify output of different kinds of fields.  Use these
@@ -159,7 +160,7 @@ static void outDouble(StringInfo str, double d);
 /* Write a parse location field (actually same as INT case) */
 #define WRITE_LOCATION_FIELD(fldname) \
 	do { \
-		if (node->fldname != -1) \
+		if (node->fldname != -1 && output_parse_locations) \
 			appendStringInfo(str, " :" CppAsString(fldname) " %d", \
 							 node->fldname); \
 	} while (0)
@@ -877,10 +878,27 @@ char *
 nodeToString(const void *obj)
 {
 	StringInfoData str;
+	bool save_output_parse_locations = output_parse_locations;
+	output_parse_locations = true;
 
 	/* see stringinfo.h for an explanation of this maneuver */
 	initStringInfo(&str);
 	outNode(&str, obj);
+	output_parse_locations = save_output_parse_locations;
+	return str.data;
+}
+
+char *
+nodeToStringNoParseLocs(const void *obj)
+{
+	StringInfoData str;
+	bool save_output_parse_locations = output_parse_locations;
+	output_parse_locations = false;
+
+	/* see stringinfo.h for an explanation of this maneuver */
+	initStringInfo(&str);
+	outNode(&str, obj);
+	output_parse_locations = save_output_parse_locations;
 	return str.data;
 }
 
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 6cc9a8d8bf..f1ef7130b5 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -25,6 +25,7 @@
 #include "catalog/pg_rewrite.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
+#include "nodes/nodes.h"
 #include "parser/parse_utilcmd.h"
 #include "rewrite/rewriteDefine.h"
 #include "rewrite/rewriteManip.h"
@@ -57,8 +58,8 @@ InsertRule(const char *rulname,
 		   List *action,
 		   bool replace)
 {
-	char	   *evqual = nodeToString(event_qual);
-	char	   *actiontree = nodeToString((Node *) action);
+	char	   *evqual = nodeToStringNoParseLocs(event_qual);
+	char	   *actiontree = nodeToStringNoParseLocs((Node *) action);
 	Datum		values[Natts_pg_rewrite];
 	bool		nulls[Natts_pg_rewrite] = {0};
 	NameData	rname;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 2969dd831b..3dd52d5cad 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 *nodeToStringNoParseLocs(const void *obj);
 extern char *bmsToString(const struct Bitmapset *bms);
 
 /*
-- 
2.40.1

