From f31bf88a9066e63772852ab8299201a7fb49e59e Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 4 Jul 2018 08:25:00 +0200 Subject: [PATCH] Cosmetic cleanups around join ordering code Fixing some of the more obvious breaches of common style around the code I just read for another patchset. There is no logical changes introduced here, only rearrangement for clarity. Discussion: https://github.com/greenplum-db/gpdb/pull/5216 Reviewed-by: Venkatesh Raghavan --- src/backend/optimizer/path/allpaths.c | 10 ++--- src/backend/optimizer/util/plancat.c | 58 ++++++++++++++------------- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index a44d949629..3ecf9aebe3 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -118,8 +118,7 @@ make_one_rel(PlannerInfo *root, List *joinlist) rel = make_rel_from_joinlist(root, joinlist); /* CDB: No path might be found if user set enable_xxx = off */ - if (!rel || - !rel->cheapest_total_path) + if (!rel || !rel->cheapest_total_path) cdb_no_path_for_query(); /* raise error - no return */ /* @@ -1335,8 +1334,7 @@ make_rel_from_joinlist(PlannerInfo *root, List *joinlist) } /* CDB: Fail if no path could be built due to set enable_xxx = off. */ - if (!thisrel || - !thisrel->cheapest_total_path) + if (!thisrel || !thisrel->cheapest_total_path) return NULL; initial_rels = lappend(initial_rels, thisrel); @@ -1456,9 +1454,7 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels, b * Do cleanup work on each just-processed rel. */ prev = NULL; - for (lc = list_head(root->join_rel_level[lev]); - lc != NULL; - lc = next) + for (lc = list_head(root->join_rel_level[lev]); lc != NULL; lc = next) { next = lnext(lc); rel = (RelOptInfo *) lfirst(lc); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 476c89e759..c210a4ba37 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -442,69 +442,71 @@ cdb_estimate_rel_size(RelOptInfo *relOptInfo, return; } - /* coerce values in pg_class to more desirable types */ relpages = (BlockNumber) rel->rd_rel->relpages; reltuples = (double) rel->rd_rel->reltuples; /* - * Asking the QE for the size of the relation is a bit expensive. - * Do we want to do it all the time? Or only for tables that have never had analyze run? + * Asking the QE for the size of the relation is a bit expensive. Do we + * want to do it all the time? Or only for tables that have never had + * analyze run? */ - if (relpages > 0) { - /* - * Let's trust the values we had from analyze, even though they might be out of date. + * Let's trust the values we had from analyze, even though they might + * be out of date. * - * NOTE: external tables are created with estimated larger than zero values. therefore - * we will get here too even though we can never analyze them. + * NOTE: external tables are created with estimated larger than zero + * values, therefore we will get here too even though we can never + * analyze them. */ - curpages = relpages; } else if (RelationIsExternal(rel)) { - /* - * If the relation is an external table use default curpages - */ curpages = DEFAULT_EXTERNAL_TABLE_PAGES; } else { /* - * If GUC gp_enable_relsize_collection is on, get the size of the table to derive curpages - * else use the default value + * If GUC gp_enable_relsize_collection is on, get the size of the table + * to derive curpages, else use the default value. */ - curpages = gp_enable_relsize_collection ? cdbRelMaxSegSize(rel) / BLCKSZ : DEFAULT_INTERNAL_TABLE_PAGES; + if (gp_enable_relsize_collection) + curpages = cdbRelMaxSegSize(rel) / BLCKSZ; + else + curpages = DEFAULT_INTERNAL_TABLE_PAGES; } /* report estimated # pages */ *pages = curpages; /* - * If it's an index, discount the metapage. This is a kluge - * because it assumes more than it ought to about index contents; - * it's reasonably OK for btrees but a bit suspect otherwise. + * If it's an index, discount the metapage. This is a kluge because it + * assumes more than it ought to about index contents; it's reasonably OK + * for btrees but a bit suspect otherwise. */ - if (rel->rd_rel->relkind == RELKIND_INDEX && - relpages > 0) + if (rel->rd_rel->relkind == RELKIND_INDEX && relpages > 0) { curpages--; relpages--; } - /* estimate number of tuples from previous tuple density (as of last analyze) */ + + /* + * Estimate number of tuples from previous tuple density (as of last + * analyze) + */ if (relpages > 0) density = reltuples / (double) relpages; else { - /* - * When we have no data because the relation was truncated, - * estimate tuples per page from attribute datatypes. + /* + * When we have no data because the relation was truncated, estimate + * tuples per page from attribute datatypes. * * (This is the same computation as in get_relation_info() - */ + */ int32 tuple_width; tuple_width = get_rel_data_width(rel, attr_widths); @@ -515,9 +517,9 @@ cdb_estimate_rel_size(RelOptInfo *relOptInfo, } *tuples = ceil(density * curpages); - elog(DEBUG2,"cdb_estimate_rel_size estimated %g tuples and %d pages",*tuples,(int)*pages); - -} /* cdb_estimate_rel_size */ + elog(DEBUG2, "cdb_estimate_rel_size estimated %g tuples and %d pages", + *tuples, (int) *pages); +} /* -- GitLab