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

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index c55375e7f917217b956333b0e1f941e6e44359c4..3337b77ae6d7a1b8cdf96cebfb682e29475aafc6 100644 (file)
--- 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,18 +760,46 @@ 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 d2a58a5956a247af0cf08b403f7d064f8b4d9fa6..02798f4482d04e9552d255493f5a0fa485b9ad53 100644 (file)
--- 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 fd4199a098366472bc6d3dffdce29903595e576b..76f48b13d2081cfea7d16273ee7c948cbde8de81 100644 (file)
--- 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 f7a532ea0a6d231683faa78838a7adc945cc8ece..855009fd6e208524b7faec83bfab37ff9d462873 100644 (file)
--- 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);
 
 /*