Fix sorting a child rel by an expression containing a PlaceHolderVar

When building sort keys for a child relation, such as the per-child
sort columns of a MergeAppend over partitionwise child joins,
prepare_sort_from_pathkeys() may need to compute an EquivalenceClass
member from the child's targetlist.  find_computable_ec_member()
accepts a member if all the Vars and quasi-Vars it contains appear in
that targetlist.  It ignores child members that don't belong to the
given relids, but it can still choose a parent member.  Usually that
is harmless, since a parent member's Vars never appear in a child's
targetlist.  PlaceHolderVars are different: equal() ignores their
phexpr and phrels, so a parent PlaceHolderVar matches its translated
copy in the child's targetlist.  As a result, an untranslated parent
expression could be added to the child plan's targetlist, leading to
"variable not found in subplan target lists" errors from setrefs.c.

To fix, when relids is given, also ignore parent members that don't
belong to it.  This changes nothing for a non-child relation, because
any member computable from its targetlist belongs to it anyway.  When
relids is NULL, only child members are filtered, as before.

Also update the comments for prepare_sort_from_pathkeys() to reflect
that 'relids' can identify a non-child relation being sorted, which
has long been the case.

Back-patch to all supported branches.

Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: wenhui qiu <qiuwenhuifx@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48bLLEGy6YC+n01w6=YR=28cj_kpoKcAgOHQR7jbtLb6w@mail.gmail.com
Backpatch-through: 14

src/backend/optimizer/path/equivclass.c

