Harden range_table_mutator() against null RangeTblEntry.subquery.

tglsfdc · tglsfdc · commit ebc584ed495d · 2022-06-26T08:58:05.000-04:00
Commit 64919aa made pull_up_simple_subquery set rte->subquery = NULL after doing the deed, so that we don't waste cycles copying a now-useless subquery tree around. This turns out to create a core dump hazard in range_table_mutator, which supposes that that field is never NULL. Apparently none of our own code invokes query_tree_mutator or range_table_mutator on the top Query after subquery pullup; but it wouldn't be surprising if outside code does, and anyway I'm working on a v16 patch that will need it. We can fix this cleanly by just getting rid of the special-case handling of this field and treating it more like all the rest. I think the special case might be left over from a time when QTW_DONT_COPY_QUERY was the default behavior, but that was eons ago. Thanks to Dean Rasheed for review. Discussion: https://postgr.es/m/545569.1656107045@sss.pgh.pa.us
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
@@ -2825,11 +2825,6 @@ expression_tree_mutator(Node *node,
 	( (newnode) = (nodetype *) palloc(sizeof(nodetype)), \
 	  memcpy((newnode), (node), sizeof(nodetype)) )
 
-#define CHECKFLATCOPY(newnode, node, nodetype)	\
-	( AssertMacro(IsA((node), nodetype)), \
-	  (newnode) = (nodetype *) palloc(sizeof(nodetype)), \
-	  memcpy((newnode), (node), sizeof(nodetype)) )
-
 #define MUTATE(newfield, oldfield, fieldtype)  \
 		( (newfield) = (fieldtype) mutator((Node *) (oldfield), context) )
 
@@ -3640,9 +3635,9 @@ expression_tree_mutator(Node *node,
  * which is the bitwise OR of flag values to suppress mutating of
  * indicated items.  (More flag bits may be added as needed.)
  *
- * Normally the Query node itself is copied, but some callers want it to be
- * modified in-place; they must pass QTW_DONT_COPY_QUERY in flags.  All
- * modified substructure is safely copied in any case.
+ * Normally the top-level Query node itself is copied, but some callers want
+ * it to be modified in-place; they must pass QTW_DONT_COPY_QUERY in flags.
+ * All modified substructure is safely copied in any case.
  */
 Query *
 query_tree_mutator(Query *query,
@@ -3758,10 +3753,7 @@ range_table_mutator(List *rtable,
 				break;
 			case RTE_SUBQUERY:
 				if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
-				{
-					CHECKFLATCOPY(newrte->subquery, rte->subquery, Query);
-					MUTATE(newrte->subquery, newrte->subquery, Query *);
-				}
+					MUTATE(newrte->subquery, rte->subquery, Query *);
 				else
 				{
 					/* else, copy RT subqueries as-is */
