public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: automating RangeTblEntry node support
Date: Wed, 6 Dec 2023 21:02:20 +0100
Message-ID: <[email protected]> (raw)

I have been looking into what it would take to get rid of the 
custom_read_write and custom_query_jumble for the RangeTblEntry node 
type.  This is one of the larger and more complex exceptions left.

(Similar considerations would also apply to the Constraint node type.)

Allegedly, only certain fields of RangeTblEntry are valid based on 
rtekind.  But exactly which ones seems to be documented and handled 
inconsistently.  It seems that over time, new RTE kinds have "borrowed" 
fields that notionally belong to other RTE kinds, which is technically 
not a problem but creates a bit of a mess when trying to understand all 
this.

I have some WIP patches to accompany this discussion.

Let's start with the jumble function.  I suppose that this was just 
carried over from the pg_stat_statements-specific code without any 
detailed review.  For example, the "inh" field is documented to be valid 
in all RTEs, but it's only jumbled for RTE_RELATION.  The "lateral" 
field isn't looked at at all.  I wouldn't be surprised if there are more 
cases like this.

In the first attached patch, I remove _jumbleRangeTblEntry() and instead 
add per-field query_jumble_ignore annotations to approximately match the 
behavior of the previous custom code.  The pg_stat_statements test suite 
has some coverage of this.  I get rid of switch on rtekind; this should 
be technically correct, since we do the equal and copy functions like 
this also.  So for example the "inh" field is now considered in each 
case.  But I left "lateral" alone.  I suspect several of these new 
query_jumble_ignore should actually be dropped because the code was 
wrong before.

In the second patch, I'm removing the switch on rtekind from 
range_table_mutator_impl().  This should be fine because all the 
subroutines can handle unset/NULL fields.  And it removes one more place 
that needs to track knowledge about which fields are valid when.

In the third patch, I'm removing the custom read/write functions for 
RangeTblEntry.  Those functions wanted to have a few fields at the front 
to make the dump more legible; I'm doing that now by moving the fields 
up in the actual struct.

Not done here, but something we should do is restructure the 
documentation of RangeTblEntry itself.  I'm still thinking about the 
best way to structure this, but I'm thinking more like noting for each 
field when it's used, instead by block like it is now, which makes it 
awkward if a new RTE wants to borrow some fields.

Now one could probably rightfully complain that having all these unused 
fields dumped would make the RangeTblEntry serialization bigger.  I'm 
not sure who big of a problem that actually is, considering how many 
often-unset fields other node types have.  But it deserves some 
consideration.  I think the best way to work around that would be to 
have a mode that omits fields that have their default value (zero). 
This would be more generally useful; for example Query also has a bunch 
of fields that are not often set.  I think this would be pretty easy to 
implement, for example like

#define WRITE_INT_FIELD(fldname) \
     if (full_mode || node->fldname) \
         appendStringInfo(str, " :" CppAsString(fldname) " %d", 
node->fldname)

There is also the discussion over at [0] about larger redesigns of the 
node serialization format.  I'm also interested in that, but here I'm 
mainly trying to remove more special cases to make that kind of work 
easier in the future.

Any thoughts about the direction?


[0]: 
https://www.postgresql.org/message-id/flat/CACxu%3DvL_SD%3DWJiFSJyyBuZAp_2v_XBqb1x9JBiqz52a_g9z3jA%4...
From ef08061a6d8f01e7db350b74eaa2d403df6079b9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 1 Dec 2023 11:45:50 +0100
Subject: [PATCH v1 1/3] Remove custom _jumbleRangeTblEntry()

---
 src/backend/nodes/queryjumblefuncs.c | 48 ----------------------------
 src/include/nodes/parsenodes.h       | 42 ++++++++++++------------
 2 files changed, 21 insertions(+), 69 deletions(-)

diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c
index 281907a4d8..9094ea02d8 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -347,51 +347,3 @@ _jumbleA_Const(JumbleState *jstate, Node *node)
 		}
 	}
 }
