From 1339a8c782f941d213300f6bb7067a79956b3280 Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Mon, 6 Aug 2018 19:50:11 +1200 Subject: [PATCH 3/6] 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. Author: David Rowley --- src/backend/optimizer/path/allpaths.c | 39 +++++++++++++++++++++++++++++++++-- src/backend/optimizer/plan/planmain.c | 30 --------------------------- src/include/nodes/relation.h | 3 ++- 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index aa678eac82..051e4a8e74 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -151,6 +151,7 @@ make_one_rel(PlannerInfo *root, List *joinlist) { RelOptInfo *rel; Index rti; + double total_pages; /* * Construct the all_baserels Relids set. @@ -177,10 +178,44 @@ 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, 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; + + /* Generate access paths. */ 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 @@ -232,34 +230,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. */ final_rel = make_one_rel(root, joinlist); diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 21f1e4e6e6..06212ce382 100644 --- 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 */ -- 2.11.0