@@ -982,7 +982,8 @@ find_ec_member_matching_expr(EquivalenceClass *ec,
  * compute an expression in this way, setrefs.c is going to insist on exact
  * matches of Vars to the source tlist.
  *
- * Child EC members are ignored unless they belong to given 'relids'.
+ * If 'relids' is given, EC members are ignored unless they belong to it.  If
+ * 'relids' is NULL, all child members are ignored.
  * Also, non-parallel-safe expressions are ignored if 'require_parallel_safe'.
  *
  * Note: some callers pass root == NULL for notational reasons.  This is OK
@@ -1024,9 +1025,13 @@ find_computable_ec_member(PlannerInfo *root,
 			continue;
 
 		/*
-		 * Ignore child members unless they belong to the requested rel.
+		 * Ignore child members unless they belong to the requested rel.  If
+		 * 'relids' is given, ignore parent members that don't belong to it
+		 * either: for a child rel, a parent member's PlaceHolderVars would
+		 * otherwise match the translated ones in "exprs", since equal()
+		 * ignores phexpr and phrels when comparing PlaceHolderVars.
 		 */
-		if (em->em_is_child &&
+		if ((em->em_is_child || relids != NULL) &&
 			!bms_is_subset(em->em_relids, relids))
 			continue;
 

src/backend/optimizer/plan/createplan.c

@@ -6141,7 +6141,7 @@ make_incrementalsort(Plan *lefttree, int numCols, int nPresortedCols,
  * Input parameters:
  *	  'lefttree' is the plan node which yields input tuples
  *	  'pathkeys' is the list of pathkeys by which the result is to be sorted
- *	  'relids' identifies the child relation being sorted, if any
+ *	  'relids' identifies the relation being sorted, or is NULL
  *	  'reqColIdx' is NULL or an array of required sort key column numbers
  *	  'adjust_tlist_in_place' is true if lefttree must be modified in-place
  *
@@ -6153,7 +6153,9 @@ make_incrementalsort(Plan *lefttree, int numCols, int nPresortedCols,
  * When looking for matches to an EquivalenceClass's members, we will only
  * consider child EC members if they belong to given 'relids'.  This protects
  * against possible incorrect matches to child expressions that contain no
- * Vars.
+ * Vars.  When computing a sort expression for a child relation, we will not
+ * use parent EC members either, since their PlaceHolderVars could falsely
+ * match the translated ones in the child's tlist.
  *
  * If reqColIdx isn't NULL then it contains sort key column numbers that
  * we should match.  This is used when making child plans for a MergeAppend;

src/test/regress/expected/partition_join.out

@@ -829,6 +829,50 @@ SELECT t1.a, t1.c, t2.a, t2.c FROM prt4 t1 LEFT JOIN
 
 RESET enable_hashjoin;
 RESET enable_mergejoin;
+-- bug with sorting a child join by an expression containing a PlaceHolderVar
+SET enable_seqscan TO false;
+EXPLAIN (COSTS OFF)
+SELECT s.x + 0 FROM prt1 t0 LEFT JOIN
+  (SELECT t1.a, COALESCE(t2.a, t1.b) AS x FROM prt1 t1 JOIN prt2 t2 ON t1.a = t2.b) s
+  ON true WHERE s.a = s.x + 0 AND t0.a = 0 ORDER BY 1;
+                              QUERY PLAN                              
+----------------------------------------------------------------------
+ Nested Loop
+   ->  Merge Append
+         Sort Key: ((COALESCE(t2.a, t1.b) + 0))
+         ->  Merge Join
+               Merge Cond: (t1_1.a = t2_1.b)
+               Join Filter: (t1_1.a = (COALESCE(t2_1.a, t1_1.b) + 0))
+               ->  Index Scan using iprt1_p1_a on prt1_p1 t1_1
+               ->  Index Scan using iprt2_p1_b on prt2_p1 t2_1
+         ->  Merge Join
+               Merge Cond: (t1_2.a = t2_2.b)
+               Join Filter: (t1_2.a = (COALESCE(t2_2.a, t1_2.b) + 0))
+               ->  Index Scan using iprt1_p2_a on prt1_p2 t1_2
+               ->  Index Scan using iprt2_p2_b on prt2_p2 t2_2
+         ->  Nested Loop
+               Join Filter: (t1_3.a = (COALESCE(t2_3.a, t1_3.b) + 0))
+               ->  Index Scan using iprt2_p3_b on prt2_p3 t2_3
+               ->  Index Scan using iprt1_p3_a on prt1_p3 t1_3
+                     Index Cond: (a = t2_3.b)
+   ->  Materialize
+         ->  Index Only Scan using iprt1_p1_a on prt1_p1 t0
+               Index Cond: (a = 0)
+(21 rows)
+
+SELECT s.x + 0 FROM prt1 t0 LEFT JOIN
+  (SELECT t1.a, COALESCE(t2.a, t1.b) AS x FROM prt1 t1 JOIN prt2 t2 ON t1.a = t2.b) s
+  ON true WHERE s.a = s.x + 0 AND t0.a = 0 ORDER BY 1;
+ ?column? 
+----------
+        0
+        6
+       12
+       18
+       24
+(5 rows)
+
+RESET enable_seqscan;
 -- bug in freeing the SpecialJoinInfo of a child-join
 EXPLAIN (COSTS OFF)
 SELECT * FROM prt1 t1 JOIN prt1 t2 ON t1.a = t2.a WHERE t1.a IN (SELECT a FROM prt1 t3);

src/test/regress/sql/partition_join.sql

@@ -191,6 +191,17 @@ SELECT t1.a, t1.c, t2.a, t2.c FROM prt4 t1 LEFT JOIN
 RESET enable_hashjoin;
 RESET enable_mergejoin;
 
+-- bug with sorting a child join by an expression containing a PlaceHolderVar
+SET enable_seqscan TO false;
+EXPLAIN (COSTS OFF)
+SELECT s.x + 0 FROM prt1 t0 LEFT JOIN
+  (SELECT t1.a, COALESCE(t2.a, t1.b) AS x FROM prt1 t1 JOIN prt2 t2 ON t1.a = t2.b) s
+  ON true WHERE s.a = s.x + 0 AND t0.a = 0 ORDER BY 1;
+SELECT s.x + 0 FROM prt1 t0 LEFT JOIN
+  (SELECT t1.a, COALESCE(t2.a, t1.b) AS x FROM prt1 t1 JOIN prt2 t2 ON t1.a = t2.b) s
+  ON true WHERE s.a = s.x + 0 AND t0.a = 0 ORDER BY 1;
+RESET enable_seqscan;
+
 -- bug in freeing the SpecialJoinInfo of a child-join
 EXPLAIN (COSTS OFF)
 SELECT * FROM prt1 t1 JOIN prt1 t2 ON t1.a = t2.a WHERE t1.a IN (SELECT a FROM prt1 t3);