-
-static void
-_jumbleRangeTblEntry(JumbleState *jstate, Node *node)
-{
-	RangeTblEntry *expr = (RangeTblEntry *) node;
-
-	JUMBLE_FIELD(rtekind);
-	switch (expr->rtekind)
-	{
-		case RTE_RELATION:
-			JUMBLE_FIELD(relid);
-			JUMBLE_NODE(tablesample);
-			JUMBLE_FIELD(inh);
-			break;
-		case RTE_SUBQUERY:
-			JUMBLE_NODE(subquery);
-			break;
-		case RTE_JOIN:
-			JUMBLE_FIELD(jointype);
-			break;
-		case RTE_FUNCTION:
-			JUMBLE_NODE(functions);
-			break;
-		case RTE_TABLEFUNC:
-			JUMBLE_NODE(tablefunc);
-			break;
-		case RTE_VALUES:
-			JUMBLE_NODE(values_lists);
-			break;
-		case RTE_CTE:
-
-			/*
-			 * Depending on the CTE name here isn't ideal, but it's the only
-			 * info we have to identify the referenced WITH item.
-			 */
-			JUMBLE_STRING(ctename);
-			JUMBLE_FIELD(ctelevelsup);
-			break;
-		case RTE_NAMEDTUPLESTORE:
-			JUMBLE_STRING(enrname);
-			break;
-		case RTE_RESULT:
-			break;
-		default:
-			elog(ERROR, "unrecognized RTE kind: %d", (int) expr->rtekind);
-			break;
-	}
-}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e494309da8..a97f3cb16f 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1018,7 +1018,7 @@ typedef enum RTEKind
 
 typedef struct RangeTblEntry
 {
-	pg_node_attr(custom_read_write, custom_query_jumble)
+	pg_node_attr(custom_read_write)
 
 	NodeTag		type;
 
@@ -1062,16 +1062,16 @@ typedef struct RangeTblEntry
 	 * tables to be invalidated if the underlying table is altered.
 	 */
 	Oid			relid;			/* OID of the relation */
-	char		relkind;		/* relation kind (see pg_class.relkind) */
-	int			rellockmode;	/* lock level that query requires on the rel */
+	char		relkind pg_node_attr(query_jumble_ignore);		/* relation kind (see pg_class.relkind) */
+	int			rellockmode pg_node_attr(query_jumble_ignore);	/* lock level that query requires on the rel */
 	struct TableSampleClause *tablesample;	/* sampling info, or NULL */
-	Index		perminfoindex;
+	Index		perminfoindex pg_node_attr(query_jumble_ignore);
 
 	/*
 	 * Fields valid for a subquery RTE (else NULL):
 	 */
 	Query	   *subquery;		/* the sub-query */
-	bool		security_barrier;	/* is from security_barrier view? */
+	bool		security_barrier pg_node_attr(query_jumble_ignore);	/* is from security_barrier view? */
 
 	/*
 	 * Fields valid for a join RTE (else NULL/zero):
@@ -1117,17 +1117,17 @@ typedef struct RangeTblEntry
 	 * joinleftcols/joinrighttcols.
 	 */
 	JoinType	jointype;		/* type of join */
-	int			joinmergedcols; /* number of merged (JOIN USING) columns */
-	List	   *joinaliasvars;	/* list of alias-var expansions */
-	List	   *joinleftcols;	/* left-side input column numbers */
-	List	   *joinrightcols;	/* right-side input column numbers */
+	int			joinmergedcols pg_node_attr(query_jumble_ignore); /* number of merged (JOIN USING) columns */
+	List	   *joinaliasvars pg_node_attr(query_jumble_ignore);	/* list of alias-var expansions */
+	List	   *joinleftcols pg_node_attr(query_jumble_ignore);	/* left-side input column numbers */
+	List	   *joinrightcols pg_node_attr(query_jumble_ignore);	/* right-side input column numbers */
 
 	/*
 	 * join_using_alias is an alias clause attached directly to JOIN/USING. It
 	 * is different from the alias field (below) in that it does not hide the
 	 * range variables of the tables being joined.
 	 */
-	Alias	   *join_using_alias;
+	Alias	   *join_using_alias pg_node_attr(query_jumble_ignore);
 
 	/*
 	 * Fields valid for a function RTE (else NIL/zero):
@@ -1138,7 +1138,7 @@ typedef struct RangeTblEntry
 	 * expandRTE().
 	 */
 	List	   *functions;		/* list of RangeTblFunction nodes */
-	bool		funcordinality; /* is this called WITH ORDINALITY? */
+	bool		funcordinality pg_node_attr(query_jumble_ignore); /* is this called WITH ORDINALITY? */
 
 	/*
 	 * Fields valid for a TableFunc RTE (else NULL):
@@ -1155,7 +1155,7 @@ typedef struct RangeTblEntry
 	 */
 	char	   *ctename;		/* name of the WITH list item */
 	Index		ctelevelsup;	/* number of query levels up */
-	bool		self_reference; /* is this a recursive self-reference? */
+	bool		self_reference pg_node_attr(query_jumble_ignore); /* is this a recursive self-reference? */
 
 	/*
 	 * Fields valid for CTE, VALUES, ENR, and TableFunc RTEs (else NIL):
@@ -1175,25 +1175,25 @@ typedef struct RangeTblEntry
 	 * all three lists (as well as an empty-string entry in eref).  Testing
 	 * for zero coltype is the standard way to detect a dropped column.
 	 */
-	List	   *coltypes;		/* OID list of column type OIDs */
-	List	   *coltypmods;		/* integer list of column typmods */
-	List	   *colcollations;	/* OID list of column collation OIDs */
+	List	   *coltypes pg_node_attr(query_jumble_ignore);		/* OID list of column type OIDs */
+	List	   *coltypmods pg_node_attr(query_jumble_ignore);		/* integer list of column typmods */
+	List	   *colcollations pg_node_attr(query_jumble_ignore);	/* OID list of column collation OIDs */
 
 	/*
 	 * Fields valid for ENR RTEs (else NULL/zero):
 	 */
 	char	   *enrname;		/* name of ephemeral named relation */
-	Cardinality enrtuples;		/* estimated or actual from caller */
+	Cardinality enrtuples pg_node_attr(query_jumble_ignore);		/* estimated or actual from caller */
 
 	/*
 	 * Fields valid in all RTEs:
 	 */
-	Alias	   *alias;			/* user-written alias clause, if any */
-	Alias	   *eref;			/* expanded reference names */
-	bool		lateral;		/* subquery, function, or values is LATERAL? */
+	Alias	   *alias pg_node_attr(query_jumble_ignore);			/* user-written alias clause, if any */
+	Alias	   *eref pg_node_attr(query_jumble_ignore);			/* expanded reference names */
+	bool		lateral pg_node_attr(query_jumble_ignore);		/* subquery, function, or values is LATERAL? */
 	bool		inh;			/* inheritance requested? */
-	bool		inFromCl;		/* present in FROM clause? */
-	List	   *securityQuals;	/* security barrier quals to apply, if any */
+	bool		inFromCl pg_node_attr(query_jumble_ignore);		/* present in FROM clause? */
+	List	   *securityQuals pg_node_attr(query_jumble_ignore);	/* security barrier quals to apply, if any */
 } RangeTblEntry;
 
 /*
-- 
2.43.0


From 4a5efa4057ba8eddad3fe62b9a789bd3a5e03db8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 1 Dec 2023 11:54:26 +0100
Subject: [PATCH v1 2/3] Simplify range_table_mutator_impl()

---
 src/backend/nodes/nodeFuncs.c | 61 +++++++++++++----------------------
 1 file changed, 23 insertions(+), 38 deletions(-)

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index c03f4f23e2..ac195930af 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -3688,47 +3688,32 @@ range_table_mutator_impl(List *rtable,
 		RangeTblEntry *newrte;
 
 		FLATCOPY(newrte, rte, RangeTblEntry);
-		switch (rte->rtekind)
+
+		MUTATE(newrte->tablesample, rte->tablesample, TableSampleClause *);
+
+		if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
+			MUTATE(newrte->subquery, rte->subquery, Query *);
+		else
 		{
-			case RTE_RELATION:
-				MUTATE(newrte->tablesample, rte->tablesample,
-					   TableSampleClause *);
-				/* we don't bother to copy eref, aliases, etc; OK? */
-				break;
-			case RTE_SUBQUERY:
-				if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
-					MUTATE(newrte->subquery, rte->subquery, Query *);
-				else
-				{
-					/* else, copy RT subqueries as-is */
-					newrte->subquery = copyObject(rte->subquery);
-				}
-				break;
-			case RTE_JOIN:
-				if (!(flags & QTW_IGNORE_JOINALIASES))
-					MUTATE(newrte->joinaliasvars, rte->joinaliasvars, List *);
-				else
-				{
-					/* else, copy join aliases as-is */
-					newrte->joinaliasvars = copyObject(rte->joinaliasvars);
-				}
-				break;
-			case RTE_FUNCTION:
-				MUTATE(newrte->functions, rte->functions, List *);
-				break;
-			case RTE_TABLEFUNC:
-				MUTATE(newrte->tablefunc, rte->tablefunc, TableFunc *);
-				break;
-			case RTE_VALUES:
-				MUTATE(newrte->values_lists, rte->values_lists, List *);
-				break;
-			case RTE_CTE:
-			case RTE_NAMEDTUPLESTORE:
-			case RTE_RESULT:
-				/* nothing to do */
-				break;
+			/* else, copy RT subqueries as-is */
+			newrte->subquery = copyObject(rte->subquery);
+		}
+
+		if (!(flags & QTW_IGNORE_JOINALIASES))
+			MUTATE(newrte->joinaliasvars, rte->joinaliasvars, List *);
+		else
+		{
+			/* else, copy join aliases as-is */
+			newrte->joinaliasvars = copyObject(rte->joinaliasvars);
 		}
