Fix planning of parameterized appendrel paths with expensive join quals.

The code in set_append_rel_pathlist() for building parameterized paths
for append relations (inheritance and UNION ALL combinations) supposed
that the cheapest regular path for a child relation would still be cheapest
when reparameterized.  Which might not be the case, particularly if the
added join conditions are expensive to compute, as in a recent example from
Jeff Janes.  Fix it to compare child path costs *after* reparameterizing.
We can short-circuit that if the cheapest pre-existing path is already
parameterized correctly, which seems likely to be true often enough to be
worth checking for.

Back-patch to 9.2 where parameterized paths were introduced.

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 5c8e5c8d1f8be33d1369bd82a7ebc6f5a7f020c1..349040237c6cb3d6a124f7a446cce0a28a1c7959 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -68,6 +68,9 @@ static void set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
 static void generate_mergeappend_paths(PlannerInfo *root, RelOptInfo *rel,
                                                   List *live_childrels,
                                                   List *all_child_pathkeys);
+static Path *get_cheapest_parameterized_child_path(PlannerInfo *root,
+                                                                         RelOptInfo *rel,
+                                                                         Relids required_outer);
 static List *accumulate_append_subpath(List *subpaths, Path *path);
 static void set_dummy_rel_pathlist(RelOptInfo *rel);
 static void set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
@@ -803,7 +806,7 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
        foreach(l, all_child_outers)
        {
                Relids          required_outer = (Relids) lfirst(l);
-               bool            ok = true;
+               bool            subpaths_valid = true;
                ListCell   *lcr;
 
                /* Select the child paths for an Append with this parameterization */
@@ -811,31 +814,21 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
                foreach(lcr, live_childrels)
                {
                        RelOptInfo *childrel = (RelOptInfo *) lfirst(lcr);
-                       Path       *cheapest_total;
+                       Path       *subpath;
 
-                       cheapest_total =
-                               get_cheapest_path_for_pathkeys(childrel->pathlist,
-                                                                                          NIL,
-                                                                                          required_outer,
-                                                                                          TOTAL_COST);
-                       Assert(cheapest_total != NULL);
-
-                       /* Children must have exactly the desired parameterization */
-                       if (!bms_equal(PATH_REQ_OUTER(cheapest_total), required_outer))
+                       subpath = get_cheapest_parameterized_child_path(root,
+                                                                                                                       childrel,
+                                                                                                                       required_outer);
+                       if (subpath == NULL)
                        {
-                               cheapest_total = reparameterize_path(root, cheapest_total,
-                                                                                                        required_outer, 1.0);
-                               if (cheapest_total == NULL)
-                               {
-                                       ok = false;
-                                       break;
-                               }
+                               /* failed to make a suitable path for this child */
+                               subpaths_valid = false;
+                               break;
                        }
-
-                       subpaths = accumulate_append_subpath(subpaths, cheapest_total);
+                       subpaths = accumulate_append_subpath(subpaths, subpath);
                }
 
-               if (ok)
+               if (subpaths_valid)
                        add_path(rel, (Path *)
                                         create_append_path(rel, subpaths, required_outer));
        }
@@ -941,6 +934,79 @@ generate_mergeappend_paths(PlannerInfo *root, RelOptInfo *rel,
        }
 }
 
