Fix bad plan for materialization

REFRESH MATERIALIZED VIEW statements sometimes crash
because the generated plan has a HashAgg node
instead of a Partial Aggregate node when executing
INSERT INTO <materialization_table>
       SELECT * FROM <view>

Where the view stmt itself is along these lines
SELECT time_bucket('1 day'::interval, cpu."time") ,
 ......
 _timescaledb_internal.partialize_agg(avg(...))
FROM <table>

src/plan_partialize.c

@@ -166,33 +166,33 @@ partialize_agg_paths(RelOptInfo *rel)
  * cases where the planner transparently reduces the having expression to a
  * simple filter (e.g., HAVING device > 3). In such cases, the HAVING clause is
  * removed and replaced by a filter on the input.
+ * Returns : true if partial aggs were found, false otherwise.
  */
-void
+bool
 ts_plan_process_partialize_agg(PlannerInfo *root, RelOptInfo *input_rel, RelOptInfo *output_rel)
 {
 	Query *parse = root->parse;
-
 #if PG10_GE
 	Assert(IS_UPPER_REL(output_rel));
 #endif
 
 	if (CMD_SELECT != parse->commandType || !parse->hasAggs)
-		return;
+		return false;
 
-	if (has_partialize_function(parse))
-	{
-		/* We cannot check root->hasHavingqual here because sometimes the
-		 * planner can replace the HAVING clause with a simple filter. But
-		 * root->hashavingqual stays true to remember that the query had a
-		 * HAVING clause initially. */
-		if (NULL != parse->havingQual)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot partialize aggregate with HAVING clause"),
-					 errhint(
-						 "Any aggregates in a HAVING clause need to be partialized in the output "
+	if (!has_partialize_function(parse))
+		return false;
+
+	/* We cannot check root->hasHavingqual here because sometimes the
+	 * planner can replace the HAVING clause with a simple filter. But
+	 * root->hashavingqual stays true to remember that the query had a
+	 * HAVING clause initially. */
+	if (NULL != parse->havingQual)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot partialize aggregate with HAVING clause"),
+				 errhint("Any aggregates in a HAVING clause need to be partialized in the output "
 						 "target list.")));
 
-		partialize_agg_paths(output_rel);
-	}
+	partialize_agg_paths(output_rel);
+	return true;
 }

src/plan_partialize.h

@@ -8,7 +8,7 @@
 #include <postgres.h>
 #include <optimizer/planner.h>
 
-void ts_plan_process_partialize_agg(PlannerInfo *root, RelOptInfo *input_rel,
+bool ts_plan_process_partialize_agg(PlannerInfo *root, RelOptInfo *input_rel,
 									RelOptInfo *output_rel);
 
 #endif /* TIMESCALEDB_PLAN_PARTIALIZE_H */

src/planner.c

@@ -715,6 +715,7 @@ timescale_create_upper_paths_hook(PlannerInfo *root, UpperRelationKind stage, Re
 								  RelOptInfo *output_rel)
 {
 	Query *parse = root->parse;
+	bool partials_found = false;
 
 	if (prev_create_upper_paths_hook != NULL)
 		prev_create_upper_paths_hook(root, stage, input_rel, output_rel);
@@ -723,6 +724,7 @@ timescale_create_upper_paths_hook(PlannerInfo *root, UpperRelationKind stage, Re
 								  RelOptInfo *output_rel, void *extra)
 {
 	Query *parse = root->parse;
+	bool partials_found = false;
 
 	if (prev_create_upper_paths_hook != NULL)
 		prev_create_upper_paths_hook(root, stage, input_rel, output_rel, extra);
@@ -739,11 +741,12 @@ timescale_create_upper_paths_hook(PlannerInfo *root, UpperRelationKind stage, Re
 		/* Modify for INSERTs on a hypertable */
 		if (output_rel->pathlist != NIL)
 			output_rel->pathlist = replace_hypertable_insert_paths(root, output_rel->pathlist);
-
 		if (parse->hasAggs && stage == UPPERREL_GROUP_AGG)
 		{
-			/* modify aggregates that need to be partialized */
-			ts_plan_process_partialize_agg(root, input_rel, output_rel);
+			/* existing AggPaths are modified here.
+			 * No new AggPaths should be added after this if there
+			 * are partials*/
+			partials_found = ts_plan_process_partialize_agg(root, input_rel, output_rel);
 		}
 	}
 
@@ -752,10 +755,10 @@ timescale_create_upper_paths_hook(PlannerInfo *root, UpperRelationKind stage, Re
 
 	if (!ts_guc_optimize_non_hypertables && !involves_hypertable(root, input_rel))
 		return;
-
-	if (UPPERREL_GROUP_AGG == stage && output_rel != NULL)
+	if (stage == UPPERREL_GROUP_AGG && output_rel != NULL)
 	{
-		ts_plan_add_hashagg(root, input_rel, output_rel);
+		if (!partials_found)
+			ts_plan_add_hashagg(root, input_rel, output_rel);
 
 		if (parse->hasAggs)
 			ts_preprocess_first_last_aggregates(root, root->processed_tlist);

tsl/test/expected/partialize_finalize.out

@@ -206,6 +206,22 @@ INSERT INTO foo VALUES(1,  '2005-10-19 10:23:54', repeat('I am a tall big giraff
 INSERT INTO foo values( 1, '2005-01-01 00:00:00+00', NULL);
 INSERT INTO foo values( 2, '2005-01-01 00:00:00+00', NULL);
 create or replace  view v1(a, partialb, partialtv) as select a, _timescaledb_internal.partialize_agg( max(b) ), _timescaledb_internal.partialize_agg( min(toastval)) from foo group by a;
+EXPLAIN (VERBOSE, COSTS OFF)
+create table t1 as select * from v1;
+                                                                                    QUERY PLAN                                                                                     
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Partial HashAggregate
+   Output: _hyper_1_1_chunk.a, _timescaledb_internal.partialize_agg(PARTIAL max(_hyper_1_1_chunk.b)), _timescaledb_internal.partialize_agg(PARTIAL min(_hyper_1_1_chunk.toastval))
+   Group Key: _hyper_1_1_chunk.a
+   ->  Append
+         ->  Seq Scan on _timescaledb_internal._hyper_1_1_chunk
+               Output: _hyper_1_1_chunk.a, _hyper_1_1_chunk.b, _hyper_1_1_chunk.toastval
+         ->  Seq Scan on _timescaledb_internal._hyper_1_2_chunk
+               Output: _hyper_1_2_chunk.a, _hyper_1_2_chunk.b, _hyper_1_2_chunk.toastval
+         ->  Seq Scan on _timescaledb_internal._hyper_1_3_chunk
+               Output: _hyper_1_3_chunk.a, _hyper_1_3_chunk.b, _hyper_1_3_chunk.toastval
+(10 rows)
+
 create table t1 as select * from v1;
 insert into t1 select * from v1;
 select a, _timescaledb_internal.finalize_agg( 'max(timestamp with time zone)', null, null, null, partialb, null::timestamptz ) maxb,

tsl/test/sql/partialize_finalize.sql

@@ -162,6 +162,9 @@ INSERT INTO foo values( 2, '2005-01-01 00:00:00+00', NULL);
 
 create or replace  view v1(a, partialb, partialtv) as select a, _timescaledb_internal.partialize_agg( max(b) ), _timescaledb_internal.partialize_agg( min(toastval)) from foo group by a;
 
+EXPLAIN (VERBOSE, COSTS OFF)
+create table t1 as select * from v1;
+
 create table t1 as select * from v1;
 
 insert into t1 select * from v1;