Fix HAVING clause with time_bucket_gapfill returning wrong rows

HAVING quals were attached to the GroupAggregate below the GapFill
node, so filtered-out groups disappeared before gap rows could be
generated. When HAVING eliminated every group the query returned zero
rows; when it eliminated some, GapFill still synthesized NULL rows for
the filtered buckets.

Lift the quals off the aggregate subpath and attach them to the
CustomScan plan's scan.plan.qual, then evaluate them on every tuple
(real and gap-filled) returned from gapfill_exec. Gap rows have NULL
aggregates, so standard SQL HAVING semantics apply: count(*) > N drops
them, count(*) IS NULL keeps them.

Fall back to the old behaviour for queries where HAVING references an
Aggref that is not a top-level expression of the GapFill pathtarget
(e.g. HAVING sum(x) > 4 with locf(sum(x)) in the target list), since
set_customscan_references cannot resolve the bare Aggref against
custom_scan_tlist in that case.

Fixes #5202

Author: svenklemm

.unreleased/pr_9624

@@ -0,0 +1,2 @@
+Fixes: #9624 Fix HAVING clause with time_bucket_gapfill
+Thanks: @fr3aker for reporting an issue with time_bucket_gapfill and HAVING

tsl/src/nodes/gapfill/gapfill.h

@@ -32,5 +32,7 @@ void gapfill_adjust_window_targetlist(PlannerInfo *root, RelOptInfo *input_rel,
 typedef struct GapFillPath
 {
 	CustomPath cpath;
-	FuncExpr *func; /* time_bucket_gapfill function call */
+	FuncExpr *func;		/* time_bucket_gapfill function call */
+	List *having_quals; /* HAVING quals lifted from the aggregate subpath so they are evaluated
+						   after gaps are generated (fixes #5202) */
 } GapFillPath;

tsl/src/nodes/gapfill/gapfill_exec.c

@@ -857,14 +857,13 @@ gapfill_begin(CustomScanState *node, EState *estate, int eflags)
 }
 
 /*
- * This is the main loop of the node it is called whenever the upper node
- * wants to consume a new tuple. Returning NULL signals that the tuples
- * are exhausted. All gapfill state transitions happen in this function.
+ * Produce the next candidate tuple from the gapfill state machine without
+ * applying the HAVING qual. Returning NULL signals that the tuples are
+ * exhausted. All gapfill state transitions happen in this function.
  */
 static TupleTableSlot *
-gapfill_exec(CustomScanState *node)
+gapfill_next_candidate(GapFillState *state)
 {
-	GapFillState *state = (GapFillState *) node;
 	TupleTableSlot *slot = NULL;
 
 	while (true)
@@ -944,6 +943,32 @@ gapfill_exec(CustomScanState *node)
 	}
 }
 
