From 9e54a1034a5126c52b605e13289d6e9e73b25892 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Wed, 3 Jan 2024 01:39:42 +0100 Subject: [PATCH v6 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 | 75 ++++++++++++++++----------- src/backend/rewrite/rewriteDefine.c | 4 +- src/include/nodes/nodes.h | 4 +- 13 files changed, 94 insertions(+), 52 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 348943e36c..9d2fd6fae8 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2064,9 +2064,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 @@ -3676,7 +3676,7 @@ StorePartitionKey(Relation rel, { char *exprString; - exprString = nodeToString(partexprs); + exprString = nodeToStringNoQLocs(partexprs); partexprDatum = CStringGetTextDatum(exprString); pfree(exprString); } @@ -3834,7 +3834,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 8ad81be1cd..c4fd16dc37 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,7 +819,7 @@ _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); } @@ -824,7 +830,7 @@ _outA_Const(StringInfo str, const A_Const *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(); @@ -833,18 +839,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 @@ -879,7 +885,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