From c74218f4301d617bb64b821d263c9c67edbfbc1b Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Tue, 2 Jan 2024 23:55:02 +0100
Subject: [PATCH v3 1/8] pg_node_tree: Omit serialization of fields with
 default values.

Often, the values in nodes are their default values. By not serializing
those fields and inserting the defaults during deserialization, we
reduce the size of pg_node_tree attributes seen in e.g. pg_rewrite by a
significant factor.

Note: Enum fields are excluded from this for now, as it would hinder
debugging.

In passing, we fix a test that had a strict dependency on the
serialization of pg_node_tree; we now do the checks in a more generic
manner, making it more stable and ensuring its stability in future work.
---
 src/backend/nodes/outfuncs.c              | 180 +++++++++++++++----
 src/backend/nodes/read.c                  |  58 ++++++
 src/backend/nodes/readfuncs.c             | 210 ++++++++++++++++------
 src/include/nodes/readfuncs.h             |   1 +
 src/test/regress/expected/rowsecurity.out |   5 +-
 src/test/regress/sql/rowsecurity.sql      |   5 +-
 6 files changed, 372 insertions(+), 87 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 25171864db..25b0a21f79 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -15,6 +15,7 @@
 #include "postgres.h"
 
 #include <ctype.h>
+#include <math.h>
 
 #include "access/attnum.h"
 #include "common/shortest_dec.h"
@@ -41,94 +42,207 @@ static void outDouble(StringInfo str, double d);
 	appendStringInfoString(str, nodelabel)
 
 /* Write an integer field (anything written as ":fldname %d") */
+#define WRITE_INT_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default) \
+			appendStringInfo(str, " :" CppAsString(fldname) " %d", \
+							 node->fldname); \
+	} while (0)
 #define WRITE_INT_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", node->fldname)
+	WRITE_INT_FIELD_DEFAULT(fldname, 0)
 
 /* Write an unsigned integer field (anything written as ":fldname %u") */
+#define WRITE_UINT_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default) \
+			appendStringInfo(str, " :" CppAsString(fldname) " %u", \
+							 node->fldname); \
+	} while (0)
 #define WRITE_UINT_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %u", node->fldname)
+	WRITE_UINT_FIELD_DEFAULT(fldname, 0)
 
 /* Write an unsigned integer field (anything written with UINT64_FORMAT) */
+#define WRITE_UINT64_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default) \
+			appendStringInfo(str, " :" CppAsString(fldname) " " UINT64_FORMAT, \
+							 node->fldname); \
+	} while (0)
 #define WRITE_UINT64_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " " UINT64_FORMAT, \
-					 node->fldname)
+	WRITE_UINT64_FIELD_DEFAULT(fldname, 0)
 
 /* Write an OID field (don't hard-wire assumption that OID is same as uint) */
+#define WRITE_OID_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default) \
+			appendStringInfo(str, " :" CppAsString(fldname) " %u", \
+							 node->fldname); \
+	} while (0)
 #define WRITE_OID_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %u", node->fldname)
+	WRITE_OID_FIELD_DEFAULT(fldname, 0)
 
 /* Write a long-integer field */
+#define WRITE_LONG_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default) \
+			appendStringInfo(str, " :" CppAsString(fldname) " %ld", \
+							 node->fldname); \
+	} while (0)
 #define WRITE_LONG_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %ld", node->fldname)
+	WRITE_LONG_FIELD_DEFAULT(fldname, 0)
+
 
 /* Write a char field (ie, one ascii character) */
+#define WRITE_CHAR_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			outChar(str, node->fldname); \
+		} \
+	} while (0)
 #define WRITE_CHAR_FIELD(fldname) \
-	(appendStringInfo(str, " :" CppAsString(fldname) " "), \
-	 outChar(str, node->fldname))
+	WRITE_CHAR_FIELD_DEFAULT(fldname, '\0')
 
 /* Write an enumerated-type field as an integer code */
+#define WRITE_ENUM_FIELD_DEFAULT(fldname, enumtype, default) \
+	do { \
+		if (node->fldname != default) \
+			appendStringInfo(str, " :" CppAsString(fldname) " %d", \
+							 (int) node->fldname); \
+	} while (0)
 #define WRITE_ENUM_FIELD(fldname, enumtype) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", \
-					 (int) node->fldname)
+	do { \
+		appendStringInfo(str, " :" CppAsString(fldname) " %d", \
+						 (int) node->fldname); \
+	} while (0)
 
 /* Write a float field (actually, they're double) */
+#define WRITE_FLOAT_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default || \
+			signbit(node->fldname) != signbit(default)) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			outDouble(str, node->fldname); \
+		} \
+	} while (0)
 #define WRITE_FLOAT_FIELD(fldname) \