+
+		MUTATE(newrte->functions, rte->functions, List *);
+		MUTATE(newrte->tablefunc, rte->tablefunc, TableFunc *);
+		MUTATE(newrte->values_lists, rte->values_lists, List *);
 		MUTATE(newrte->securityQuals, rte->securityQuals, List *);
+
+		/* we don't bother to copy eref, aliases, etc; OK? */
+
 		newrt = lappend(newrt, newrte);
 	}
 	return newrt;
-- 
2.43.0


From 0ecfd88afc56d564b1c4c60492c80ca716694ec6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 1 Dec 2023 12:12:31 +0100
Subject: [PATCH v1 3/3] Remove custom RangeTblEntry read/write

---
 src/backend/nodes/outfuncs.c   | 80 -----------------------------
 src/backend/nodes/readfuncs.c  | 92 ----------------------------------
 src/include/nodes/parsenodes.h | 12 +++--
 3 files changed, 8 insertions(+), 176 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index e66a99247e..ded3c59b76 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -489,86 +489,6 @@ _outExtensibleNode(StringInfo str, const ExtensibleNode *node)
 	methods->nodeOut(str, node);
 }
 
-static void
-_outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
-{
-	WRITE_NODE_TYPE("RANGETBLENTRY");
-
-	/* put alias + eref first to make dump more legible */
-	WRITE_NODE_FIELD(alias);
-	WRITE_NODE_FIELD(eref);
-	WRITE_ENUM_FIELD(rtekind, RTEKind);
-
-	switch (node->rtekind)
-	{
-		case RTE_RELATION:
-			WRITE_OID_FIELD(relid);
-			WRITE_CHAR_FIELD(relkind);
-			WRITE_INT_FIELD(rellockmode);
-			WRITE_NODE_FIELD(tablesample);
-			WRITE_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_SUBQUERY:
-			WRITE_NODE_FIELD(subquery);
-			WRITE_BOOL_FIELD(security_barrier);
-			/* we re-use these RELATION fields, too: */
-			WRITE_OID_FIELD(relid);
-			WRITE_CHAR_FIELD(relkind);
-			WRITE_INT_FIELD(rellockmode);
-			WRITE_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_JOIN:
-			WRITE_ENUM_FIELD(jointype, JoinType);
-			WRITE_INT_FIELD(joinmergedcols);
-			WRITE_NODE_FIELD(joinaliasvars);
-			WRITE_NODE_FIELD(joinleftcols);
-			WRITE_NODE_FIELD(joinrightcols);
-			WRITE_NODE_FIELD(join_using_alias);
-			break;
-		case RTE_FUNCTION:
-			WRITE_NODE_FIELD(functions);
-			WRITE_BOOL_FIELD(funcordinality);
-			break;
-		case RTE_TABLEFUNC:
-			WRITE_NODE_FIELD(tablefunc);
-			break;
-		case RTE_VALUES:
-			WRITE_NODE_FIELD(values_lists);
-			WRITE_NODE_FIELD(coltypes);
-			WRITE_NODE_FIELD(coltypmods);
-			WRITE_NODE_FIELD(colcollations);
-			break;
-		case RTE_CTE:
-			WRITE_STRING_FIELD(ctename);
-			WRITE_UINT_FIELD(ctelevelsup);
-			WRITE_BOOL_FIELD(self_reference);
-			WRITE_NODE_FIELD(coltypes);
-			WRITE_NODE_FIELD(coltypmods);
-			WRITE_NODE_FIELD(colcollations);
-			break;
-		case RTE_NAMEDTUPLESTORE:
-			WRITE_STRING_FIELD(enrname);
-			WRITE_FLOAT_FIELD(enrtuples);
-			WRITE_NODE_FIELD(coltypes);
-			WRITE_NODE_FIELD(coltypmods);
-			WRITE_NODE_FIELD(colcollations);
-			/* we re-use these RELATION fields, too: */
-			WRITE_OID_FIELD(relid);
-			break;
-		case RTE_RESULT:
-			/* no extra fields */
-			break;
-		default:
-			elog(ERROR, "unrecognized RTE kind: %d", (int) node->rtekind);
-			break;
-	}
-
-	WRITE_BOOL_FIELD(lateral);
-	WRITE_BOOL_FIELD(inh);
-	WRITE_BOOL_FIELD(inFromCl);
-	WRITE_NODE_FIELD(securityQuals);
-}
-
 static void
 _outA_Expr(StringInfo str, const A_Expr *node)
 {
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index cc2021c1f7..0c64e06c38 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -486,98 +486,6 @@ _readConstraint(void)
 	READ_DONE();
 }
 
