Fix eclass_useful_for_merging to give valid results for appendrel children.

Formerly, this function would always return "true" for an appendrel child
relation, because it would think that the appendrel parent was a potential
join target for the child.  In principle that should only lead to some
inefficiency in planning, but fuzz testing by Andreas Seltenreich disclosed
that it could lead to "could not find pathkey item to sort" planner errors
in odd corner cases.  Specifically, we would think that all columns of a
child table's multicolumn index were interesting pathkeys, causing us to
generate a MergeAppend path that sorts by all the columns.  However, if any
of those columns weren't actually used above the level of the appendrel,
they would not get added to that rel's targetlist, which would result in
being unable to resolve the MergeAppend's sort keys against its targetlist
during createplan.c.

Backpatch to 9.3.  In older versions, columns of an appendrel get added
to its targetlist even if they're not mentioned above the scan level,
so that the failure doesn't occur.  It might be worth back-patching this
fix to older versions anyway, but I'll refrain for the moment.

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 80021d57bdc0634a232a7519f85ef32587e28aaa..48be45dffc974a3c9818a6ba9b7e74262cd5ffa6 100644 (file)
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -2285,9 +2285,11 @@ has_relevant_eclass_joinclause(PlannerInfo *root, RelOptInfo *rel1)
  * from actually being generated.
  */
 bool
-eclass_useful_for_merging(EquivalenceClass *eclass,
+eclass_useful_for_merging(PlannerInfo *root,
+                                                 EquivalenceClass *eclass,
                                                  RelOptInfo *rel)
 {
+       Relids          relids;
        ListCell   *lc;
 
        Assert(!eclass->ec_merged);
@@ -2305,8 +2307,14 @@ eclass_useful_for_merging(EquivalenceClass *eclass,
         * possibly-overoptimistic heuristic.
         */
 
+       /* If specified rel is a child, we must consider the topmost parent rel */
+       if (rel->reloptkind == RELOPT_OTHER_MEMBER_REL)
+               relids = find_childrel_top_parent(root, rel)->relids;
+       else
+               relids = rel->relids;
+
        /* If rel already includes all members of eclass, no point in searching */
-       if (bms_is_subset(eclass->ec_relids, rel->relids))
+       if (bms_is_subset(eclass->ec_relids, relids))
                return false;
 
        /* To join, we need a member not in the given rel */
@@ -2317,7 +2325,7 @@ eclass_useful_for_merging(EquivalenceClass *eclass,
                if (cur_em->em_is_child)
                        continue;                       /* ignore children here */
 
-               if (!bms_overlap(cur_em->em_relids, rel->relids))
+               if (!bms_overlap(cur_em->em_relids, relids))
                        return true;
        }
 

diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 8b25222b93a82e80021c76ddc647089204d59694..c6b5d78724b804fba86ff37a3a2923aaa09a98b4 100644 (file)
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -1373,7 +1373,7 @@ pathkeys_useful_for_merging(PlannerInfo *root, RelOptInfo *rel, List *pathkeys)
                 * surely possible to generate a mergejoin clause using them.
                 */
                if (rel->has_eclass_joins &&
-                       eclass_useful_for_merging(pathkey->pk_eclass, rel))
+                       eclass_useful_for_merging(root, pathkey->pk_eclass, rel))
                        matched = true;
                else
                {

diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h
index 3e2378aeb7df731fd600890190f1f9fc34ceb63d..87123a57295d271f9b69a7581190721f31e41ace 100644 (file)
--- a/src/include/optimizer/paths.h
+++ b/src/include/optimizer/paths.h
@@ -146,7 +146,8 @@ extern bool have_relevant_eclass_joinclause(PlannerInfo *root,
                                                                RelOptInfo *rel1, RelOptInfo *rel2);
 extern bool has_relevant_eclass_joinclause(PlannerInfo *root,
                                                           RelOptInfo *rel1);
-extern bool eclass_useful_for_merging(EquivalenceClass *eclass,
+extern bool eclass_useful_for_merging(PlannerInfo *root,
+                                                 EquivalenceClass *eclass,
                                                  RelOptInfo *rel);
 extern bool is_redundant_derived_clause(RestrictInfo *rinfo, List *clauselist);
 

diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 56e2c9951509ce6d131432df7ac96dc07994320e..89b6c1caf47e1d71677101c0a4f82275bf6cdefe 100644 (file)
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1307,6 +1307,40 @@ DETAIL:  drop cascades to table matest1
 drop cascades to table matest2
 drop cascades to table matest3
 --
+-- Check that use of an index with an extraneous column doesn't produce
+-- a plan with extraneous sorting
+--
+create table matest0 (a int, b int, c int, d int);
+create table matest1 () inherits(matest0);
+create index matest0i on matest0 (b, c);
+create index matest1i on matest1 (b, c);
+set enable_nestloop = off;  -- we want a plan with two MergeAppends
+explain (costs off)
+select t1.* from matest0 t1, matest0 t2
+where t1.b = t2.b and t2.c = t2.d
+order by t1.b limit 10;
+                            QUERY PLAN                             
+-------------------------------------------------------------------
+ Limit
+   ->  Merge Join
+         Merge Cond: (t1.b = t2.b)
+         ->  Merge Append
+               Sort Key: t1.b
+               ->  Index Scan using matest0i on matest0 t1
+               ->  Index Scan using matest1i on matest1 t1_1
+         ->  Materialize
+               ->  Merge Append
+                     Sort Key: t2.b
+                     ->  Index Scan using matest0i on matest0 t2
+                           Filter: (c = d)
+                     ->  Index Scan using matest1i on matest1 t2_1
+                           Filter: (c = d)
+(14 rows)
+
+reset enable_nestloop;
+drop table matest0 cascade;
+NOTICE:  drop cascades to table matest1
+--
 -- Test merge-append for UNION ALL append relations
 --
 set enable_seqscan = off;

diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 09bb7507ad9eafe7a1c03db05686f2f9dad30d18..793c216376858a69b3a0c3cfb7f662f0347a8b3c 100644 (file)
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -402,6 +402,27 @@ reset enable_seqscan;
 
 drop table matest0 cascade;
 
+--
+-- Check that use of an index with an extraneous column doesn't produce
+-- a plan with extraneous sorting
+--
+
+create table matest0 (a int, b int, c int, d int);
+create table matest1 () inherits(matest0);
+create index matest0i on matest0 (b, c);
+create index matest1i on matest1 (b, c);
+
+set enable_nestloop = off;  -- we want a plan with two MergeAppends
+
+explain (costs off)
+select t1.* from matest0 t1, matest0 t2
+where t1.b = t2.b and t2.c = t2.d
+order by t1.b limit 10;
+
+reset enable_nestloop;
+
+drop table matest0 cascade;
+
 --
 -- Test merge-append for UNION ALL append relations
 --