From 1d366399d1ea60dccf5c6d5f3cc951e36ce0fc6e Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Thu, 20 Jun 2024 14:44:20 +0200 Subject: [PATCH 1/2] Make cost-based decision about partial aggregation paths Currently we just remove all existing paths, but after that we add a hash aggregation path (ts_plan_add_hashagg()) which we can prefer based on the cost, which leads to weird behavior. --- src/planner/partialize.c | 4 ---- src/planner/planner.c | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/planner/partialize.c b/src/planner/partialize.c index ea13257d904..7e9400a2bc5 100644 --- a/src/planner/partialize.c +++ b/src/planner/partialize.c @@ -851,10 +851,6 @@ ts_pushdown_partial_agg(PlannerInfo *root, Hypertable *ht, RelOptInfo *input_rel if (partially_grouped_rel->pathlist == NIL) return; - /* Prefer our paths */ - output_rel->pathlist = NIL; - output_rel->partial_pathlist = NIL; - /* Finalize the created partially aggregated paths by adding a 'Finalize Aggregate' node on top * of them. */ AggClauseCosts *agg_final_costs = &extra_data->agg_final_costs; diff --git a/src/planner/planner.c b/src/planner/planner.c index f22537ebc8f..0860f1549af 100644 --- a/src/planner/planner.c +++ b/src/planner/planner.c @@ -1571,11 +1571,11 @@ timescaledb_create_upper_paths_hook(PlannerInfo *root, UpperRelationKind stage, if (parse->hasAggs) ts_preprocess_first_last_aggregates(root, root->processed_tlist); - if (ts_guc_enable_chunkwise_aggregation) - ts_pushdown_partial_agg(root, ht, input_rel, output_rel, extra); - if (!partials_found) ts_plan_add_hashagg(root, input_rel, output_rel); + + if (ts_guc_enable_chunkwise_aggregation) + ts_pushdown_partial_agg(root, ht, input_rel, output_rel, extra); } } From 58bbd49657700f7893976f206f0706b1dd889337 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Thu, 20 Jun 2024 17:54:06 +0200 Subject: [PATCH 2/2] try not cost-based but before the hash agg paths --- src/planner/partialize.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/planner/partialize.c b/src/planner/partialize.c index 7e9400a2bc5..ea13257d904 100644 --- a/src/planner/partialize.c +++ b/src/planner/partialize.c @@ -851,6 +851,10 @@ ts_pushdown_partial_agg(PlannerInfo *root, Hypertable *ht, RelOptInfo *input_rel if (partially_grouped_rel->pathlist == NIL) return; + /* Prefer our paths */ + output_rel->pathlist = NIL; + output_rel->partial_pathlist = NIL; + /* Finalize the created partially aggregated paths by adding a 'Finalize Aggregate' node on top * of them. */ AggClauseCosts *agg_final_costs = &extra_data->agg_final_costs;