Fix GEQO to not assume its join order heuristic always works.

Back in commit 400e2c934457bef4bc3cc9a3e49b6289bd761bc0 I rewrote GEQO's
gimme_tree function to improve its heuristic for modifying the given tour
into a legal join order.  In what can only be called a fit of hubris,
I supposed that this new heuristic would *always* find a legal join order,
and ripped out the old logic that allowed gimme_tree to sometimes fail.

The folly of this is exposed by bug #12760, in which the "greedy" clumping
behavior of merge_clump() can lead it into a dead end which could only be
recovered from by un-clumping.  We have no code for that and wouldn't know
exactly what to do with it if we did.  Rather than try to improve the
heuristic rules still further, let's just recognize that it *is* a
heuristic and probably must always have failure cases.  So, put back the
code removed in the previous commit to allow for failure (but comment it
a bit better this time).

It's possible that this code was actually fully correct at the time and
has only been broken by the introduction of LATERAL.  But having seen this
example I no longer have much faith in that proposition, so back-patch to
all supported branches.

diff --git a/src/backend/optimizer/geqo/geqo_eval.c b/src/backend/optimizer/geqo/geqo_eval.c
index de2a6709dd395dac8309a4f2c455e3bf88889715..5f45b65caab8ee3d2cbc6591f36bd07a9f0557c2 100644 (file)
--- a/src/backend/optimizer/geqo/geqo_eval.c
+++ b/src/backend/optimizer/geqo/geqo_eval.c
@@ -49,6 +49,9 @@ static bool desirable_join(PlannerInfo *root,
  * geqo_eval
  *
  * Returns cost of a query tree as an individual of the population.
+ *
+ * If no legal join order can be extracted from the proposed tour,
+ * returns DBL_MAX.
  */
 Cost
 geqo_eval(PlannerInfo *root, Gene *tour, int num_gene)
@@ -56,7 +59,6 @@ geqo_eval(PlannerInfo *root, Gene *tour, int num_gene)
        MemoryContext mycontext;
        MemoryContext oldcxt;
        RelOptInfo *joinrel;
-       Path       *best_path;
        Cost            fitness;
        int                     savelength;
        struct HTAB *savehash;
@@ -100,16 +102,22 @@ geqo_eval(PlannerInfo *root, Gene *tour, int num_gene)
 
        /* construct the best path for the given combination of relations */
        joinrel = gimme_tree(root, tour, num_gene);
-       best_path = joinrel->cheapest_total_path;
 
        /*
-        * compute fitness
+        * compute fitness, if we found a valid join
         *
         * XXX geqo does not currently support optimization for partial result
         * retrieval, nor do we take any cognizance of possible use of
         * parameterized paths --- how to fix?
         */
-       fitness = best_path->total_cost;
+       if (joinrel)
+       {
+               Path       *best_path = joinrel->cheapest_total_path;
+
+               fitness = best_path->total_cost;
+       }
+       else
+               fitness = DBL_MAX;
 
        /*
         * Restore join_rel_list to its former state, and put back original
@@ -134,7 +142,8 @@ geqo_eval(PlannerInfo *root, Gene *tour, int num_gene)
  *      'tour' is the proposed join order, of length 'num_gene'
  *
  * Returns a new join relation whose cheapest path is the best plan for
- * this join order.
+ * this join order.  NB: will return NULL if join order is invalid and
+ * we can't modify it into a valid order.
  *
  * The original implementation of this routine always joined in the specified
  * order, and so could only build left-sided plans (and right-sided and
@@ -147,7 +156,10 @@ geqo_eval(PlannerInfo *root, Gene *tour, int num_gene)
  * postpones joins that are illegal or seem unsuitable according to some
  * heuristic rules.  This allows correct bushy plans to be generated at need,
  * and as a nice side-effect it seems to materially improve the quality of the
- * generated plans.
+ * generated plans.  Note however that since it's just a heuristic, it can
+ * still fail in some cases.  (In particular, we might clump together
+ * relations that actually mustn't be joined yet due to LATERAL restrictions;
+ * since there's no provision for un-clumping, this must lead to failure.)
  */
 RelOptInfo *
 gimme_tree(PlannerInfo *root, Gene *tour, int num_gene)
@@ -164,9 +176,8 @@ gimme_tree(PlannerInfo *root, Gene *tour, int num_gene)
         * to; if there is none then it becomes a new clump of its own. When we
         * enlarge an existing clump we check to see if it can now be merged with
         * any other clumps.  After the tour is all scanned, we forget about the
-        * heuristics and try to forcibly join any remaining clumps.  Some forced
-        * joins might still fail due to semantics, but we should always be able
-        * to find some join order that works.
+        * heuristics and try to forcibly join any remaining clumps.  If we are
+        * unable to merge all the clumps into one, fail.
         */
        clumps = NIL;
 
@@ -208,7 +219,7 @@ gimme_tree(PlannerInfo *root, Gene *tour, int num_gene)
 
        /* Did we succeed in forming a single join relation? */
        if (list_length(clumps) != 1)
-               elog(ERROR, "failed to join all relations together");
+               return NULL;
 
        return ((Clump *) linitial(clumps))->joinrel;
 }

