From 3d92084c00a4fabf2e8b3f2a05bc93a51610a199 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 v3 2/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 "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, just omit serialization, saving several 10s of kBs.
---
 src/backend/catalog/heap.c            |  9 ++--
 src/backend/catalog/index.c           |  4 +-
 src/backend/catalog/pg_attrdef.c      |  6 ++-
 src/backend/catalog/pg_proc.c         | 10 +++-
 src/backend/catalog/pg_publication.c  |  6 ++-
 src/backend/commands/policy.c         | 11 +++-
 src/backend/commands/statscmds.c      |  2 +-
 src/backend/commands/trigger.c        |  3 +-
 src/backend/commands/typecmds.c       |  8 +--
 src/backend/nodes/gen_node_support.pl |  4 +-
 src/backend/nodes/outfuncs.c          | 77 ++++++++++++++++-----------
 src/backend/rewrite/rewriteDefine.c   |  4 +-
 src/include/nodes/nodes.h             |  4 +-
 13 files changed, 95 insertions(+), 53 deletions(-)

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 252e106cad..b0e0e8191d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2063,9 +2063,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 = nodeToStringNoQLocs(expr);
 
 	/*
 	 * Find columns of rel that are used in expr
@@ -3679,7 +3679,7 @@ StorePartitionKey(Relation rel,
 	{
 		char	   *exprString;
 
-		exprString = nodeToString(partexprs);
+		exprString = nodeToStringNoQLocs(partexprs);
 		partexprDatum = CStringGetTextDatum(exprString);
 		pfree(exprString);
 	}
@@ -3837,7 +3837,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(nodeToStringNoQLocs(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 4b88a9cb87..3d5f4e53d5 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -587,7 +587,7 @@ UpdateIndexRelation(Oid indexoid,
 	{
 		char	   *exprsString;
 
-		exprsString = nodeToString(indexInfo->ii_Expressions);
+		exprsString = nodeToStringNoQLocs(indexInfo->ii_Expressions);
 		exprsDatum = CStringGetTextDatum(exprsString);
 		pfree(exprsString);
 	}
@@ -602,7 +602,7 @@ UpdateIndexRelation(Oid indexoid,
 	{
 		char	   *predString;
 
-		predString = nodeToString(make_ands_explicit(indexInfo->ii_Predicate));
+		predString = nodeToStringNoQLocs(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..a900c9bb28 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/nodeFuncs.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 = nodeToStringNoQLocs(expr);
 
 	/*
 	 * Make the pg_attrdef entry.
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index b581d334d3..c5790a2224 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -331,7 +331,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(nodeToStringNoQLocs(parameterDefaults));
+	}
 	else
 		nulls[Anum_pg_proc_proargdefaults - 1] = true;
 	if (trftypes != PointerGetDatum(NULL))
@@ -344,7 +347,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(nodeToStringNoQLocs(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 b98b0ce0ae..b201313430 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -36,6 +36,7 @@
 #include "commands/publicationcmds.h"
 #include "funcapi.h"
 #include "miscadmin.h"
+#include "nodes/nodeFuncs.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/catcache.h"
@@ -422,7 +423,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(nodeToStringNoQLocs(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 596326e5ec..1e6842bf41 100644
--- a/src/backend/commands/policy.c
+++ b/src/backend/commands/policy.c
@@ -30,6 +30,7 @@
 #include "commands/policy.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.h"
 #include "nodes/pg_list.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_collate.h"
@@ -701,13 +702,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(nodeToStringNoQLocs(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(nodeToStringNoQLocs(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 b1a9c74bd6..58e8133f93 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -477,7 +477,7 @@ CreateStatistics(CreateStatsStmt *stmt)
 	{
 		char	   *exprsString;
 
-		exprsString = nodeToString(stxexprs);
+		exprsString = nodeToStringNoQLocs(stxexprs);
 		exprsDatum = CStringGetTextDatum(exprsString);
 		pfree(exprsString);
 	}
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index c344ff0944..6a3dc13a67 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -39,6 +39,7 @@
 #include "miscadmin.h"
 #include "nodes/bitmapset.h"
 #include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
 #include "parser/parse_clause.h"
 #include "parser/parse_collate.h"
@@ -674,7 +675,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 		/* we'll need the rtable for recordDependencyOnExpr */
 		whenRtable = pstate->p_rtable;
 
