Fix sub-second intervals in hierarchical caggs

[konskov]

Previously we used date_part("epoch", interval) and integer division
internally to determine whether the top cagg's interval is a
multiple of its parent's.
This led to precision loss and wrong results
in the case of intervals with sub-second components.

Fixed by using the ts_interval_value_to_internal function to convert
intervals to appropriate integer representation for division.

Fixes #5277

[github-actions]

@svenklemm, @akuzm: please review this pull request.

Powered by pull-review

[codecov]

Codecov Report

Merging #5304 (b97210d) into main (00b566d) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5304      +/-   ##
==========================================
- Coverage   90.70%   90.67%   -0.03%     
==========================================
  Files         227      227              
  Lines       52589    52584       -5     
==========================================
- Hits        47702    47683      -19     
- Misses       4887     4901      +14     

Impacted Files	Coverage Δ
tsl/src/continuous_aggs/create.c	89.12% <100.00%> (ø)
src/loader/bgw_launcher.c	89.51% <0.00%> (-2.55%)	⬇️
src/loader/bgw_message_queue.c	86.36% <0.00%> (-2.28%)	⬇️
src/bgw/scheduler.c	86.95% <0.00%> (-1.28%)	⬇️
tsl/src/bgw_policy/job.c	87.54% <0.00%> (-0.05%)	⬇️
src/compat/compat.h	96.61% <0.00%> (+6.13%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[fabriziomello]

@konskov isn't easier to use float8 instead of int64 for bucket size validation?? Something like:

diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c
index 6dc311712..349f7fa86 100644
--- a/tsl/src/continuous_aggs/create.c
+++ b/tsl/src/continuous_aggs/create.c
@@ -58,6 +58,7 @@
 #include <utils/typcache.h>
 #include <optimizer/prep.h>
 
+#include "math.h"
 #include "create.h"
 
 #include "ts_catalog/catalog.h"
@@ -1121,10 +1122,10 @@ cagg_query_supported(const Query *query, StringInfo hint, StringInfo detail, con
 	return true; /* Query was OK and is supported. */
 }
 
-static inline int64
+static inline float8
 get_bucket_width(CAggTimebucketInfo bucket_info)
 {
-	int64 width = 0;
+	float8 width = 0;
 
 	/* Calculate the width. */
 	switch (bucket_info.bucket_width_type)
@@ -1132,7 +1133,7 @@ get_bucket_width(CAggTimebucketInfo bucket_info)
 		case INT8OID:
 		case INT4OID:
 		case INT2OID:
-			width = bucket_info.bucket_width;
+			width = (float8) bucket_info.bucket_width;
 			break;
 		case INTERVALOID:
 		{
@@ -1150,8 +1151,7 @@ get_bucket_width(CAggTimebucketInfo bucket_info)
 			Datum epoch = DirectFunctionCall2(interval_part,
 											  PointerGetDatum(cstring_to_text("epoch")),
 											  IntervalPGetDatum(bucket_info.interval));
-			/* Cast float8 to int8. */
-			width = DatumGetInt64(DirectFunctionCall1(dtoi8, epoch));
+			width = DatumGetFloat8(epoch);
 			break;
 		}
 		default:
@@ -1505,7 +1505,7 @@ cagg_validate_query(const Query *query, const bool finalized, const char *cagg_s
 	/* hierarchical cagg validations */
 	if (is_hierarchical)
 	{
-		int64 bucket_width = 0, bucket_width_parent = 0;
+		float8 bucket_width = 0, bucket_width_parent = 0;
 		bool is_greater_or_equal_than_parent = true, is_multiple_of_parent = true;
 
 		Assert(prev_query->groupClause);
@@ -1542,9 +1542,9 @@ cagg_validate_query(const Query *query, const bool finalized, const char *cagg_s
 		if (bucket_width_parent != 0)
 		{
 			if (bucket_width_parent > bucket_width && bucket_width != 0)
-				is_multiple_of_parent = ((bucket_width_parent % bucket_width) == 0);
+				is_multiple_of_parent = (fmod(bucket_width_parent, bucket_width) == 0);
 			else
-				is_multiple_of_parent = ((bucket_width % bucket_width_parent) == 0);
+				is_multiple_of_parent = (fmod(bucket_width, bucket_width_parent) == 0);
 		}
 
 		/* Proceed with validation errors. */

[konskov]

Thank you very much @fabriziomello!

Need more work on it