-static RangeTblEntry *
-_readRangeTblEntry(void)
-{
-	READ_LOCALS(RangeTblEntry);
-
-	/* put alias + eref first to make dump more legible */
-	READ_NODE_FIELD(alias);
-	READ_NODE_FIELD(eref);
-	READ_ENUM_FIELD(rtekind, RTEKind);
-
-	switch (local_node->rtekind)
-	{
-		case RTE_RELATION:
-			READ_OID_FIELD(relid);
-			READ_CHAR_FIELD(relkind);
-			READ_INT_FIELD(rellockmode);
-			READ_NODE_FIELD(tablesample);
-			READ_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_SUBQUERY:
-			READ_NODE_FIELD(subquery);
-			READ_BOOL_FIELD(security_barrier);
-			/* we re-use these RELATION fields, too: */
-			READ_OID_FIELD(relid);
-			READ_CHAR_FIELD(relkind);
-			READ_INT_FIELD(rellockmode);
-			READ_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_JOIN:
-			READ_ENUM_FIELD(jointype, JoinType);
-			READ_INT_FIELD(joinmergedcols);
-			READ_NODE_FIELD(joinaliasvars);
-			READ_NODE_FIELD(joinleftcols);
-			READ_NODE_FIELD(joinrightcols);
-			READ_NODE_FIELD(join_using_alias);
-			break;
-		case RTE_FUNCTION:
-			READ_NODE_FIELD(functions);
-			READ_BOOL_FIELD(funcordinality);
-			break;
-		case RTE_TABLEFUNC:
-			READ_NODE_FIELD(tablefunc);
-			/* The RTE must have a copy of the column type info, if any */
-			if (local_node->tablefunc)
-			{
-				TableFunc  *tf = local_node->tablefunc;
-
-				local_node->coltypes = tf->coltypes;
-				local_node->coltypmods = tf->coltypmods;
-				local_node->colcollations = tf->colcollations;
-			}
-			break;
-		case RTE_VALUES:
-			READ_NODE_FIELD(values_lists);
-			READ_NODE_FIELD(coltypes);
-			READ_NODE_FIELD(coltypmods);
-			READ_NODE_FIELD(colcollations);
-			break;
-		case RTE_CTE:
-			READ_STRING_FIELD(ctename);
-			READ_UINT_FIELD(ctelevelsup);
-			READ_BOOL_FIELD(self_reference);
-			READ_NODE_FIELD(coltypes);
-			READ_NODE_FIELD(coltypmods);
-			READ_NODE_FIELD(colcollations);
-			break;
-		case RTE_NAMEDTUPLESTORE:
-			READ_STRING_FIELD(enrname);
-			READ_FLOAT_FIELD(enrtuples);
-			READ_NODE_FIELD(coltypes);
-			READ_NODE_FIELD(coltypmods);
-			READ_NODE_FIELD(colcollations);
-			/* we re-use these RELATION fields, too: */
-			READ_OID_FIELD(relid);
-			break;
-		case RTE_RESULT:
-			/* no extra fields */
-			break;
-		default:
-			elog(ERROR, "unrecognized RTE kind: %d",
-				 (int) local_node->rtekind);
-			break;
-	}
-
-	READ_BOOL_FIELD(lateral);
-	READ_BOOL_FIELD(inh);
-	READ_BOOL_FIELD(inFromCl);
-	READ_NODE_FIELD(securityQuals);
-
-	READ_DONE();
-}
-
 static A_Expr *
 _readA_Expr(void)
 {
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index a97f3cb16f..d3b1af6531 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1018,8 +1018,6 @@ typedef enum RTEKind
 
 typedef struct RangeTblEntry
 {
-	pg_node_attr(custom_read_write)
-
 	NodeTag		type;
 
 	RTEKind		rtekind;		/* see above */
@@ -1030,6 +1028,14 @@ typedef struct RangeTblEntry
 	 * code that is being actively worked on.  FIXME someday.
 	 */
 
+	/*
+	 * Fields valid in all RTEs:
+	 *
+	 * put alias + eref first to make dump more legible
+	 */
+	Alias	   *alias pg_node_attr(query_jumble_ignore);			/* user-written alias clause, if any */
+	Alias	   *eref pg_node_attr(query_jumble_ignore);			/* expanded reference names */
+
 	/*
 	 * Fields valid for a plain relation RTE (else zero):
 	 *
@@ -1188,8 +1194,6 @@ typedef struct RangeTblEntry
 	/*
 	 * Fields valid in all RTEs:
 	 */
-	Alias	   *alias pg_node_attr(query_jumble_ignore);			/* user-written alias clause, if any */
-	Alias	   *eref pg_node_attr(query_jumble_ignore);			/* expanded reference names */
 	bool		lateral pg_node_attr(query_jumble_ignore);		/* subquery, function, or values is LATERAL? */
 	bool		inh;			/* inheritance requested? */
 	bool		inFromCl pg_node_attr(query_jumble_ignore);		/* present in FROM clause? */
-- 
2.43.0



Attachments:

  [text/plain] v1-0001-Remove-custom-_jumbleRangeTblEntry.patch (6.7K, ../[email protected]/2-v1-0001-Remove-custom-_jumbleRangeTblEntry.patch)
  download | inline diff:
From ef08061a6d8f01e7db350b74eaa2d403df6079b9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 1 Dec 2023 11:45:50 +0100
Subject: [PATCH v1 1/3] Remove custom _jumbleRangeTblEntry()

---
 src/backend/nodes/queryjumblefuncs.c | 48 ----------------------------
 src/include/nodes/parsenodes.h       | 42 ++++++++++++------------
 2 files changed, 21 insertions(+), 69 deletions(-)

diff --git a/src/backend/nodes/queryjumblefuncs.c b/src/backend/nodes/queryjumblefuncs.c
index 281907a4d8..9094ea02d8 100644
--- a/src/backend/nodes/queryjumblefuncs.c
+++ b/src/backend/nodes/queryjumblefuncs.c
@@ -347,51 +347,3 @@ _jumbleA_Const(JumbleState *jstate, Node *node)
 		}
 	}
 }