+/*
+ * Return the next tuple that passes the HAVING qual (if any). The qual is
+ * evaluated on top of the GapFill node so that it does not prevent gap rows
+ * from being generated.
+ */
+static TupleTableSlot *
+gapfill_exec(CustomScanState *node)
+{
+	GapFillState *state = (GapFillState *) node;
+	ExprState *qual = node->ss.ps.qual;
+	ExprContext *econtext = node->ss.ps.ps_ExprContext;
+
+	while (true)
+	{
+		TupleTableSlot *slot = gapfill_next_candidate(state);
+
+		if (TupIsNull(slot) || qual == NULL)
+			return slot;
+
+		ResetExprContext(econtext);
+		econtext->ecxt_scantuple = slot;
+		if (ExecQual(qual, econtext))
+			return slot;
+	}
+}
+
 static void
 gapfill_end(CustomScanState *node)
 {

tsl/src/nodes/gapfill/gapfill_plan.c

@@ -275,6 +275,13 @@ gapfill_plan_create(PlannerInfo *root, RelOptInfo *rel, CustomPath *path, List *
 	cscan->flags = path->flags;
 	cscan->methods = &gapfill_plan_methods;
 
+	/*
+	 * HAVING quals stolen from the aggregate subpath get evaluated on top of
+	 * the GapFill node so that they do not prevent gap rows from being
+	 * generated. See fix for #5202.
+	 */
+	cscan->scan.plan.qual = gfpath->having_quals;
+
 	cscan->custom_private = ts_new_list(T_List, GFP_Count);
 	lfirst(list_nth_cell(cscan->custom_private, GFP_GapfillFunc)) = gfpath->func;
 	lfirst(list_nth_cell(cscan->custom_private, GFP_GroupClause)) = root->parse->groupClause;
@@ -413,6 +420,35 @@ gapfill_build_pathtarget(PathTarget *pt_upper, PathTarget *pt_path, PathTarget *
 	}
 }
 
+/*
+ * Return true when every Aggref inside the given quals also appears as a
+ * top-level expression in the pathtarget. Used to decide whether HAVING quals
+ * can be safely lifted onto the CustomScan plan's qual list, since
+ * set_customscan_references matches expressions against custom_scan_tlist and
+ * would otherwise fail to resolve Vars buried inside an Aggref's arguments.
+ */
+static bool
+gapfill_quals_reference_only_pathtarget_aggs(List *quals, PathTarget *pt)
+{
+	List *aggs = pull_var_clause((Node *) quals,
+								 PVC_INCLUDE_AGGREGATES | PVC_RECURSE_PLACEHOLDERS);
+	ListCell *lc;
+
+	foreach (lc, aggs)
+	{
+		Node *agg = lfirst(lc);
+
+		if (IsA(agg, Aggref) && !list_member(pt->exprs, agg))
+		{
+			list_free(aggs);
+			return false;
+		}
+	}
+
+	list_free(aggs);
+	return true;
+}
+
 /*
  * Create a Gapfill Path node.
  *
@@ -446,6 +482,45 @@ gapfill_path_create(PlannerInfo *root, Path *subpath, FuncExpr *func)
 							 path->cpath.path.pathtarget,
 							 subpath->pathtarget);
 
+	/*
+	 * Lift the HAVING quals off the aggregate subpath so they can be evaluated
+	 * on top of the GapFill node. Evaluating them below gapfill drops groups
+	 * before gaps are generated (issue #5202): when every group is filtered
+	 * out, gapfill has nothing to extend; when only some are filtered out,
+	 * gapfill still generates synthetic rows for the filtered buckets.
+	 *
+	 * We only lift when every Aggref referenced by HAVING is also a top-level
+	 * expression in the GapFill pathtarget. Otherwise setrefs would fail to
+	 * resolve Aggref references when processing scan.plan.qual against
+	 * custom_scan_tlist. This happens for queries that wrap aggregates in
+	 * locf/interpolate - pt_path contains locf(agg) rather than the bare agg.
+	 * For those queries we keep the old (partially broken) behaviour, which
+	 * at least works when HAVING does not filter out every group.
+	 */
+	List **subpath_qual_slot = NULL;
+	switch (nodeTag(subpath))
+	{
+		case T_AggPath:
+			subpath_qual_slot = &((AggPath *) subpath)->qual;
+			break;
+		case T_GroupPath:
+			subpath_qual_slot = &((GroupPath *) subpath)->qual;
+			break;
+		case T_GroupingSetsPath:
+			subpath_qual_slot = &((GroupingSetsPath *) subpath)->qual;
+			break;
+		default:
+			break;
+	}
+
+	if (subpath_qual_slot != NULL && *subpath_qual_slot != NIL &&
+		gapfill_quals_reference_only_pathtarget_aggs(*subpath_qual_slot,
+													 path->cpath.path.pathtarget))
+	{
+		path->having_quals = *subpath_qual_slot;
+		*subpath_qual_slot = NIL;
+	}
+
 	if (!gapfill_correct_order(root, subpath, func))
 	{
 		List *new_order = NIL;

tsl/test/shared/expected/gapfill_bug.out

@@ -489,3 +489,95 @@ SELECT * FROM STATS LEFT JOIN VOLUME USING (bucket);
 DROP TABLE gf8844_table1;
 DROP TABLE gf8844_table2;
 RESET timezone;
+-- Fix for #5202: time_bucket_gapfill with HAVING clause returning incorrect rows
+-- HAVING quals must be evaluated on top of the GapFill node so they do not
+-- remove groups before gap rows have been generated.
+SET timezone TO 'UTC';
+CREATE TABLE gf5202(time timestamptz, device_id int, value float);
+INSERT INTO gf5202 VALUES
+    ('2023-01-03T00:00:00Z', 1, 4),
+    ('2023-01-03T01:00:00Z', 1, 4),
+    ('2023-01-03T02:00:00Z', 1, 4),
+    ('2023-01-05T00:00:00Z', 1, 6);
+-- Filter should appear on the Custom Scan (GapFill) node, not on the GroupAggregate.
+EXPLAIN (COSTS OFF)
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) < 2;
+--- QUERY PLAN ---
+ Custom Scan (GapFill)
+   Filter: ((count(*)) < 2)
+   ->  Sort
+         Sort Key: device_id, (time_bucket_gapfill('@ 1 day'::interval, "time", NULL::timestamp with time zone, NULL::timestamp with time zone))
+         ->  GroupAggregate
+               Group Key: (time_bucket_gapfill('@ 1 day'::interval, "time", NULL::timestamp with time zone, NULL::timestamp with time zone)), device_id
+               ->  Sort
+                     Sort Key: (time_bucket_gapfill('@ 1 day'::interval, "time", NULL::timestamp with time zone, NULL::timestamp with time zone)), device_id
+                     ->  Seq Scan on gf5202
+                           Filter: (("time" >= 'Sun Jan 01 00:00:00 2023 UTC'::timestamp with time zone) AND ("time" < 'Sun Jan 08 00:00:00 2023 UTC'::timestamp with time zone))
+
+-- HAVING count(*) < 2: real group with count=3 (2023-01-03) is dropped.
+-- Real group with count=1 (2023-01-05) is kept. Gap rows (NULL count) are
+-- rejected by <2 (NULL<2 is UNKNOWN -> false) under SQL semantics.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) < 2
+ORDER BY day, device_id;
+             day              | device_id | count 
+------------------------------+-----------+-------
+ Thu Jan 05 00:00:00 2023 UTC |         1 |     1
+
+-- HAVING count(*) IS NULL keeps only the gap rows.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) IS NULL
+ORDER BY day, device_id;
+             day              | device_id | count 
+------------------------------+-----------+-------
+ Sun Jan 01 00:00:00 2023 UTC |         1 |      
+ Mon Jan 02 00:00:00 2023 UTC |         1 |      
+ Wed Jan 04 00:00:00 2023 UTC |         1 |      
+ Fri Jan 06 00:00:00 2023 UTC |         1 |      
+ Sat Jan 07 00:00:00 2023 UTC |         1 |      
+
+-- HAVING predicate combining agg and group column.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) IS NULL OR count(*) < 2
+ORDER BY day, device_id;
+             day              | device_id | count 
+------------------------------+-----------+-------
+ Sun Jan 01 00:00:00 2023 UTC |         1 |      
+ Mon Jan 02 00:00:00 2023 UTC |         1 |      
+ Wed Jan 04 00:00:00 2023 UTC |         1 |      
+ Thu Jan 05 00:00:00 2023 UTC |         1 |     1
+ Fri Jan 06 00:00:00 2023 UTC |         1 |      
+ Sat Jan 07 00:00:00 2023 UTC |         1 |      
+
+-- HAVING that rejects every real group must still produce the gap rows when
+-- the predicate leaves room for them via IS NULL. Before the fix this
+-- returned zero rows because HAVING ran below the gapfill node.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) > 1000 OR count(*) IS NULL
+ORDER BY day, device_id;
+             day              | device_id | count 
+------------------------------+-----------+-------
+ Sun Jan 01 00:00:00 2023 UTC |         1 |      
+ Mon Jan 02 00:00:00 2023 UTC |         1 |      
+ Wed Jan 04 00:00:00 2023 UTC |         1 |      
+ Fri Jan 06 00:00:00 2023 UTC |         1 |      
+ Sat Jan 07 00:00:00 2023 UTC |         1 |      
+
+DROP TABLE gf5202;
+RESET timezone;

