From cb70a2df99bb4286b11f0a2a22511ce92a450f1c Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Mon, 19 Oct 2026 11:41:20 +0900 Subject: [PATCH v1] 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() compares them only by phid, ignoring 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. --- src/backend/optimizer/path/equivclass.c | 11 +++-- src/backend/optimizer/plan/createplan.c | 6 ++- src/test/regress/expected/partition_join.out | 44 ++++++++++++++++++++ src/test/regress/sql/partition_join.sql | 11 +++++ 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 553d9d13dc5..e6c4c9cc52f 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/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() + * compares PlaceHolderVars only by phid. */ - if (em->em_is_child && + if ((em->em_is_child || relids != NULL) && !bms_is_subset(em->em_relids, relids)) continue; diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 2f8a08ad78e..282435788b6 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -6176,7 +6176,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 * @@ -6188,7 +6188,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; diff --git a/src/test/regress/expected/partition_join.out b/src/test/regress/expected/partition_join.out index a7e26afa793..ad051a92a5e 100644 --- a/src/test/regress/expected/partition_join.out +++ b/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); diff --git a/src/test/regress/sql/partition_join.sql b/src/test/regress/sql/partition_join.sql index 3dc3e0bbc93..31b0e8a455c 100644 --- a/src/test/regress/sql/partition_join.sql +++ b/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); -- 2.37.1 (Apple Git-137.1)