Re: Partition-wise aggregation/grouping

On Fri, Oct 13, 2017 at 1:13 PM, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
wrote:

>
> I looked over the patch and saw this:
>
> @@ -1800,6 +1827,9 @@ cost_merge_append(Path *path, PlannerInfo *root,
> */
> run_cost += cpu_operator_cost * tuples;
>
> + /* Add MergeAppend node overhead like we do it for the Append node */
> + run_cost += cpu_tuple_cost * DEFAULT_APPEND_COST_FACTOR * tuples;
> +
> path->startup_cost = startup_cost + input_startup_cost;
> path->total_cost = startup_cost + run_cost + input_total_cost;
> }
>
> You're doing that right after a comment that says we don't do that. It
> also does look like the "run_cost += cpu_operator_cost * tuples" is
> trying to do the same thing, so perhaps it's worth just replacing
> that, which by default will double that additional cost, although
> doing so would have the planner slightly prefer a MergeAppend to an
> Append than previously.
>

I think we can remove that code block entirely. I have added relevant
comments
around DEFAULT_APPEND_COST_FACTOR already.
However, I am not sure of doing this as you correctly said it may prefer
MergeAppend to an Append. Will it be fine we remove that code block?

> +#define DEFAULT_APPEND_COST_FACTOR 0.5
>
> I don't really think the DEFAULT_APPEND_COST_FACTOR adds much. it
> means very little by itself. It also seems that most of the other cost
> functions just use the magic number.
>

Agree, but those magic numbers used only once at that place. But here there
are two places. So if someone wants to update it, (s)he needs to make sure
to update that at two places. To minimize that risk, having a #define seems
better.

>
> --
> David Rowley http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

--
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company