-	(appendStringInfo(str, " :" CppAsString(fldname) " "), \
-	 outDouble(str, node->fldname))
+	WRITE_FLOAT_FIELD_DEFAULT(fldname, 0.0)
 
 /* Write a boolean field */
+#define WRITE_BOOL_FIELD_DEFAULT(fldname, default) \
+	do { \
+		if (node->fldname != default) \
+			appendStringInfo(str, " :" CppAsString(fldname) " %s", \
+							 booltostr(node->fldname)); \
+	} while (0)
 #define WRITE_BOOL_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %s", \
-					 booltostr(node->fldname))
+	WRITE_BOOL_FIELD_DEFAULT(fldname, false)
+
+/*
+ * Non-null defaults of by-ref types are exceedingly rare (if not generally
+ * nonexistent), so we don't (yet) have a specialized macro for non-NULL
+ * defaults omission.
+ */
 
 /* Write a character-string (possibly NULL) field */
 #define WRITE_STRING_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 outToken(str, node->fldname))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			outToken(str, node->fldname); \
+		} \
+	} while (0)
 
 /* Write a parse location field (actually same as INT case) */
 #define WRITE_LOCATION_FIELD(fldname) \
-	appendStringInfo(str, " :" CppAsString(fldname) " %d", node->fldname)
+	do { \
+		if (node->fldname != -1) \
+			appendStringInfo(str, " :" CppAsString(fldname) " %d", \
+							 node->fldname); \
+	} while (0)
 
 /* Write a Node field */
 #define WRITE_NODE_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 outNode(str, node->fldname))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			outNode(str, node->fldname); \
+		} \
+	} while (0)
 
 /* Write a bitmapset field */
 #define WRITE_BITMAPSET_FIELD(fldname) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 outBitmapset(str, node->fldname))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			outBitmapset(str, node->fldname); \
+		} \
+	} while (0)
 
 /* Write a variable-length array (not a List) of Node pointers */
 #define WRITE_NODE_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 writeNodeArray(str, (const Node * const *) node->fldname, len))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			writeNodeArray(str, (const Node * const *) node->fldname, len); \
+		} \
+	} while (0)
 
 /* Write a variable-length array of AttrNumber */
 #define WRITE_ATTRNUMBER_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 writeAttrNumberCols(str, node->fldname, len))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			writeAttrNumberCols(str, node->fldname, len); \
+		} \
+	} while (0)
 
 /* Write a variable-length array of Oid */
 #define WRITE_OID_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 writeOidCols(str, node->fldname, len))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			writeOidCols(str, node->fldname, len); \
+		} \
+	} while (0)
 
 /* Write a variable-length array of Index */
 #define WRITE_INDEX_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 writeIndexCols(str, node->fldname, len))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			writeIndexCols(str, node->fldname, len); \
+		} \
+	} while (0)
 
 /* Write a variable-length array of int */
 #define WRITE_INT_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 writeIntCols(str, node->fldname, len))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			writeIntCols(str, node->fldname, len); \
+		} \
+	} while (0)
 
 /* Write a variable-length array of bool */
 #define WRITE_BOOL_ARRAY(fldname, len) \
-	(appendStringInfoString(str, " :" CppAsString(fldname) " "), \
-	 writeBoolCols(str, node->fldname, len))
+	do { \
+		if (node->fldname != NULL) \
+		{ \
+			appendStringInfoString(str, " :" CppAsString(fldname) " "); \
+			writeBoolCols(str, node->fldname, len); \
+		} \
+	} while (0)
 
 #define booltostr(x)  ((x) ? "true" : "false")
 
diff --git a/src/backend/nodes/read.c b/src/backend/nodes/read.c
index 4eb42445c5..cffd913ba6 100644
--- a/src/backend/nodes/read.c
+++ b/src/backend/nodes/read.c
@@ -205,6 +205,64 @@ pg_strtok(int *length)
 	return ret_str;
 }
 