-		qual = nodeToString(whenClause);
+		qual = nodeToStringNoQLocs(whenClause);
 
 		free_parsestate(pstate);
 	}
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index e0275e5fe9..3c0e7bc5db 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -58,6 +58,7 @@
 #include "executor/executor.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
+#include "nodes/nodeFuncs.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 = nodeToStringNoQLocs(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 = nodeToStringNoQLocs(expr);
 
 	/*
 	 * Store the constraint in pg_constraint
diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index 2f0a59bc87..487f6f7728 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -921,7 +921,7 @@ foreach my $n (@node_types)
 	my $N = uc $n;
 
 	print $ofs "\t\t\tcase T_${n}:\n"
-	  . "\t\t\t\t_out${n}(str, obj);\n"
+	  . "\t\t\t\t_out${n}(str, obj, omitLocation);\n"
 	  . "\t\t\t\tbreak;\n";
 
 	print $rfs "\tif (MATCH(\"$N\", "
@@ -933,7 +933,7 @@ foreach my $n (@node_types)
 
 	print $off "
 static void
-_out${n}(StringInfo str, const $n *node)
+_out${n}(StringInfo str, const $n *node, bool omitLocation)
 {
 \tWRITE_NODE_TYPE(\"$N\");
 
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 25b0a21f79..a9b5c4d754 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -159,7 +159,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 && !omitLocation) \
 			appendStringInfo(str, " :" CppAsString(fldname) " %d", \
 							 node->fldname); \
 	} while (0)
@@ -170,7 +170,7 @@ static void outDouble(StringInfo str, double d);
 		if (node->fldname != NULL) \
 		{ \
 			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
-			outNode(str, node->fldname); \
+			outNode(str, node->fldname, omitLocation); \
 		} \
 	} while (0)
 
@@ -190,7 +190,8 @@ static void outDouble(StringInfo str, double d);
 		if (node->fldname != NULL) \
 		{ \
 			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
-			writeNodeArray(str, (const Node * const *) node->fldname, len); \
+			writeNodeArray(str, (const Node * const *) node->fldname, len, \
+						   omitLocation); \
 		} \
 	} while (0)
 
@@ -363,7 +364,8 @@ WRITE_SCALAR_ARRAY(writeBoolCols, bool, " %s", booltostr)
  * quite use appendStringInfo() in the loop.
  */
 static void
