Postpone calculating total_table_pages until after pruning/exclusion.

The planner doesn't make any use of root->total_table_pages until it
estimates costs of indexscans, so we don't need to compute it as
early as that's currently done.  By doing the calculation between
set_base_rel_sizes and set_base_rel_pathlists, we can omit relations
that get removed from the query by partition pruning or constraint
exclusion, which seems like a more accurate basis for costing.

(Historical note: I think at the time this code was written, there
was not a separation between the "set sizes" and "set pathlists"
steps, so that this approach would have been impossible at the time.
But now that we have that separation, this is clearly the better way
to do things.)

David Rowley, reviewed by Edmund Horner

Discussion: https://postgr.es/m/CAKJS1f-NG1mRM0VOtkAG7=ZLQWihoqees9R4ki3CKBB0-fRfCA@mail.gmail.com

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 5f74d3b36d9c7c28a4db08d15d812a3b9b203314..738bb30848455159cfd678a412ebecf1affa6055 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -147,6 +147,7 @@ make_one_rel(PlannerInfo *root, List *joinlist)
 {
        RelOptInfo *rel;
        Index           rti;
+       double          total_pages;
 
        /*
         * Construct the all_baserels Relids set.
@@ -173,10 +174,45 @@ make_one_rel(PlannerInfo *root, List *joinlist)
        set_base_rel_consider_startup(root);
 
        /*
-        * Compute size estimates and consider_parallel flags for each base rel,
-        * then generate access paths.
+        * Compute size estimates and consider_parallel flags for each base rel.
         */
        set_base_rel_sizes(root);
+
+       /*
+        * We should now have size estimates for every actual table involved in
+        * the query, and we also know which if any have been deleted from the
+        * query by join removal, pruned by partition pruning, or eliminated by
+        * constraint exclusion.  So we can now compute total_table_pages.
+        *
+        * Note that appendrels are not double-counted here, even though we don't
+        * bother to distinguish RelOptInfos for appendrel parents, because the
+        * parents will have pages = 0.
+        *
+        * XXX if a table is self-joined, we will count it once per appearance,
+        * which perhaps is the wrong thing ... but that's not completely clear,
+        * and detecting self-joins here is difficult, so ignore it for now.
+        */
+       total_pages = 0;
+       for (rti = 1; rti < root->simple_rel_array_size; rti++)
+       {
+               RelOptInfo *brel = root->simple_rel_array[rti];
+
+               if (brel == NULL)
+                       continue;
+
+               Assert(brel->relid == rti); /* sanity check on array */
+
+               if (IS_DUMMY_REL(brel))
+                       continue;
+
+               if (IS_SIMPLE_REL(brel))
+                       total_pages += (double) brel->pages;
+       }
+       root->total_table_pages = total_pages;
+
+       /*
+        * Generate access paths for each base rel.
+        */
        set_base_rel_pathlists(root);
 
        /*
@@ -1271,6 +1307,11 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
                 * because some places assume rel->tuples is valid for any baserel.
                 */
                rel->tuples = parent_rows;
+
+               /*
+                * Note that we leave rel->pages as zero; this is important to avoid
+                * double-counting the appendrel tree in total_table_pages.
+                */
        }
        else
        {

diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index b05adc70c4a7f59bf03da1211012f10ce6584f06..9b6cc9e10f00306e7e2cefc398d4377a969bea31 100644 (file)
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -57,8 +57,6 @@ query_planner(PlannerInfo *root, List *tlist,
        Query      *parse = root->parse;
        List       *joinlist;
        RelOptInfo *final_rel;
-       Index           rti;
-       double          total_pages;
 
        /*
         * If the query has an empty join tree, then it's something easy like
@@ -231,34 +229,6 @@ query_planner(PlannerInfo *root, List *tlist,
         */
        extract_restriction_or_clauses(root);
 
-       /*
-        * We should now have size estimates for every actual table involved in
-        * the query, and we also know which if any have been deleted from the
-        * query by join removal; so we can compute total_table_pages.
-        *
-        * Note that appendrels are not double-counted here, even though we don't
-        * bother to distinguish RelOptInfos for appendrel parents, because the
-        * parents will still have size zero.
-        *
-        * XXX if a table is self-joined, we will count it once per appearance,
-        * which perhaps is the wrong thing ... but that's not completely clear,
-        * and detecting self-joins here is difficult, so ignore it for now.
-        */
-       total_pages = 0;
-       for (rti = 1; rti < root->simple_rel_array_size; rti++)
-       {
-               RelOptInfo *brel = root->simple_rel_array[rti];
-
-               if (brel == NULL)
-                       continue;
-
-               Assert(brel->relid == rti); /* sanity check on array */
-
-               if (IS_SIMPLE_REL(brel))
-                       total_pages += (double) brel->pages;
-       }
-       root->total_table_pages = total_pages;
-
        /*
         * Ready to do the primary planning.
         */

diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 46de00460d9f1a96dee7ac385d6aa242e0c15b6c..0c88c90de4da81b8d53cb06e5e436ee231226fff 100644 (file)
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -138,9 +138,8 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 
        /*
         * Estimate relation size --- unless it's an inheritance parent, in which
-        * case the size will be computed later in set_append_rel_pathlist, and we
-        * must leave it zero for now to avoid bollixing the total_table_pages
-        * calculation.
+        * case the size we want is not the rel's own size but the size of its
+        * inheritance tree.  That will be computed in set_append_rel_size().
         */
        if (!inhparent)
                estimate_rel_size(relation, rel->attr_widths - rel->min_attr,

diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 88d37236f7d3c70e0dfbe066151e41f32b860b09..6fd24203dd6f71b4054fdb650d8c9ff67df67131 100644 (file)
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -310,7 +310,8 @@ typedef struct PlannerInfo
 
        MemoryContext planner_cxt;      /* context holding PlannerInfo */
 
-       double          total_table_pages;      /* # of pages in all tables of query */
+       double          total_table_pages;      /* # of pages in all non-dummy tables of
+                                                                        * query */
 
        double          tuple_fraction; /* tuple_fraction passed to query_planner */
        double          limit_tuples;   /* limit_tuples passed to query_planner */
@@ -687,9 +688,8 @@ typedef struct RelOptInfo
        bool            has_eclass_joins;       /* T means joininfo is incomplete */
 
        /* used by partitionwise joins: */
-       bool            consider_partitionwise_join;    /* consider partitionwise
-                                                                                                * join paths? (if
-                                                                                                * partitioned rel) */
+       bool            consider_partitionwise_join;    /* consider partitionwise join
+                                                                                                * paths? (if partitioned rel) */
        Relids          top_parent_relids;      /* Relids of topmost parents (if "other"
                                                                         * rel) */