+/*
+ * Check if the next token is the expected field name, and if so,
+ * consume and return it.
+ *
+ * It handles similar to pg_strtok, except that it is special-cased for user-
+ * provided field names, rather than arbitrary tokens.
+ */
+const char *
+pg_strtok_fieldname(const char *fldname, int *length)
+{
+	const char *local_str;		/* working pointer to string */
+	int			expect_len;
+	char		next_char;
+
+	/* skip global state to the next token */
+	while (*pg_strtok_ptr == ' ' ||
+		   *pg_strtok_ptr == '\n' ||
+		   *pg_strtok_ptr == '\t')
+		pg_strtok_ptr++;
+
+	local_str = pg_strtok_ptr;
+
+	if (*local_str != ':')
+		return false;			/* not a field name */
+
+	expect_len = strlen(fldname);
+
+	Assert(expect_len > 0);
+
+	/* check if the next few bytes match the token */
+	if (strncmp(local_str, fldname, expect_len) != 0)
+		return false;
+
+	next_char = local_str[expect_len];
+
+	/*
+	 * Check that the token was actually terminated at the end of the
+	 * expected token with a character that is a separate token.
+	 * Otherwise, we'd get positive matches for mathing the token of "is"
+	 * against a local_str of "isn't", which is clearly wrong.
+	 */
+	if  (next_char == '\0' ||
+		 next_char == ' ' ||
+		 next_char == '\n' ||
+		 next_char == '\t' ||
+		 next_char == '(' ||
+		 next_char == ')' ||
+		 next_char == '{' ||
+		 next_char == '}')
+	{
+		pg_strtok_ptr = local_str + expect_len;
+		*length = expect_len;
+
+		return local_str;
+	}
+	return NULL;
+}
+
 /*
  * debackslash -
  *	  create a palloc'd string holding the given token.
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index cfb552fde7..96ddf0f9e2 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -57,111 +57,217 @@
 	READ_TEMP_LOCALS()
 
 /* Read an integer field (anything written as ":fldname %d") */
+#define READ_INT_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = atoi(token); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_INT_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = atoi(token)
+	READ_INT_FIELD_DEFAULT(fldname, 0)
 
 /* Read an unsigned integer field (anything written as ":fldname %u") */
+#define READ_UINT_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = atoui(token); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_UINT_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = atoui(token)
+	READ_UINT_FIELD_DEFAULT(fldname, 0)
 
 /* Read an unsigned integer field (anything written using UINT64_FORMAT) */
+#define READ_UINT64_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = strtou64(token, NULL, 10); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_UINT64_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = strtou64(token, NULL, 10)
+	READ_UINT64_FIELD_DEFAULT(fldname, 0)
 
 /* Read a long integer field (anything written as ":fldname %ld") */
+#define READ_LONG_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = atol(token); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_LONG_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = atol(token)
+	READ_LONG_FIELD_DEFAULT(fldname, 0)
 
 /* Read an OID field (don't hard-wire assumption that OID is same as uint) */
+#define READ_OID_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = atooid(token); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_OID_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = atooid(token)
+	READ_OID_FIELD_DEFAULT(fldname, 0)
 
 /* Read a char field (ie, one ascii character) */
+#define READ_CHAR_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			/* avoid overhead of calling debackslash() for one char */ \
+			local_node->fldname = (length == 0) ? '\0' : (token[0] == '\\' ? token[1] : token[0]); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_CHAR_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	/* avoid overhead of calling debackslash() for one char */ \
-	local_node->fldname = (length == 0) ? '\0' : (token[0] == '\\' ? token[1] : token[0])
+	READ_CHAR_FIELD_DEFAULT(fldname, '\0')
 
 /* Read an enumerated-type field that was written as an integer code */
 #define READ_ENUM_FIELD(fldname, enumtype) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = (enumtype) atoi(token)
+	do { \
+		token = pg_strtok_fieldname(":" CppAsString(fldname), &length); \
+		Assert(token != NULL); \
+		token = pg_strtok(&length);		/* get field value */ \
+		local_node->fldname = (enumtype) atoi(token); \
+	} while (0)
 
 /* Read a float field */
+#define READ_FLOAT_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = atof(token); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_FLOAT_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = atof(token)
+	READ_FLOAT_FIELD_DEFAULT(fldname, 0.0)
 
 /* Read a boolean field */
+#define READ_BOOL_FIELD_DEFAULT(fldname, default_value) \
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = strtobool(token); \
+		} \
+		else \
+			local_node->fldname = default_value; \
+	} while (0)
 #define READ_BOOL_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = strtobool(token)
+	READ_BOOL_FIELD_DEFAULT(fldname, false)
 
 /* Read a character-string field */
+/* see WRITE_STRING_FIELD in outfuncs.c */
 #define READ_STRING_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = nullable_string(token, length)
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = nullable_string(token, length); \
+		} \
+		else \
+			local_node->fldname = NULL; \
+	} while (0)
 
 /* Read a parse location field (and possibly throw away the value) */
 #ifdef WRITE_READ_PARSE_PLAN_TREES
 #define READ_LOCATION_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	local_node->fldname = restore_location_fields ? atoi(token) : -1
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* get field value */ \
+			local_node->fldname = restore_location_fields ? atoi(token) : -1; \
+		} \
+		local_node->fldname = -1; \
+	} while (0)
 #else
 #define READ_LOCATION_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	token = pg_strtok(&length);		/* get field value */ \