diff --git a/src/backend/optimizer/geqo/geqo_main.c b/src/backend/optimizer/geqo/geqo_main.c
index 579f94042af9011be601ebaeb2753eabf914c657..8928d2028c04d677bc103ee82829ac4f2a0370c7 100644 (file)
--- a/src/backend/optimizer/geqo/geqo_main.c
+++ b/src/backend/optimizer/geqo/geqo_main.c
@@ -261,6 +261,9 @@ geqo(PlannerInfo *root, int number_of_rels, List *initial_rels)
 
        best_rel = gimme_tree(root, best_tour, pool->string_length);
 
+       if (best_rel == NULL)
+               elog(ERROR, "geqo failed to make a valid plan");
+
        /* DBG: show the query plan */
 #ifdef NOT_USED
        print_plan(best_plan, root);

diff --git a/src/backend/optimizer/geqo/geqo_pool.c b/src/backend/optimizer/geqo/geqo_pool.c
index f60f3df21ea68cf2040f8d70235c5f01c61a8dff..adea22baa2b732fd5975d163f94a3671a0c43863 100644 (file)
--- a/src/backend/optimizer/geqo/geqo_pool.c
+++ b/src/backend/optimizer/geqo/geqo_pool.c
@@ -92,13 +92,37 @@ random_init_pool(PlannerInfo *root, Pool *pool)
 {
        Chromosome *chromo = (Chromosome *) pool->data;
        int                     i;
+       int                     bad = 0;
 
-       for (i = 0; i < pool->size; i++)
+       /*
+        * We immediately discard any invalid individuals (those that geqo_eval
+        * returns DBL_MAX for), thereby not wasting pool space on them.
+        *
+        * If we fail to make any valid individuals after 10000 tries, give up;
+        * this probably means something is broken, and we shouldn't just let
+        * ourselves get stuck in an infinite loop.
+        */
+       i = 0;
+       while (i < pool->size)
        {
                init_tour(root, chromo[i].string, pool->string_length);
                pool->data[i].worth = geqo_eval(root, chromo[i].string,
                                                                                pool->string_length);
+               if (pool->data[i].worth < DBL_MAX)
+                       i++;
+               else
+               {
+                       bad++;
+                       if (i == 0 && bad >= 10000)
+                               elog(ERROR, "geqo failed to make a valid plan");
+               }
        }
+
+#ifdef GEQO_DEBUG
+       if (bad > 0)
+               elog(DEBUG1, "%d invalid tours found while selecting %d pool entries",
+                        bad, pool->size);
+#endif
 }
 
 /*