-writeNodeArray(StringInfo str, const Node *const *arr, int len)
+writeNodeArray(StringInfo str, const Node *const *arr, int len,
+			   bool omitLocation)
 {
 	if (arr != NULL)
 	{
@@ -371,7 +373,7 @@ writeNodeArray(StringInfo str, const Node *const *arr, int len)
 		for (int i = 0; i < len; i++)
 		{
 			appendStringInfoChar(str, ' ');
-			outNode(str, arr[i]);
+			outNode(str, arr[i], omitLocation);
 		}
 		appendStringInfoChar(str, ')');
 	}
@@ -383,7 +385,7 @@ writeNodeArray(StringInfo str, const Node *const *arr, int len)
  * Print a List.
  */
 static void
-_outList(StringInfo str, const List *node)
+_outList(StringInfo str, const List *node, bool omitLocation)
 {
 	const ListCell *lc;
 
@@ -405,7 +407,7 @@ _outList(StringInfo str, const List *node)
 		 */
 		if (IsA(node, List))
 		{
-			outNode(str, lfirst(lc));
+			outNode(str, lfirst(lc), omitLocation);
 			if (lnext(node, lc))
 				appendStringInfoChar(str, ' ');
 		}
@@ -490,7 +492,7 @@ outDatum(StringInfo str, Datum value, int typlen, bool typbyval)
  */
 
 static void
-_outConst(StringInfo str, const Const *node)
+_outConst(StringInfo str, const Const *node, bool omitLocation)
 {
 	WRITE_NODE_TYPE("CONST");
 
@@ -510,7 +512,7 @@ _outConst(StringInfo str, const Const *node)
 }
 
 static void
-_outBoolExpr(StringInfo str, const BoolExpr *node)
+_outBoolExpr(StringInfo str, const BoolExpr *node, bool omitLocation)
 {
 	char	   *opstr = NULL;
 
@@ -537,7 +539,8 @@ _outBoolExpr(StringInfo str, const BoolExpr *node)
 }
 
 static void
-_outForeignKeyOptInfo(StringInfo str, const ForeignKeyOptInfo *node)
+_outForeignKeyOptInfo(StringInfo str, const ForeignKeyOptInfo *node,
+					  bool omitLocation)
 {
 	int			i;
 
@@ -563,7 +566,8 @@ _outForeignKeyOptInfo(StringInfo str, const ForeignKeyOptInfo *node)
 }
 
 static void
-_outEquivalenceClass(StringInfo str, const EquivalenceClass *node)
+_outEquivalenceClass(StringInfo str, const EquivalenceClass *node,
+					 bool omitLocation)
 {
 	/*
 	 * To simplify reading, we just chase up to the topmost merged EC and
@@ -589,7 +593,8 @@ _outEquivalenceClass(StringInfo str, const EquivalenceClass *node)
 }
 
 static void
-_outExtensibleNode(StringInfo str, const ExtensibleNode *node)
+_outExtensibleNode(StringInfo str, const ExtensibleNode *node,
+				   bool omitLocation)
 {
 	const ExtensibleNodeMethods *methods;
 
@@ -604,7 +609,8 @@ _outExtensibleNode(StringInfo str, const ExtensibleNode *node)
 }
 
 static void
-_outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
+_outRangeTblEntry(StringInfo str, const RangeTblEntry *node,
+				  bool omitLocation)
 {
 	WRITE_NODE_TYPE("RANGETBLENTRY");
 
@@ -684,7 +690,7 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
 }
 
 static void
-_outA_Expr(StringInfo str, const A_Expr *node)
+_outA_Expr(StringInfo str, const A_Expr *node, bool omitLocation)
 {
 	WRITE_NODE_TYPE("A_EXPR");
 
@@ -756,13 +762,13 @@ _outA_Expr(StringInfo str, const A_Expr *node)
 }
 
 static void
-_outInteger(StringInfo str, const Integer *node)
+_outInteger(StringInfo str, const Integer *node, bool omitLocation)
 {
 	appendStringInfo(str, "%d", node->ival);
 }
 
 static void
-_outFloat(StringInfo str, const Float *node)
+_outFloat(StringInfo str, const Float *node, bool omitLocation)
 {
 	/*
 	 * We assume the value is a valid numeric literal and so does not need
@@ -772,13 +778,13 @@ _outFloat(StringInfo str, const Float *node)
 }
 
 static void
-_outBoolean(StringInfo str, const Boolean *node)
+_outBoolean(StringInfo str, const Boolean *node, bool omitLocation)
 {
 	appendStringInfoString(str, node->boolval ? "true" : "false");
 }
 
 static void
-_outString(StringInfo str, const String *node)
+_outString(StringInfo str, const String *node, bool omitLocation)
 {
 	/*
 	 * We use outToken to provide escaping of the string's content, but we
@@ -792,7 +798,7 @@ _outString(StringInfo str, const String *node)
 }
 
 static void
-_outBitString(StringInfo str, const BitString *node)
+_outBitString(StringInfo str, const BitString *node, bool omitLocation)
 {
 	/*
 	 * The lexer will always produce a string starting with 'b' or 'x'.  There
@@ -804,7 +810,7 @@ _outBitString(StringInfo str, const BitString *node)
 }
 
 static void
-_outA_Const(StringInfo str, const A_Const *node)
+_outA_Const(StringInfo str, const A_Const *node, bool omitLocation)
 {
 	WRITE_NODE_TYPE("A_CONST");
 
@@ -813,13 +819,13 @@ _outA_Const(StringInfo str, const A_Const *node)
 	else
 	{
 		appendStringInfoString(str, " :val ");
-		outNode(str, &node->val);
+		outNode(str, &node->val, omitLocation);
 	}
 	WRITE_LOCATION_FIELD(location);
 }
 
 static void
-_outConstraint(StringInfo str, const Constraint *node)
+_outConstraint(StringInfo str, const Constraint *node, bool omitLocation)
 {
 	WRITE_NODE_TYPE("CONSTRAINT");
 
@@ -952,7 +958,7 @@ _outConstraint(StringInfo str, const Constraint *node)
  *	  converts a Node into ascii string and append it to 'str'
  */
 void
-outNode(StringInfo str, const void *obj)
+outNode(StringInfo str, const void *obj, bool omitLocation)
 {
 	/* Guard against stack overflow due to overly complex expressions */
 	check_stack_depth();
@@ -961,18 +967,18 @@ outNode(StringInfo str, const void *obj)
 		appendStringInfoString(str, "<>");
 	else if (IsA(obj, List) || IsA(obj, IntList) || IsA(obj, OidList) ||
 			 IsA(obj, XidList))
-		_outList(str, obj);
+		_outList(str, obj, omitLocation);
 	/* nodeRead does not want to see { } around these! */
 	else if (IsA(obj, Integer))
-		_outInteger(str, (Integer *) obj);
+		_outInteger(str, (Integer *) obj, omitLocation);
 	else if (IsA(obj, Float))
-		_outFloat(str, (Float *) obj);
+		_outFloat(str, (Float *) obj, omitLocation);
 	else if (IsA(obj, Boolean))
-		_outBoolean(str, (Boolean *) obj);
+		_outBoolean(str, (Boolean *) obj, omitLocation);
 	else if (IsA(obj, String))
-		_outString(str, (String *) obj);
+		_outString(str, (String *) obj, omitLocation);
 	else if (IsA(obj, BitString))
-		_outBitString(str, (BitString *) obj);
+		_outBitString(str, (BitString *) obj, omitLocation);
 	else if (IsA(obj, Bitmapset))
 		outBitmapset(str, (Bitmapset *) obj);
 	else
@@ -1007,7 +1013,18 @@ nodeToString(const void *obj)
 
 	/* see stringinfo.h for an explanation of this maneuver */
 	initStringInfo(&str);
-	outNode(&str, obj);
+	outNode(&str, obj, false);
+	return str.data;
+}
+
+char *
+nodeToStringNoQLocs(const void *obj)
+{
+	StringInfoData str;
+
+	/* see stringinfo.h for an explanation of this maneuver */
+	initStringInfo(&str);
+	outNode(&str, obj, true);
 	return str.data;
 }
 
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index b449244a53..6302cd1472 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -64,8 +64,8 @@ InsertRule(const char *rulname,
 		   List *action,
 		   bool replace)
 {
-	char	   *evqual = nodeToString(event_qual);
-	char	   *actiontree = nodeToString((Node *) action);
+	char	   *evqual = nodeToStringNoQLocs(event_qual);
+	char	   *actiontree = nodeToStringNoQLocs((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..f7adb5e767 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -188,13 +188,15 @@ castNodeImpl(NodeTag type, void *ptr)
 struct Bitmapset;				/* not to include bitmapset.h here */
 struct StringInfoData;			/* not to include stringinfo.h here */
 
-extern void outNode(struct StringInfoData *str, const void *obj);
+extern void outNode(struct StringInfoData *str, const void *obj,
+					bool omitLocation);
 extern void outToken(struct StringInfoData *str, const char *s);
 extern void outBitmapset(struct StringInfoData *str,
 						 const struct Bitmapset *bms);
 extern void outDatum(struct StringInfoData *str, uintptr_t value,
 					 int typlen, bool typbyval);
 extern char *nodeToString(const void *obj);
+extern char *nodeToStringNoQLocs(const void *obj);
 extern char *bmsToString(const struct Bitmapset *bms);
 
 /*
-- 
2.40.1

