In the planner, delete joinaliasvars lists after we're done with them.

tglsfdc · tglsfdc · commit 896eb5efbdce · 2017-10-24T18:42:47.000-04:00
Although joinaliasvars lists coming out of the parser are quite simple, those lists can contain arbitrarily complex expressions after subquery pullup. We do not perform expression preprocessing on them, meaning that expressions in those lists will not meet the expectations of later phases of the planner (for example, that they do not contain SubLinks). This had been thought pretty harmless, since we don't intentionally touch those lists in later phases --- but Andreas Seltenreich found a case in which adjust_appendrel_attrs() could recurse into a joinaliasvars list and then die on its assertion that it never sees a SubLink. We considered a couple of localized fixes to prevent that specific case from looking at the joinaliasvars lists, but really this seems like a generic hazard for all expression processing in the planner. Therefore, probably the best answer is to delete the joinaliasvars lists from the parsetree at the end of expression preprocessing, so that there are no reachable expressions that haven't been through preprocessing. The case Andreas found seems to be harmless in non-Assert builds, and so far there are no field reports suggesting that there are user-visible effects in other cases. I considered back-patching this anyway, but it turns out that Andreas' test doesn't fail at all in 9.4-9.6, because in those versions adjust_appendrel_attrs contains code (added in commit 842faa7 and removed again in commit 215b43c) to process SubLinks rather than complain about them. Barring discovery of another path by which unprocessed joinaliasvars lists can cause trouble, the most prudent compromise seems to be to patch this into v10 but not further. Patch by me, with thanks to Amit Langote for initial investigation and review. Discussion: https://postgr.es/m/87r2tvt9f1.fsf@ansel.ydns.eu
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
@@ -776,6 +776,27 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
 		}
 	}
 
+	/*
+	 * Now that we are done preprocessing expressions, and in particular done
+	 * flattening join alias variables, get rid of the joinaliasvars lists.
+	 * They no longer match what expressions in the rest of the tree look
+	 * like, because we have not preprocessed expressions in those lists (and
+	 * do not want to; for example, expanding a SubLink there would result in
+	 * a useless unreferenced subplan).  Leaving them in place simply creates
+	 * a hazard for later scans of the tree.  We could try to prevent that by
+	 * using QTW_IGNORE_JOINALIASES in every tree scan done after this point,
+	 * but that doesn't sound very reliable.
+	 */
+	if (root->hasJoinRTEs)
+	{
+		foreach(l, parse->rtable)
+		{
+			RangeTblEntry *rte = lfirst_node(RangeTblEntry, l);
+
+			rte->joinaliasvars = NIL;
+		}
+	}
+
 	/*
 	 * In some cases we may want to transfer a HAVING clause into WHERE. We
 	 * cannot do so if the HAVING clause contains aggregates (obviously) or
@@ -902,11 +923,12 @@ preprocess_expression(PlannerInfo *root, Node *expr, int kind)
 
 	/*
 	 * If the query has any join RTEs, replace join alias variables with
-	 * base-relation variables.  We must do this before sublink processing,
-	 * else sublinks expanded out from join aliases would not get processed.
-	 * We can skip it in non-lateral RTE functions, VALUES lists, and
-	 * TABLESAMPLE clauses, however, since they can't contain any Vars of the
-	 * current query level.
+	 * base-relation variables.  We must do this first, since any expressions
+	 * we may extract from the joinaliasvars lists have not been preprocessed.
+	 * For example, if we did this after sublink processing, sublinks expanded
+	 * out from join aliases would not get processed.  But we can skip this in
+	 * non-lateral RTE functions, VALUES lists, and TABLESAMPLE clauses, since
+	 * they can't contain any Vars of the current query level.
 	 */
 	if (root->hasJoinRTEs &&
 		!(kind == EXPRKIND_RTFUNC ||
diff --git a/src/test/regress/expected/subselect.out b/src/test/regress/expected/subselect.out
@@ -678,6 +678,20 @@ with q as (select max(f1) from int4_tbl group by f1 order by f1)
  (2147483647)
 (5 rows)
 
+--
+-- Test case for sublinks pulled up into joinaliasvars lists in an
+-- inherited update/delete query
+--
+begin;  --  this shouldn't delete anything, but be safe
+delete from road
+where exists (
+  select 1
+  from
+    int4_tbl cross join
+    ( select f1, array(select q1 from int8_tbl) as arr
+      from text_tbl ) ss
+  where road.name = ss.f1 );
+rollback;
 --
 -- Test case for sublinks pushed down into subselects via join alias expansion
 --
diff --git a/src/test/regress/sql/subselect.sql b/src/test/regress/sql/subselect.sql
@@ -376,6 +376,24 @@ select q from (select max(f1) from int4_tbl group by f1 order by f1) q;
 with q as (select max(f1) from int4_tbl group by f1 order by f1)
   select q from q;
 
+--
+-- Test case for sublinks pulled up into joinaliasvars lists in an
+-- inherited update/delete query
+--
+
+begin;  --  this shouldn't delete anything, but be safe
+
+delete from road
+where exists (
+  select 1
+  from
+    int4_tbl cross join
+    ( select f1, array(select q1 from int8_tbl) as arr
+      from text_tbl ) ss
+  where road.name = ss.f1 );
+
+rollback;
+
 --
 -- Test case for sublinks pushed down into subselects via join alias expansion
 --