-
-static void
-_jumbleRangeTblEntry(JumbleState *jstate, Node *node)
-{
-	RangeTblEntry *expr = (RangeTblEntry *) node;
-
-	JUMBLE_FIELD(rtekind);
-	switch (expr->rtekind)
-	{
-		case RTE_RELATION:
-			JUMBLE_FIELD(relid);
-			JUMBLE_NODE(tablesample);
-			JUMBLE_FIELD(inh);
-			break;
-		case RTE_SUBQUERY:
-			JUMBLE_NODE(subquery);
-			break;
-		case RTE_JOIN:
-			JUMBLE_FIELD(jointype);
-			break;
-		case RTE_FUNCTION:
-			JUMBLE_NODE(functions);
-			break;
-		case RTE_TABLEFUNC:
-			JUMBLE_NODE(tablefunc);
-			break;
-		case RTE_VALUES:
-			JUMBLE_NODE(values_lists);
-			break;
-		case RTE_CTE:
-
-			/*
-			 * Depending on the CTE name here isn't ideal, but it's the only
-			 * info we have to identify the referenced WITH item.
-			 */
-			JUMBLE_STRING(ctename);
-			JUMBLE_FIELD(ctelevelsup);
-			break;
-		case RTE_NAMEDTUPLESTORE:
-			JUMBLE_STRING(enrname);
-			break;
-		case RTE_RESULT:
-			break;
-		default:
-			elog(ERROR, "unrecognized RTE kind: %d", (int) expr->rtekind);
-			break;
-	}
-}
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e494309da8..a97f3cb16f 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1018,7 +1018,7 @@ typedef enum RTEKind
 
 typedef struct RangeTblEntry
 {
-	pg_node_attr(custom_read_write, custom_query_jumble)
+	pg_node_attr(custom_read_write)
 
 	NodeTag		type;
 
@@ -1062,16 +1062,16 @@ typedef struct RangeTblEntry
 	 * tables to be invalidated if the underlying table is altered.
 	 */
 	Oid			relid;			/* OID of the relation */
-	char		relkind;		/* relation kind (see pg_class.relkind) */
-	int			rellockmode;	/* lock level that query requires on the rel */
+	char		relkind pg_node_attr(query_jumble_ignore);		/* relation kind (see pg_class.relkind) */
+	int			rellockmode pg_node_attr(query_jumble_ignore);	/* lock level that query requires on the rel */
 	struct TableSampleClause *tablesample;	/* sampling info, or NULL */
-	Index		perminfoindex;
+	Index		perminfoindex pg_node_attr(query_jumble_ignore);
 
 	/*
 	 * Fields valid for a subquery RTE (else NULL):
 	 */
 	Query	   *subquery;		/* the sub-query */
-	bool		security_barrier;	/* is from security_barrier view? */
+	bool		security_barrier pg_node_attr(query_jumble_ignore);	/* is from security_barrier view? */
 
 	/*
 	 * Fields valid for a join RTE (else NULL/zero):
@@ -1117,17 +1117,17 @@ typedef struct RangeTblEntry
 	 * joinleftcols/joinrighttcols.
 	 */
 	JoinType	jointype;		/* type of join */
-	int			joinmergedcols; /* number of merged (JOIN USING) columns */
-	List	   *joinaliasvars;	/* list of alias-var expansions */
-	List	   *joinleftcols;	/* left-side input column numbers */
-	List	   *joinrightcols;	/* right-side input column numbers */
+	int			joinmergedcols pg_node_attr(query_jumble_ignore); /* number of merged (JOIN USING) columns */
+	List	   *joinaliasvars pg_node_attr(query_jumble_ignore);	/* list of alias-var expansions */
+	List	   *joinleftcols pg_node_attr(query_jumble_ignore);	/* left-side input column numbers */
+	List	   *joinrightcols pg_node_attr(query_jumble_ignore);	/* right-side input column numbers */
 
 	/*
 	 * join_using_alias is an alias clause attached directly to JOIN/USING. It
 	 * is different from the alias field (below) in that it does not hide the
 	 * range variables of the tables being joined.
 	 */
-	Alias	   *join_using_alias;
+	Alias	   *join_using_alias pg_node_attr(query_jumble_ignore);
 
 	/*
 	 * Fields valid for a function RTE (else NIL/zero):
@@ -1138,7 +1138,7 @@ typedef struct RangeTblEntry
 	 * expandRTE().
 	 */
 	List	   *functions;		/* list of RangeTblFunction nodes */
-	bool		funcordinality; /* is this called WITH ORDINALITY? */
+	bool		funcordinality pg_node_attr(query_jumble_ignore); /* is this called WITH ORDINALITY? */
 
 	/*
 	 * Fields valid for a TableFunc RTE (else NULL):
@@ -1155,7 +1155,7 @@ typedef struct RangeTblEntry
 	 */
 	char	   *ctename;		/* name of the WITH list item */
 	Index		ctelevelsup;	/* number of query levels up */
-	bool		self_reference; /* is this a recursive self-reference? */
+	bool		self_reference pg_node_attr(query_jumble_ignore); /* is this a recursive self-reference? */
 
 	/*
 	 * Fields valid for CTE, VALUES, ENR, and TableFunc RTEs (else NIL):
@@ -1175,25 +1175,25 @@ typedef struct RangeTblEntry
 	 * all three lists (as well as an empty-string entry in eref).  Testing
 	 * for zero coltype is the standard way to detect a dropped column.
 	 */
-	List	   *coltypes;		/* OID list of column type OIDs */
-	List	   *coltypmods;		/* integer list of column typmods */
-	List	   *colcollations;	/* OID list of column collation OIDs */
+	List	   *coltypes pg_node_attr(query_jumble_ignore);		/* OID list of column type OIDs */
+	List	   *coltypmods pg_node_attr(query_jumble_ignore);		/* integer list of column typmods */
+	List	   *colcollations pg_node_attr(query_jumble_ignore);	/* OID list of column collation OIDs */
 
 	/*
 	 * Fields valid for ENR RTEs (else NULL/zero):
 	 */
 	char	   *enrname;		/* name of ephemeral named relation */
-	Cardinality enrtuples;		/* estimated or actual from caller */
+	Cardinality enrtuples pg_node_attr(query_jumble_ignore);		/* estimated or actual from caller */
 
 	/*
 	 * Fields valid in all RTEs:
 	 */
-	Alias	   *alias;			/* user-written alias clause, if any */
-	Alias	   *eref;			/* expanded reference names */
-	bool		lateral;		/* subquery, function, or values is LATERAL? */
+	Alias	   *alias pg_node_attr(query_jumble_ignore);			/* user-written alias clause, if any */
+	Alias	   *eref pg_node_attr(query_jumble_ignore);			/* expanded reference names */
+	bool		lateral pg_node_attr(query_jumble_ignore);		/* subquery, function, or values is LATERAL? */
 	bool		inh;			/* inheritance requested? */
-	bool		inFromCl;		/* present in FROM clause? */
-	List	   *securityQuals;	/* security barrier quals to apply, if any */
+	bool		inFromCl pg_node_attr(query_jumble_ignore);		/* present in FROM clause? */
+	List	   *securityQuals pg_node_attr(query_jumble_ignore);	/* security barrier quals to apply, if any */
 } RangeTblEntry;
 
 /*
-- 
2.43.0



  [text/plain] v1-0002-Simplify-range_table_mutator_impl.patch (2.6K, ../[email protected]/3-v1-0002-Simplify-range_table_mutator_impl.patch)
  download | inline diff:
From 4a5efa4057ba8eddad3fe62b9a789bd3a5e03db8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 1 Dec 2023 11:54:26 +0100
Subject: [PATCH v1 2/3] Simplify range_table_mutator_impl()

---
 src/backend/nodes/nodeFuncs.c | 61 +++++++++++++----------------------
 1 file changed, 23 insertions(+), 38 deletions(-)

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index c03f4f23e2..ac195930af 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -3688,47 +3688,32 @@ range_table_mutator_impl(List *rtable,
 		RangeTblEntry *newrte;
 
 		FLATCOPY(newrte, rte, RangeTblEntry);
-		switch (rte->rtekind)
+
+		MUTATE(newrte->tablesample, rte->tablesample, TableSampleClause *);
+
+		if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
+			MUTATE(newrte->subquery, rte->subquery, Query *);
+		else
 		{
-			case RTE_RELATION:
-				MUTATE(newrte->tablesample, rte->tablesample,
-					   TableSampleClause *);
-				/* we don't bother to copy eref, aliases, etc; OK? */
-				break;
-			case RTE_SUBQUERY:
-				if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
-					MUTATE(newrte->subquery, rte->subquery, Query *);
-				else
-				{
-					/* else, copy RT subqueries as-is */
-					newrte->subquery = copyObject(rte->subquery);
-				}
-				break;
-			case RTE_JOIN:
-				if (!(flags & QTW_IGNORE_JOINALIASES))
-					MUTATE(newrte->joinaliasvars, rte->joinaliasvars, List *);
-				else
-				{
-					/* else, copy join aliases as-is */
-					newrte->joinaliasvars = copyObject(rte->joinaliasvars);
-				}
-				break;
-			case RTE_FUNCTION:
-				MUTATE(newrte->functions, rte->functions, List *);
-				break;
-			case RTE_TABLEFUNC:
-				MUTATE(newrte->tablefunc, rte->tablefunc, TableFunc *);
-				break;
-			case RTE_VALUES:
-				MUTATE(newrte->values_lists, rte->values_lists, List *);
-				break;
-			case RTE_CTE:
-			case RTE_NAMEDTUPLESTORE:
-			case RTE_RESULT:
-				/* nothing to do */
-				break;
+			/* else, copy RT subqueries as-is */
+			newrte->subquery = copyObject(rte->subquery);
+		}
+
+		if (!(flags & QTW_IGNORE_JOINALIASES))
+			MUTATE(newrte->joinaliasvars, rte->joinaliasvars, List *);
+		else
+		{
+			/* else, copy join aliases as-is */
+			newrte->joinaliasvars = copyObject(rte->joinaliasvars);
 		}
