Horton

Re: [HACKERS] Partition-wise aggregation/grouping

Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>

View thread View on postgr.es
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
To: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, David Rowley <david.rowley@2ndquadrant.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2018-03-07T11:09:45Z
Lists: pgsql-hackers

Attachments

On Wed, Mar 7, 2018 at 10:04 AM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> On Tue, Mar 6, 2018 at 7:52 PM, Jeevan Chalke
> <jeevan.chalke@enterprisedb.com> wrote:
>
>>
>>
>> Changes look good to me and refactoring will be useful for partitionwise
>> patches.
>>
>> However, will it be good if we add agg_costs into the GroupPathExtraData
>> too?
>> Also can we pass this to the add_partial_paths_to_grouping_rel() and
>> add_paths_to_grouping_rel() to avoid passing can_sort, can_hash and costs
>> related details individually to them?
>
> I think so too.

Here's patch doing that. agg_costs is calculated way before we
populate other members of GroupPathExtraData, which means that we
either set the pointer to agg_costs in GroupPathExtraData or memcpy
its contents. The first option will make GroupPathExtraData asymmetric
about the costs it holds, some as pointers and some as whole
structure.Holding whole structures allows us to compute those anywhere
without worrying about memory allocation or variable life time. So, I
am reluctant to make all costs as pointers. So, I have not added
agg_costs to GroupPathExtraData yet.

We could make GroupPathExtraData as a variable in grouping_planner()
and populate its members as we progress. But I think that's digression
from the original purpose of the patch.

I observe that we are computing agg_costs, number of groups etc. again
in postgres_fdw so there seems to be a merit in passing those values
as GroupPathExtraData to FDW as well like what you have done with
OtherUpperExtraData. But we will come to that once we have
straightened the partition-wise aggregate patches.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Commits

  1. postgres_fdw: Push down partition-wise aggregation.

  2. Remove 'target' from GroupPathExtraData.

  3. Implement partition-wise grouping/aggregation.

  4. Don't pass the grouping target around unnecessarily.

  5. Determine grouping strategies in create_grouping_paths.

  6. Defer creation of partially-grouped relation until it's needed.

  7. Split create_grouping_paths into degenerate and non-degenerate cases.

  8. Pass additional arguments to a couple of grouping-related functions.

  9. Fix logic error in add_paths_to_partial_grouping_rel.

  10. Minor cleanup of code related to partially_grouped_rel.

  11. Add a new upper planner relation for partially-aggregated results.

  12. Charge cpu_tuple_cost * 0.5 for Append and MergeAppend nodes.

  13. Rename enable_partition_wise_join to enable_partitionwise_join

  14. Factor some code out of create_grouping_paths.

  15. Pad XLogReaderState's main_data buffer more aggressively.

  16. Prevent int128 from requiring more than MAXALIGN alignment.

  17. Fix DROP SUBSCRIPTION hang

  18. Inject $(ICU_LIBS) regardless of platform.

  19. Some preliminary refactoring towards partitionwise join.