tsl/test/shared/sql/gapfill_bug.sql

@@ -279,3 +279,62 @@ DROP TABLE gf8844_table1;
 DROP TABLE gf8844_table2;
 
 RESET timezone;
+
+-- Fix for #5202: time_bucket_gapfill with HAVING clause returning incorrect rows
+-- HAVING quals must be evaluated on top of the GapFill node so they do not
+-- remove groups before gap rows have been generated.
+SET timezone TO 'UTC';
+
+CREATE TABLE gf5202(time timestamptz, device_id int, value float);
+INSERT INTO gf5202 VALUES
+    ('2023-01-03T00:00:00Z', 1, 4),
+    ('2023-01-03T01:00:00Z', 1, 4),
+    ('2023-01-03T02:00:00Z', 1, 4),
+    ('2023-01-05T00:00:00Z', 1, 6);
+
+-- Filter should appear on the Custom Scan (GapFill) node, not on the GroupAggregate.
+EXPLAIN (COSTS OFF)
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) < 2;
+
+-- HAVING count(*) < 2: real group with count=3 (2023-01-03) is dropped.
+-- Real group with count=1 (2023-01-05) is kept. Gap rows (NULL count) are
+-- rejected by <2 (NULL<2 is UNKNOWN -> false) under SQL semantics.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) < 2
+ORDER BY day, device_id;
+
+-- HAVING count(*) IS NULL keeps only the gap rows.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) IS NULL
+ORDER BY day, device_id;
+
+-- HAVING predicate combining agg and group column.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) IS NULL OR count(*) < 2
+ORDER BY day, device_id;
+
+-- HAVING that rejects every real group must still produce the gap rows when
+-- the predicate leaves room for them via IS NULL. Before the fix this
+-- returned zero rows because HAVING ran below the gapfill node.
+SELECT time_bucket_gapfill('1 day', time) AS day, device_id, count(*)
+FROM gf5202
+WHERE time >= '2023-01-01T00:00:00Z' AND time < '2023-01-08T00:00:00Z'
+GROUP BY day, device_id
+HAVING count(*) > 1000 OR count(*) IS NULL
+ORDER BY day, device_id;
+
+DROP TABLE gf5202;
+RESET timezone;