+
+		MUTATE(newrte->functions, rte->functions, List *);
+		MUTATE(newrte->tablefunc, rte->tablefunc, TableFunc *);
+		MUTATE(newrte->values_lists, rte->values_lists, List *);
 		MUTATE(newrte->securityQuals, rte->securityQuals, List *);
+
+		/* we don't bother to copy eref, aliases, etc; OK? */
+
 		newrt = lappend(newrt, newrte);
 	}
 	return newrt;
-- 
2.43.0



  [text/plain] v1-0003-Remove-custom-RangeTblEntry-read-write.patch (6.9K, ../[email protected]/4-v1-0003-Remove-custom-RangeTblEntry-read-write.patch)
  download | inline diff:
From 0ecfd88afc56d564b1c4c60492c80ca716694ec6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Fri, 1 Dec 2023 12:12:31 +0100
Subject: [PATCH v1 3/3] Remove custom RangeTblEntry read/write

---
 src/backend/nodes/outfuncs.c   | 80 -----------------------------
 src/backend/nodes/readfuncs.c  | 92 ----------------------------------
 src/include/nodes/parsenodes.h | 12 +++--
 3 files changed, 8 insertions(+), 176 deletions(-)

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index e66a99247e..ded3c59b76 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -489,86 +489,6 @@ _outExtensibleNode(StringInfo str, const ExtensibleNode *node)
 	methods->nodeOut(str, node);
 }
 
