Horton

Re: Partition-wise aggregation/grouping

Jeevan Chalke <jeevan.chalke@enterprisedb.com>

View thread View on postgr.es
From: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2017-09-27T10:12:29Z
Lists: pgsql-hackers

Attachments

Thanks Ashutosh for reviewing.

Attached new patch-set with following changes:

1. Removed earlier 0007 and 0008 patches which were PoC for supporting
partial aggregation over fdw. I removed them as it will be a different
issue altogether and hence I will tackle them separately once this is
done.

This patch-set now includes support for parallel plans within partitions.

Notes:
HEAD: 59597e6
Partition-wise Join Version: 34

(First six patches 0001 - 0006, remains the same functionality-wise)
0007 - Refactors partial grouping paths creation into the separate function.
0008 - Enables parallelism within the partition-wise aggregation.

This patch also includes a fix for the crash reported by Rajkumar.
While forcibly applying scan/join target to all the Paths for the scan/join
rel, earlier I was using apply_projection_to_path() which modifies the path
in-place which causing this crash as the path finally chosen has been
updated by partition-wise agg path creation. Now I have used
create_projection_path() like we do in partial aggregation paths.

Also, fixed issues reported by Ashutosh.

On Tue, Sep 26, 2017 at 6:16 PM, Ashutosh Bapat <
ashutosh.bapat@enterprisedb.com> wrote:

> Hi Jeevan,
> I have started reviewing these patches.
>
> 0001 looks fine. There might be some changes that will be needed, but
> those will be clear when I review the patch that uses this
> refactoring.
>
> 0002
> + *
> + * If targetlist is provided, we use it else use targetlist from the root.
>   */
>  static double
>  get_number_of_groups(PlannerInfo *root,
>                      double path_rows,
> -                    grouping_sets_data *gd)
> +                    grouping_sets_data *gd,
> +                    List *tlist)
>  {
>     Query      *parse = root->parse;
>     double      dNumGroups;
> +   List       *targetList = (tlist == NIL) ? parse->targetList : tlist;
>
> May be we should just pass targetlist always. Instead of passing NIL,
> pass parse->targetList directly. That would save us one conditional
> assignment. May be passing NIL is required for the patches that use
> this refactoring, but that's not clear as is in this patch.
>

Done in attached patch-set.


>
> 0003
> In the documenation of enable_partition_wise_aggregate, we should
> probably explain why the default is off or like partition_wise_join
> GUC, explain the consequences of turning it off.


I have updated this. Please have a look.


> I doubt if we could
> accept something like partition_wise_agg_cost_factor looks. But we can
> discuss this at a later stage. Mean time it may be worthwhile to fix
> the reason why we would require this GUC. If the regular aggregation
> has cost lesser than partition-wise aggregation in most of the cases,
> then probably we need to fix the cost model.
>

Yep. I will have a look mean-while.


>
> I will continue reviewing rest of the patches.
>
>
Thanks
-- 
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL 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.