Fix wrong rows from HAVING with time_bucket_gapfill

svenklemm · svenklemm · commit 74e17dcdeda9 · 2026-04-29T08:01:29.000+02:00
When a HAVING clause was used with gapfill, groups it filtered out disappeared before gapfill could extend them. Queries that filtered out every group returned zero rows, and queries that filtered out some still got gapfilled rows for the filtered buckets. Move the HAVING quals onto the gapfill node so they run on every tuple it produces, real or gapfilled. Gap rows carry NULL aggregates, so normal SQL semantics apply: count(*) > N drops them and count(*) IS NULL keeps them. This only works when each aggregate in HAVING is a top-level entry of the gapfill output. Queries that wrap aggregates with locf or interpolate keep the old behaviour, which is correct as long as HAVING does not eliminate every real group. Fixes #5202
diff --git a/.unreleased/pr_9624 b/.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
diff --git a/tsl/src/nodes/gapfill/gapfill.h b/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;
diff --git a/tsl/src/nodes/gapfill/gapfill_exec.c b/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)
 {
diff --git a/tsl/src/nodes/gapfill/gapfill_plan.c b/tsl/src/nodes/gapfill/gapfill_plan.c
@@ -275,6 +275,8 @@ gapfill_plan_create(PlannerInfo *root, RelOptInfo *rel, CustomPath *path, List *
 	cscan->flags = path->flags;
 	cscan->methods = &gapfill_plan_methods;
 
+	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 +415,34 @@ gapfill_build_pathtarget(PathTarget *pt_upper, PathTarget *pt_path, PathTarget *
 	}
 }
 
+/*
+ * Lifting a qual onto the CustomScan plan only works when every Aggref it
+ * references is also a top-level expression of the pathtarget, because
+ * set_customscan_references resolves quals against custom_scan_tlist and
+ * cannot match a bare Aggref against a wrapped expression like locf(agg).
+ */
+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;
+	bool ok = true;
+
+	foreach (lc, aggs)
+	{
+		Node *agg = lfirst(lc);
+
+		if (IsA(agg, Aggref) && !list_member(pt->exprs, agg))
+		{
+			ok = false;
+			break;
+		}
+	}
+
+	list_free(aggs);
+	return ok;
+}
+
 /*
  * Create a Gapfill Path node.
  *
@@ -446,6 +476,37 @@ gapfill_path_create(PlannerInfo *root, Path *subpath, FuncExpr *func)
 							 path->cpath.path.pathtarget,
 							 subpath->pathtarget);
 
+	/*
+	 * Move HAVING quals from the aggregate subpath onto the GapFill node so
+	 * they run after gap rows are generated; otherwise filtered-out groups
+	 * disappear before gapfill can extend them (#5202). Skipped when an
+	 * Aggref is not a top-level pathtarget expression (locf/interpolate
+	 * wraps it), since setrefs cannot resolve it then.
+	 */
+	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;
diff --git a/tsl/test/shared/expected/gapfill_bug.out b/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;
diff --git a/tsl/test/shared/sql/gapfill_bug.sql b/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;

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Fixes: #9624 Fix HAVING clause with time_bucket_gapfill`
	`2`	`+Thanks: @fr3aker for reporting an issue with time_bucket_gapfill and HAVING`
Original file line number	Diff line number	Diff line change
`@@ -32,5 +32,7 @@ void gapfill_adjust_window_targetlist(PlannerInfo root, RelOptInfo input_rel,`
`32`	`32`	`typedef struct GapFillPath`
`33`	`33`	`{`
`34`	`34`	`CustomPath cpath;`
`35`		`- FuncExpr func; / time_bucket_gapfill function call */`
	`35`	`+ FuncExpr func; / time_bucket_gapfill function call */`
	`36`	`+ List having_quals; / HAVING quals lifted from the aggregate subpath so they are evaluated`
	`37`	`+ after gaps are generated (fixes #5202) */`
`36`	`38`	`} GapFillPath;`