-static void
-_outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
-{
-	WRITE_NODE_TYPE("RANGETBLENTRY");
-
-	/* put alias + eref first to make dump more legible */
-	WRITE_NODE_FIELD(alias);
-	WRITE_NODE_FIELD(eref);
-	WRITE_ENUM_FIELD(rtekind, RTEKind);
-
-	switch (node->rtekind)
-	{
-		case RTE_RELATION:
-			WRITE_OID_FIELD(relid);
-			WRITE_CHAR_FIELD(relkind);
-			WRITE_INT_FIELD(rellockmode);
-			WRITE_NODE_FIELD(tablesample);
-			WRITE_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_SUBQUERY:
-			WRITE_NODE_FIELD(subquery);
-			WRITE_BOOL_FIELD(security_barrier);
-			/* we re-use these RELATION fields, too: */
-			WRITE_OID_FIELD(relid);
-			WRITE_CHAR_FIELD(relkind);
-			WRITE_INT_FIELD(rellockmode);
-			WRITE_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_JOIN:
-			WRITE_ENUM_FIELD(jointype, JoinType);
-			WRITE_INT_FIELD(joinmergedcols);
-			WRITE_NODE_FIELD(joinaliasvars);
-			WRITE_NODE_FIELD(joinleftcols);
-			WRITE_NODE_FIELD(joinrightcols);
-			WRITE_NODE_FIELD(join_using_alias);
-			break;
-		case RTE_FUNCTION:
-			WRITE_NODE_FIELD(functions);
-			WRITE_BOOL_FIELD(funcordinality);
-			break;
-		case RTE_TABLEFUNC:
-			WRITE_NODE_FIELD(tablefunc);
-			break;
-		case RTE_VALUES:
-			WRITE_NODE_FIELD(values_lists);
-			WRITE_NODE_FIELD(coltypes);
-			WRITE_NODE_FIELD(coltypmods);
-			WRITE_NODE_FIELD(colcollations);
-			break;
-		case RTE_CTE:
-			WRITE_STRING_FIELD(ctename);
-			WRITE_UINT_FIELD(ctelevelsup);
-			WRITE_BOOL_FIELD(self_reference);
-			WRITE_NODE_FIELD(coltypes);
-			WRITE_NODE_FIELD(coltypmods);
-			WRITE_NODE_FIELD(colcollations);
-			break;
-		case RTE_NAMEDTUPLESTORE:
-			WRITE_STRING_FIELD(enrname);
-			WRITE_FLOAT_FIELD(enrtuples);
-			WRITE_NODE_FIELD(coltypes);
-			WRITE_NODE_FIELD(coltypmods);
-			WRITE_NODE_FIELD(colcollations);
-			/* we re-use these RELATION fields, too: */
-			WRITE_OID_FIELD(relid);
-			break;
-		case RTE_RESULT:
-			/* no extra fields */
-			break;
-		default:
-			elog(ERROR, "unrecognized RTE kind: %d", (int) node->rtekind);
-			break;
-	}
-
-	WRITE_BOOL_FIELD(lateral);
-	WRITE_BOOL_FIELD(inh);
-	WRITE_BOOL_FIELD(inFromCl);
-	WRITE_NODE_FIELD(securityQuals);
-}
-
 static void
 _outA_Expr(StringInfo str, const A_Expr *node)
 {
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index cc2021c1f7..0c64e06c38 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -486,98 +486,6 @@ _readConstraint(void)
 	READ_DONE();
 }
 