+/*
+ * get_cheapest_parameterized_child_path
+ *             Get cheapest path for this relation that has exactly the requested
+ *             parameterization.
+ *
+ * Returns NULL if unable to create such a path.
+ */
+static Path *
+get_cheapest_parameterized_child_path(PlannerInfo *root, RelOptInfo *rel,
+                                                                         Relids required_outer)
+{
+       Path       *cheapest;
+       ListCell   *lc;
+
+       /*
+        * Look up the cheapest existing path with no more than the needed
+        * parameterization.  If it has exactly the needed parameterization, we're
+        * done.
+        */
+       cheapest = get_cheapest_path_for_pathkeys(rel->pathlist,
+                                                                                         NIL,
+                                                                                         required_outer,
+                                                                                         TOTAL_COST);
+       Assert(cheapest != NULL);
+       if (bms_equal(PATH_REQ_OUTER(cheapest), required_outer))
+               return cheapest;
+
+       /*
+        * Otherwise, we can "reparameterize" an existing path to match the given
+        * parameterization, which effectively means pushing down additional
+        * joinquals to be checked within the path's scan.  However, some existing
+        * paths might check the available joinquals already while others don't;
+        * therefore, it's not clear which existing path will be cheapest after
+        * reparameterization.  We have to go through them all and find out.
+        */
+       cheapest = NULL;
+       foreach(lc, rel->pathlist)
+       {
+               Path       *path = (Path *) lfirst(lc);
+
+               /* Can't use it if it needs more than requested parameterization */
+               if (!bms_is_subset(PATH_REQ_OUTER(path), required_outer))
+                       continue;
+
+               /*
+                * Reparameterization can only increase the path's cost, so if it's
+                * already more expensive than the current cheapest, forget it.
+                */
+               if (cheapest != NULL &&
+                       compare_path_costs(cheapest, path, TOTAL_COST) <= 0)
+                       continue;
+
+               /* Reparameterize if needed, then recheck cost */
+               if (!bms_equal(PATH_REQ_OUTER(path), required_outer))
+               {
+                       path = reparameterize_path(root, path, required_outer, 1.0);
+                       if (path == NULL)
+                               continue;               /* failed to reparameterize this one */
+                       Assert(bms_equal(PATH_REQ_OUTER(path), required_outer));
+
+                       if (cheapest != NULL &&
+                               compare_path_costs(cheapest, path, TOTAL_COST) <= 0)
+                               continue;
+               }
+
+               /* We have a new best path */
+               cheapest = path;
+       }
+
+       /* Return the best path, or NULL if we found no suitable candidate */
+       return cheapest;
+}
+
 /*
  * accumulate_append_subpath
  *             Add a subpath to the list being built for an Append or MergeAppend

diff --git a/src/test/regress/expected/union.out b/src/test/regress/expected/union.out
index aafa514e4356543cc103642581b8e4092a516e00..b7c4c7b312123a498d0d3baac44f8879180c03a1 100644 (file)
--- a/src/test/regress/expected/union.out
+++ b/src/test/regress/expected/union.out
@@ -605,3 +605,37 @@ WHERE x > 3;
  2 | 4
 (1 row)
 
+-- Test proper handling of parameterized appendrel paths when the
+-- potential join qual is expensive
+create function expensivefunc(int) returns int
+language plpgsql immutable strict cost 10000
+as $$begin return $1; end$$;
+create temp table t3 as select generate_series(-1000,1000) as x;
+create index t3i on t3 (expensivefunc(x));
+analyze t3;
+explain (costs off)
+select * from
+  (select * from t3 a union all select * from t3 b) ss
+  join int4_tbl on f1 = expensivefunc(x);
+                         QUERY PLAN                         
+------------------------------------------------------------
+ Nested Loop
+   ->  Seq Scan on int4_tbl
+   ->  Append
+         ->  Index Scan using t3i on t3 a
+               Index Cond: (expensivefunc(x) = int4_tbl.f1)
+         ->  Index Scan using t3i on t3 b
+               Index Cond: (expensivefunc(x) = int4_tbl.f1)
+(7 rows)
+
+select * from
+  (select * from t3 a union all select * from t3 b) ss
+  join int4_tbl on f1 = expensivefunc(x);
+ x | f1 
+---+----
+ 0 |  0
+ 0 |  0
+(2 rows)
+
+drop table t3;
+drop function expensivefunc(int);

diff --git a/src/test/regress/sql/union.sql b/src/test/regress/sql/union.sql
index f3c9d113827eb240d055bfc32a31a60d89038dbc..6194ce475110944abb69951b70c66ef368a8834a 100644 (file)
--- a/src/test/regress/sql/union.sql
+++ b/src/test/regress/sql/union.sql
@@ -249,3 +249,24 @@ SELECT * FROM
    UNION
    SELECT 2 AS t, 4 AS x) ss
 WHERE x > 3;
+
+-- Test proper handling of parameterized appendrel paths when the
+-- potential join qual is expensive
+create function expensivefunc(int) returns int
+language plpgsql immutable strict cost 10000
+as $$begin return $1; end$$;
+
+create temp table t3 as select generate_series(-1000,1000) as x;
+create index t3i on t3 (expensivefunc(x));
+analyze t3;
+
+explain (costs off)
+select * from
+  (select * from t3 a union all select * from t3 b) ss
+  join int4_tbl on f1 = expensivefunc(x);
+select * from
+  (select * from t3 a union all select * from t3 b) ss
+  join int4_tbl on f1 = expensivefunc(x);
+
+drop table t3;
+drop function expensivefunc(int);