From 59f599f8a6ebc1ca33dac39f8029a1575b081e17 Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Mon, 6 Aug 2018 19:50:11 +1200 Subject: [PATCH v1] Calculate total_table_pages after set_base_rel_sizes Previously we did this before we attempted to prune partitions or eliminate append children with constraint exclusion. This could lead to incorrectly applying random_page_cost to more pages in an index scan, which could result in other scan types being preferred incorrectly. --- src/backend/optimizer/path/allpaths.c | 35 +++++++++++++++++++++++++++++++++++ src/backend/optimizer/plan/planmain.c | 30 ------------------------------ src/include/nodes/relation.h | 3 ++- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 0e80aeb65c..b97b5d01f5 100644 --- 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. @@ -177,6 +178,40 @@ make_one_rel(PlannerInfo *root, List *joinlist) * then generate access paths. */ 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, and eliminated by + * constraint exclusion. 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 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 *rel = root->simple_rel_array[rti]; + + if (rel == NULL) + continue; + + Assert(rel->relid == rti); /* sanity check on array */ + + if (IS_DUMMY_REL(rel)) + continue; + + if (IS_SIMPLE_REL(rel)) + total_pages += (double) rel->pages; + } + + root->total_table_pages = total_pages; + set_base_rel_pathlists(root); /* diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c index b05adc70c4..9b6cc9e10f 100644 --- 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/include/nodes/relation.h b/src/include/nodes/relation.h index 41caf873fb..c31356479a 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -311,7 +311,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 */ -- 2.17.1