-static RangeTblEntry *
-_readRangeTblEntry(void)
-{
-	READ_LOCALS(RangeTblEntry);
-
-	/* put alias + eref first to make dump more legible */
-	READ_NODE_FIELD(alias);
-	READ_NODE_FIELD(eref);
-	READ_ENUM_FIELD(rtekind, RTEKind);
-
-	switch (local_node->rtekind)
-	{
-		case RTE_RELATION:
-			READ_OID_FIELD(relid);
-			READ_CHAR_FIELD(relkind);
-			READ_INT_FIELD(rellockmode);
-			READ_NODE_FIELD(tablesample);
-			READ_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_SUBQUERY:
-			READ_NODE_FIELD(subquery);
-			READ_BOOL_FIELD(security_barrier);
-			/* we re-use these RELATION fields, too: */
-			READ_OID_FIELD(relid);
-			READ_CHAR_FIELD(relkind);
-			READ_INT_FIELD(rellockmode);
-			READ_UINT_FIELD(perminfoindex);
-			break;
-		case RTE_JOIN:
-			READ_ENUM_FIELD(jointype, JoinType);
-			READ_INT_FIELD(joinmergedcols);
-			READ_NODE_FIELD(joinaliasvars);
-			READ_NODE_FIELD(joinleftcols);
-			READ_NODE_FIELD(joinrightcols);
-			READ_NODE_FIELD(join_using_alias);
-			break;
-		case RTE_FUNCTION:
-			READ_NODE_FIELD(functions);
-			READ_BOOL_FIELD(funcordinality);
-			break;
-		case RTE_TABLEFUNC:
-			READ_NODE_FIELD(tablefunc);
-			/* The RTE must have a copy of the column type info, if any */
-			if (local_node->tablefunc)
-			{
-				TableFunc  *tf = local_node->tablefunc;
-
-				local_node->coltypes = tf->coltypes;
-				local_node->coltypmods = tf->coltypmods;
-				local_node->colcollations = tf->colcollations;
-			}
-			break;
-		case RTE_VALUES:
-			READ_NODE_FIELD(values_lists);
-			READ_NODE_FIELD(coltypes);
-			READ_NODE_FIELD(coltypmods);
-			READ_NODE_FIELD(colcollations);
-			break;
-		case RTE_CTE:
-			READ_STRING_FIELD(ctename);
-			READ_UINT_FIELD(ctelevelsup);
-			READ_BOOL_FIELD(self_reference);
-			READ_NODE_FIELD(coltypes);
-			READ_NODE_FIELD(coltypmods);
-			READ_NODE_FIELD(colcollations);
-			break;
-		case RTE_NAMEDTUPLESTORE:
-			READ_STRING_FIELD(enrname);
-			READ_FLOAT_FIELD(enrtuples);
-			READ_NODE_FIELD(coltypes);
-			READ_NODE_FIELD(coltypmods);
-			READ_NODE_FIELD(colcollations);
-			/* we re-use these RELATION fields, too: */
-			READ_OID_FIELD(relid);
-			break;
-		case RTE_RESULT:
-			/* no extra fields */
-			break;
-		default:
-			elog(ERROR, "unrecognized RTE kind: %d",
-				 (int) local_node->rtekind);
-			break;
-	}
-
-	READ_BOOL_FIELD(lateral);
-	READ_BOOL_FIELD(inh);
-	READ_BOOL_FIELD(inFromCl);
-	READ_NODE_FIELD(securityQuals);
-
-	READ_DONE();
-}
-
 static A_Expr *
 _readA_Expr(void)
 {
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index a97f3cb16f..d3b1af6531 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1018,8 +1018,6 @@ typedef enum RTEKind
 
 typedef struct RangeTblEntry
 {
-	pg_node_attr(custom_read_write)
-
 	NodeTag		type;
 
 	RTEKind		rtekind;		/* see above */
@@ -1030,6 +1028,14 @@ typedef struct RangeTblEntry
 	 * code that is being actively worked on.  FIXME someday.
 	 */
 
+	/*
+	 * Fields valid in all RTEs:
+	 *
+	 * put alias + eref first to make dump more legible
+	 */
+	Alias	   *alias pg_node_attr(query_jumble_ignore);			/* user-written alias clause, if any */
+	Alias	   *eref pg_node_attr(query_jumble_ignore);			/* expanded reference names */
+
 	/*
 	 * Fields valid for a plain relation RTE (else zero):
 	 *
@@ -1188,8 +1194,6 @@ typedef struct RangeTblEntry
 	/*
 	 * Fields valid in all RTEs:
 	 */
-	Alias	   *alias pg_node_attr(query_jumble_ignore);			/* user-written alias clause, if any */
-	Alias	   *eref pg_node_attr(query_jumble_ignore);			/* expanded reference names */
 	bool		lateral pg_node_attr(query_jumble_ignore);		/* subquery, function, or values is LATERAL? */
 	bool		inh;			/* inheritance requested? */
 	bool		inFromCl pg_node_attr(query_jumble_ignore);		/* present in FROM clause? */
-- 
2.43.0



view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: automating RangeTblEntry node support
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox