Greetings Ashutosh, * Ashutosh Bapat (ashutosh.ba...@enterprisedb.com) wrote: > Here are two patches fixing comments.
Thanks! > 0001 > A comment in add_paths_to_append_rel() mentions the number of paths > instead of number of workers by mistake. Fixed it. This looks alright. > 0002 > Commit b5635948ab165b6070e7d05d111f966e07570d81 added a single > grouping_sets_data argument instead of two lists related to grouping > sets to create_grouping_paths(), but forgot to update the prologue of > the function. Fixed it. This also looks good, but misses a similar mishap from that commit, in the function header of get_number_of_groups(). Patch attached. Will push soon. Thanks! Stephen
From d7a7e41558d9cf51d14049c6ed5f43e4bbf57b38 Mon Sep 17 00:00:00 2001 From: Stephen Frost <sfr...@snowman.net> Date: Wed, 14 Mar 2018 09:44:16 -0400 Subject: [PATCH 1/2] Fix typo in add_paths_to_append_rel() The comment should have been referring to the number of workers, not the number of paths. Author: Ashutosh Bapat Discussion: https://postgr.es/m/cafjfprcbp4702jcp387pext3fnct62qjn8++dqgwbhsw6wr...@mail.gmail.com --- src/backend/optimizer/path/allpaths.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index ea4e683abb..8735e29807 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -1574,7 +1574,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel, /* * If the use of parallel append is permitted, always request at least - * log2(# of children) paths. We assume it can be useful to have + * log2(# of children) workers. We assume it can be useful to have * extra workers in this case because they will be spread out across * the children. The precise formula is just a guess, but we don't * want to end up with a radically different answer for a table with N -- 2.14.1 From 0b60c2e8095fb64380b678d7967d539a2d8ab2ab Mon Sep 17 00:00:00 2001 From: Stephen Frost <sfr...@snowman.net> Date: Wed, 14 Mar 2018 12:17:29 -0400 Subject: [PATCH 2/2] Fix function-header comments in planner.c In b5635948ab1, a couple of function header comments weren't changed, or weren't changed correctly, to reflect the arguments being passed into the functions. Specifically, get_number_of_groups() had the wrong argument name in the commit and create_grouping_paths() wasn't updated even though the arguments had been changed. The issue with create_grouping_paths() was noticed by Ashutosh Bapat, while I discovered the issue with get_number_of_groups() by looking to see if there were any similar issues from that commit. Discussion: https://postgr.es/m/cafjfprcbp4702jcp387pext3fnct62qjn8++dqgwbhsw6wr...@mail.gmail.com --- src/backend/optimizer/plan/planner.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 66e7e7badc..182b01627e 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -3504,7 +3504,7 @@ standard_qp_callback(PlannerInfo *root, void *extra) * Estimate number of groups produced by grouping clauses (1 if not grouping) * * path_rows: number of output rows from scan/join step - * gsets: grouping set data, or NULL if not doing grouping sets + * gd: grouping sets data including list of grouping sets and their clauses * * If doing grouping sets, we also annotate the gsets data with the estimates * for each set and each individual rollup list, with a view to later @@ -3659,9 +3659,7 @@ estimate_hashagg_tablesize(Path *path, const AggClauseCosts *agg_costs, * input_rel: contains the source-data Paths * target: the pathtarget for the result Paths to compute * agg_costs: cost info about all aggregates in query (in AGGSPLIT_SIMPLE mode) - * rollup_lists: list of grouping sets, or NIL if not doing grouping sets - * rollup_groupclauses: list of grouping clauses for grouping sets, - * or NIL if not doing grouping sets + * gd: grouping sets data including list of grouping sets and their clauses * * Note: all Paths in input_rel are expected to return the target computed * by make_group_input_target. -- 2.14.1
signature.asc
Description: PGP signature