-	(void) token;				/* in case not used elsewhere */ \
-	local_node->fldname = -1	/* set field to "unknown" */
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+		{ \
+			token = pg_strtok(&length);		/* the field value is useless */ \
+			(void) token; \
+		} \
+		local_node->fldname = -1; \
+	} while (0)
 #endif
 
+
 /* Read a Node field */
 #define READ_NODE_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	(void) token;				/* in case not used elsewhere */ \
-	local_node->fldname = nodeRead(NULL, 0)
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+			local_node->fldname = nodeRead(NULL, 0); \
+		else \
+			local_node->fldname = NULL; \
+	} while (0)
 
 /* Read a bitmapset field */
 #define READ_BITMAPSET_FIELD(fldname) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	(void) token;				/* in case not used elsewhere */ \
-	local_node->fldname = _readBitmapset()
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+			local_node->fldname = _readBitmapset(); \
+		else \
+			local_node->fldname = NULL; \
+	} while (0)
 
 /* Read an attribute number array */
 #define READ_ATTRNUMBER_ARRAY(fldname, len) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	local_node->fldname = readAttrNumberCols(len)
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+			local_node->fldname = readAttrNumberCols(len); \
+		else \
+			local_node->fldname = NULL; \
+	} while (0)
 
 /* Read an oid array */
 #define READ_OID_ARRAY(fldname, len) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	local_node->fldname = readOidCols(len)
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+			local_node->fldname = readOidCols(len); \
+		else \
+			local_node->fldname = NULL; \
+	} while (0)
 
 /* Read an int array */
 #define READ_INT_ARRAY(fldname, len) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	local_node->fldname = readIntCols(len)
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+			local_node->fldname = readIntCols(len); \
+		else \
+			local_node->fldname = NULL; \
+	} while (0)
 
 /* Read a bool array */
 #define READ_BOOL_ARRAY(fldname, len) \
-	token = pg_strtok(&length);		/* skip :fldname */ \
-	local_node->fldname = readBoolCols(len)
+	do { \
+		if ((token = pg_strtok_fieldname(":" CppAsString(fldname), &length))) \
+			local_node->fldname = readBoolCols(len); \
+		else \
+			local_node->fldname = NULL; \
+	} while (0)
 
 /* Routine exit */
 #define READ_DONE() \
diff --git a/src/include/nodes/readfuncs.h b/src/include/nodes/readfuncs.h
index 8466038ed0..0579c62973 100644
--- a/src/include/nodes/readfuncs.h
+++ b/src/include/nodes/readfuncs.h
@@ -27,6 +27,7 @@ extern PGDLLIMPORT bool restore_location_fields;
  * prototypes for functions in read.c (the lisp token parser)
  */
 extern const char *pg_strtok(int *length);
+extern const char *pg_strtok_fieldname(const char *fldname, int *length);
 extern char *debackslash(const char *token, int length);
 extern void *nodeRead(const char *token, int tok_len);
 
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 6988128aa4..a69aa40f82 100644
--- a/src/test/regress/expected/rowsecurity.out
+++ b/src/test/regress/expected/rowsecurity.out
@@ -3937,7 +3937,10 @@ CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
 CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
 ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;
 GRANT SELECT ON coll_t TO regress_rls_alice;
-SELECT (string_to_array(polqual, ':'))[7] AS inputcollid FROM pg_policy WHERE polrelid = 'coll_t'::regclass;
+SELECT split AS inputcollid
+FROM pg_policy,
+     lateral unnest(string_to_array(polqual, ':')) as split
+WHERE polrelid = 'coll_t'::regclass and split LIKE '%inputcollid%';
    inputcollid    
 ------------------
  inputcollid 950 
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index dec7340538..cb7a4c776b 100644
--- a/src/test/regress/sql/rowsecurity.sql
+++ b/src/test/regress/sql/rowsecurity.sql
@@ -1732,7 +1732,10 @@ CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
 CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
 ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;
 GRANT SELECT ON coll_t TO regress_rls_alice;
-SELECT (string_to_array(polqual, ':'))[7] AS inputcollid FROM pg_policy WHERE polrelid = 'coll_t'::regclass;
+SELECT split AS inputcollid
+FROM pg_policy,
+     lateral unnest(string_to_array(polqual, ':')) as split
+WHERE polrelid = 'coll_t'::regclass and split LIKE '%inputcollid%';
 SET SESSION AUTHORIZATION regress_rls_alice;
 SELECT * FROM coll_t;
 ROLLBACK;
-- 
2.40.1

