Fix CAgg on CAgg variable bucket size validation

Previous attempt to fix it (PR #5130) was not entirely correct because
the bucket width calculation for interval width was wrong.

Fixed it by properly calculate the bucket width for intervals using the
Postgres internal function `interval_part` to extract the epoch of the
interval and execute the validations. For integer widths use the already
calculated bucket width.

Fixes #5158, #5168

CHANGELOG.md

@@ -9,14 +9,14 @@ accidentally triggering the load of a previous DB version.**
 **Bugfixes**
 * #4926 Fix corruption when inserting into compressed chunks
 * #5114 Fix issue with deleting data node and dropping database
-* #5130 Fix CAgg on CAgg variable bucket size validation
 * #5133 Fix CAgg on CAgg using different column order on the original hypertable
 * #5152 Fix adding column with NULL constraint to compressed hypertable
+* #5170 Fix CAgg on CAgg variable bucket size validation
 
 **Thanks**
 * @ikkala for reporting error when adding column with NULL constraint to compressed hypertable
 * @salquier-appvizer for reporting error on CAgg on CAgg using different column order on the original hypertable
-* @ssmoss for reporting error on CAgg on CAgg variable bucket size validation
+* @ssmoss, @adbnexxtlab and @ivanzamanov for reporting error on CAgg on CAgg variable bucket size validation
 
 ## 2.9.1 (2022-12-23)
 

tsl/src/continuous_aggs/create.c

@@ -1099,24 +1099,53 @@ cagg_query_supported(const Query *query, StringInfo hint, StringInfo detail, con
 static inline int64
 get_bucket_width(CAggTimebucketInfo bucket_info)
 {
-	if (bucket_info.bucket_width == BUCKET_WIDTH_VARIABLE)
+	int64 width = 0;
+
+	/* calculate the width */
+	switch (bucket_info.bucket_width_type)
 	{
-		/* calculate the width */
+		case INT8OID:
-		return ts_interval_value_to_internal(IntervalPGetDatum(bucket_info.interval),
+		case INT4OID:
-											 bucket_info.bucket_width_type);
+		case INT2OID:
+			width = bucket_info.bucket_width;
+			break;
+		case INTERVALOID:
+		{
+			Datum epoch = DirectFunctionCall2(interval_part,
+											  PointerGetDatum(cstring_to_text("epoch")),
+											  IntervalPGetDatum(bucket_info.interval));
+			/* cast float8 to int8 */
+			width = DatumGetInt64(DirectFunctionCall1(dtoi8, epoch));
+			break;
+		}
+		default:
+			Assert(false);
 	}
 
-	return bucket_info.bucket_width;
+	return width;
 }
 
 static inline Datum
 get_bucket_width_datum(CAggTimebucketInfo bucket_info)
 {
-	if (bucket_info.bucket_width == BUCKET_WIDTH_VARIABLE)
+	Datum width = (Datum) 0;
-		return IntervalPGetDatum(bucket_info.interval);
 
-	return ts_internal_to_interval_value(get_bucket_width(bucket_info),
+	switch (bucket_info.bucket_width_type)
-										 bucket_info.bucket_width_type);
+	{
+		case INT8OID:
+		case INT4OID:
+		case INT2OID:
+			width = ts_internal_to_interval_value(bucket_info.bucket_width,
+												  bucket_info.bucket_width_type);
+			break;
+		case INTERVALOID:
+			width = IntervalPGetDatum(bucket_info.interval);
+			break;
+		default:
+			Assert(false);
+	}
+
+	return width;
 }
 
 static CAggTimebucketInfo
@@ -1320,8 +1349,8 @@ cagg_validate_query(const Query *query, const bool finalized, const char *cagg_s
 	/* nested cagg validations */
 	if (is_nested)
 	{
-		int64 bucket_width, bucket_width_parent;
+		int64 bucket_width = 0, bucket_width_parent = 0;
-		bool is_greater_or_equal_than_parent, is_multiple_of_parent;
+		bool is_greater_or_equal_than_parent = true, is_multiple_of_parent = true;
 
 		Assert(prev_query->groupClause);
 		caggtimebucket_validate(&bucket_info_parent,
@@ -1354,7 +1383,13 @@ cagg_validate_query(const Query *query, const bool finalized, const char *cagg_s
 		is_greater_or_equal_than_parent = (bucket_width >= bucket_width_parent);
 
 		/* check if buckets are multiple */
-		is_multiple_of_parent = ((bucket_width % bucket_width_parent) == 0);
+		if (bucket_width_parent != 0)
+		{
+			if (bucket_width_parent > bucket_width && bucket_width != 0)
+				is_multiple_of_parent = ((bucket_width_parent % bucket_width) == 0);
+			else
+				is_multiple_of_parent = ((bucket_width % bucket_width_parent) == 0);
+		}
 
 		/* proceed with validation errors */
 		if (!is_greater_or_equal_than_parent || !is_multiple_of_parent)
@@ -1373,14 +1408,14 @@ cagg_validate_query(const Query *query, const bool finalized, const char *cagg_s
 			width_parent = get_bucket_width_datum(bucket_info_parent);
 			width_out_parent = DatumGetCString(OidFunctionCall1(outfuncid, width_parent));
 
-			/* new bucket should be greater than the parent */
-			if (!is_greater_or_equal_than_parent)
-				message = "greater than";
-
 			/* new bucket should be multiple of the parent */
 			if (!is_multiple_of_parent)
 				message = "multiple of";
 
+			/* new bucket should be greater than the parent */
+			if (!is_greater_or_equal_than_parent)
+				message = "greater or equal than";
+
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("cannot create continuous aggregate with incompatible bucket width"),

tsl/test/sql/cagg_on_cagg.sql

@@ -4,7 +4,8 @@
 
 -- Global test variables
 \set IS_DISTRIBUTED FALSE
-\set IS_TIME_DIMENSION_WITH_TIMEZONE FALSE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST FALSE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH FALSE
 
 -- ########################################################
 -- ## INTEGER data type tests
@@ -183,20 +184,63 @@ SET timezone TO 'UTC';
 \ir include/cagg_on_cagg_validations.sql
 
 --
--- -- Validations using time bucket with timezone (ref issue #5126)
+-- Validations using time bucket with timezone (ref issue #5126)
 --
 \set TIME_DIMENSION_DATATYPE TIMESTAMPTZ
-\set IS_TIME_DIMENSION_WITH_TIMEZONE TRUE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST TRUE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH TRUE
 \set CAGG_NAME_1ST_LEVEL conditions_summary_1_5m
 \set CAGG_NAME_2TH_LEVEL conditions_summary_2_1h
-\set BUCKET_TZNAME 'US/Pacific'
+\set BUCKET_TZNAME_1ST 'US/Pacific'
+\set BUCKET_TZNAME_2TH 'US/Pacific'
 \set BUCKET_WIDTH_1ST 'INTERVAL \'5 minutes\''
 \set BUCKET_WIDTH_2TH 'INTERVAL \'1 hour\''
 \set WARNING_MESSAGE '-- SHOULD WORK'
 \ir include/cagg_on_cagg_validations.sql
 
-\set BUCKET_TZNAME 'US/Pacific'
 \set BUCKET_WIDTH_1ST 'INTERVAL \'5 minutes\''
 \set BUCKET_WIDTH_2TH 'INTERVAL \'16 minutes\''
 \set WARNING_MESSAGE '-- SHOULD ERROR because non-multiple bucket sizes'
 \ir include/cagg_on_cagg_validations.sql
+
+--
+-- Variable bucket size with the same timezones
+--
+\set BUCKET_TZNAME_1ST 'UTC'
+\set BUCKET_TZNAME_2TH 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql
+
+--
+-- Variable bucket size with different timezones
+--
+\set BUCKET_TZNAME_1ST 'US/Pacific'
+\set BUCKET_TZNAME_2TH 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql
+
+--
+-- TZ bucket on top of non-TZ bucket
+--
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST FALSE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH TRUE
+\set BUCKET_TZNAME_2TH 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql
+
+--
+-- non-TZ bucket on top of TZ bucket
+--
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST TRUE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH FALSE
+\set BUCKET_TZNAME_1ST 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql

tsl/test/sql/cagg_on_cagg_dist_ht.sql

@@ -22,7 +22,8 @@ GRANT CREATE ON SCHEMA public TO :ROLE_DEFAULT_PERM_USER;
 
 -- Global test variables
 \set IS_DISTRIBUTED TRUE
-\set IS_TIME_DIMENSION_WITH_TIMEZONE FALSE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST FALSE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH FALSE
 
 -- ########################################################
 -- ## INTEGER data type tests
@@ -206,21 +207,64 @@ SET timezone TO 'UTC';
 -- Validations using time bucket with timezone (ref issue #5126)
 --
 \set TIME_DIMENSION_DATATYPE TIMESTAMPTZ
-\set IS_TIME_DIMENSION_WITH_TIMEZONE TRUE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST TRUE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH TRUE
 \set CAGG_NAME_1ST_LEVEL conditions_summary_1_5m
 \set CAGG_NAME_2TH_LEVEL conditions_summary_2_1h
-\set BUCKET_TZNAME 'US/Pacific'
+\set BUCKET_TZNAME_1ST 'US/Pacific'
+\set BUCKET_TZNAME_2TH 'US/Pacific'
 \set BUCKET_WIDTH_1ST 'INTERVAL \'5 minutes\''
 \set BUCKET_WIDTH_2TH 'INTERVAL \'1 hour\''
 \set WARNING_MESSAGE '-- SHOULD WORK'
 \ir include/cagg_on_cagg_validations.sql
 
-\set BUCKET_TZNAME 'US/Pacific'
 \set BUCKET_WIDTH_1ST 'INTERVAL \'5 minutes\''
 \set BUCKET_WIDTH_2TH 'INTERVAL \'16 minutes\''
 \set WARNING_MESSAGE '-- SHOULD ERROR because non-multiple bucket sizes'
 \ir include/cagg_on_cagg_validations.sql
 
+--
+-- Variable bucket size with the same timezones
+--
+\set BUCKET_TZNAME_1ST 'UTC'
+\set BUCKET_TZNAME_2TH 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql
+
+--
+-- Variable bucket size with different timezones
+--
+\set BUCKET_TZNAME_1ST 'US/Pacific'
+\set BUCKET_TZNAME_2TH 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql
+
+--
+-- TZ bucket on top of non-TZ bucket
+--
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST FALSE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH TRUE
+\set BUCKET_TZNAME_2TH 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql
+
+--
+-- non-TZ bucket on top of TZ bucket
+--
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_1ST TRUE
+\set IS_TIME_DIMENSION_WITH_TIMEZONE_2TH FALSE
+\set BUCKET_TZNAME_1ST 'UTC'
+\set BUCKET_WIDTH_1ST 'INTERVAL \'1 day\''
+\set BUCKET_WIDTH_2TH 'INTERVAL \'1 month\''
+\set WARNING_MESSAGE '-- SHOULD WORK'
+\ir include/cagg_on_cagg_validations.sql
+
 -- Cleanup
 \c :TEST_DBNAME :ROLE_CLUSTER_SUPERUSER;
 DROP DATABASE :DATA_NODE_1;

tsl/test/sql/include/cagg_on_cagg_validations.sql

@@ -11,8 +11,8 @@
 CREATE MATERIALIZED VIEW :CAGG_NAME_1ST_LEVEL
 WITH (timescaledb.continuous) AS
 SELECT
-  \if :IS_TIME_DIMENSION_WITH_TIMEZONE
+  \if :IS_TIME_DIMENSION_WITH_TIMEZONE_1ST
-    time_bucket(:BUCKET_WIDTH_1ST, "time", :'BUCKET_TZNAME') AS bucket,
+    time_bucket(:BUCKET_WIDTH_1ST, "time", :'BUCKET_TZNAME_1ST') AS bucket,
   \else
     time_bucket(:BUCKET_WIDTH_1ST, "time") AS bucket,
   \endif
@@ -21,6 +21,8 @@ FROM conditions
 GROUP BY 1
 WITH NO DATA;
 
+\d+ :CAGG_NAME_1ST_LEVEL
+
 --
 -- CAGG on CAGG (2th level)
 --
@@ -30,8 +32,8 @@ WITH NO DATA;
 CREATE MATERIALIZED VIEW :CAGG_NAME_2TH_LEVEL
 WITH (timescaledb.continuous) AS
 SELECT
-  \if :IS_TIME_DIMENSION_WITH_TIMEZONE
+  \if :IS_TIME_DIMENSION_WITH_TIMEZONE_2TH
-    time_bucket(:BUCKET_WIDTH_2TH, "bucket", :'BUCKET_TZNAME') AS bucket,
+    time_bucket(:BUCKET_WIDTH_2TH, "bucket", :'BUCKET_TZNAME_2TH') AS bucket,
   \else
     time_bucket(:BUCKET_WIDTH_2TH, "bucket") AS bucket,
   \endif
@@ -39,7 +41,10 @@ SELECT
 FROM :CAGG_NAME_1ST_LEVEL
 GROUP BY 1
 WITH NO DATA;
-\set ON_ERROR_STOP 0
+
+\d+ :CAGG_NAME_2TH_LEVEL
+
+\set ON_ERROR_STOP 1
 \set VERBOSITY terse
 
 --