Prevent usage of time_bucket_ng in CAgg definition

The function timescaledb_experimental.time_bucket_ng() has been
deprecated for two years. This PR removes it from the list of bucketing
functions supported in a CAgg. Existing CAggs using this function will
still be supported; however, no new CAggs using this function can be
created.

.unreleased/pr_6798

@@ -0,0 +1 @@
+Implements: #6798 Prevent usage of deprecated time_bucket_ng in CAgg definition 

src/func_cache.c

@@ -366,7 +366,7 @@ static FuncInfo funcinfo[] = {
 	{
 		.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
 		.is_bucketing_func = true,
-		.allowed_in_cagg_definition = true,
+		.allowed_in_cagg_definition = false,
 		.funcname = "time_bucket_ng",
 		.nargs = 2,
 		.arg_types = { INTERVALOID, DATEOID },
@@ -376,7 +376,7 @@ static FuncInfo funcinfo[] = {
 	{
 		.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
 		.is_bucketing_func = true,
-		.allowed_in_cagg_definition = true,
+		.allowed_in_cagg_definition = false,
 		.funcname = "time_bucket_ng",
 		.nargs = 3,
 		.arg_types = { INTERVALOID, DATEOID, DATEOID },
@@ -386,7 +386,7 @@ static FuncInfo funcinfo[] = {
 	{
 		.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
 		.is_bucketing_func = true,
-		.allowed_in_cagg_definition = true,
+		.allowed_in_cagg_definition = false,
 		.funcname = "time_bucket_ng",
 		.nargs = 2,
 		.arg_types = { INTERVALOID, TIMESTAMPOID },
@@ -396,7 +396,7 @@ static FuncInfo funcinfo[] = {
 	{
 		.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
 		.is_bucketing_func = true,
-		.allowed_in_cagg_definition = true,
+		.allowed_in_cagg_definition = false,
 		.funcname = "time_bucket_ng",
 		.nargs = 3,
 		.arg_types = { INTERVALOID, TIMESTAMPOID, TIMESTAMPOID },
@@ -406,7 +406,7 @@ static FuncInfo funcinfo[] = {
 	{
 		.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
 		.is_bucketing_func = true,
-		.allowed_in_cagg_definition = true,
+		.allowed_in_cagg_definition = false,
 		.funcname = "time_bucket_ng",
 		.nargs = 3,
 		.arg_types = { INTERVALOID, TIMESTAMPTZOID, TEXTOID },
@@ -416,7 +416,7 @@ static FuncInfo funcinfo[] = {
 	{
 		.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
 		.is_bucketing_func = true,
-		.allowed_in_cagg_definition = true,
+		.allowed_in_cagg_definition = false,
 		.funcname = "time_bucket_ng",
 		.nargs = 4,
 		.arg_types = { INTERVALOID, TIMESTAMPTZOID, TIMESTAMPTZOID, TEXTOID },

src/guc.c

@@ -97,6 +97,7 @@ char *ts_last_tune_time = NULL;
 char *ts_last_tune_version = NULL;
 
 bool ts_guc_debug_require_batch_sorted_merge = false;
+bool ts_guc_debug_allow_cagg_with_deprecated_funcs = false;
 
 #ifdef TS_DEBUG
 bool ts_shutdown_bgw = false;
@@ -834,6 +835,17 @@ _guc_init(void)
 							 /* check_hook= */ NULL,
 							 /* assign_hook= */ NULL,
 							 /* show_hook= */ NULL);
+
+	DefineCustomBoolVariable(/* name= */ MAKE_EXTOPTION("debug_allow_cagg_with_deprecated_funcs"),
+							 /* short_desc= */ "allow new caggs using time_bucket_ng",
+							 /* long_desc= */ "this is for debugging/testing purposes",
+							 /* valueAddr= */ &ts_guc_debug_allow_cagg_with_deprecated_funcs,
+							 /* bootValue= */ false,
+							 /* context= */ PGC_USERSET,
+							 /* flags= */ 0,
+							 /* check_hook= */ NULL,
+							 /* assign_hook= */ NULL,
+							 /* show_hook= */ NULL);
 #endif
 
 	/* register feature flags */

src/guc.h

@@ -84,6 +84,8 @@ extern TSDLLEXPORT bool ts_guc_debug_compression_path_info;
 
 extern TSDLLEXPORT bool ts_guc_debug_require_batch_sorted_merge;
 
+extern TSDLLEXPORT bool ts_guc_debug_allow_cagg_with_deprecated_funcs;
+
 void _guc_init(void);
 
 typedef enum

src/ts_catalog/continuous_agg.c

@@ -1375,7 +1375,9 @@ ts_continuous_agg_bucket_on_interval(Oid bucket_function)
 	FuncInfo *func_info = ts_func_cache_get(bucket_function);
 	Ensure(func_info != NULL, "unable to get function info for Oid %d", bucket_function);
 
-	Assert(func_info->allowed_in_cagg_definition);
+	/* The function has to be a currently allowed function or one of the deprecated bucketing
+	 * functions */
+	Assert(func_info->allowed_in_cagg_definition || IS_DEPRECATED_BUCKET_FUNC(func_info));
 
 	Oid first_bucket_arg = func_info->arg_types[0];
 

src/ts_catalog/continuous_agg.h

@@ -40,6 +40,11 @@
 			SetUserIdAndSecContext(saved_uid, saved_secctx);                                       \
 	} while (0);
 
+/* Does the function belong to a bucket function that is no longer allowed in CAgg definitions? */
+#define IS_DEPRECATED_BUCKET_FUNC(funcinfo)                                                        \
+	((funcinfo->origin == ORIGIN_TIMESCALE_EXPERIMENTAL) &&                                        \
+	 (strcmp("time_bucket_ng", funcinfo->funcname) == 0))
+
 typedef enum ContinuousAggViewOption
 {
 	ContinuousEnabled = 0,

tsl/src/continuous_aggs/common.c

@@ -9,6 +9,8 @@
 #include <utils/date.h>
 #include <utils/timestamp.h>
 
+#include "guc.h"
+
 static Const *check_time_bucket_argument(Node *arg, char *position);
 static void caggtimebucketinfo_init(CAggTimebucketInfo *src, int32 hypertable_id,
 									Oid hypertable_oid, AttrNumber hypertable_partition_colno,
@@ -87,7 +89,15 @@ function_allowed_in_cagg_definition(Oid funcid)
 	if (finfo == NULL)
 		return false;
 
-	return finfo->allowed_in_cagg_definition;
+	if (finfo->allowed_in_cagg_definition)
+		return true;
+
+	/* Allow creation of CAggs with deprecated bucket function in debug builds for testing purposes
+	 */
+	if (ts_guc_debug_allow_cagg_with_deprecated_funcs && IS_DEPRECATED_BUCKET_FUNC(finfo))
+		return true;
+
+	return false;
 }
 
 /*
@@ -239,16 +249,28 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar
 			Node *width_arg;
 			Node *col_arg;
 
-			if (!function_allowed_in_cagg_definition(fe->funcid))
+			/* Filter any non bucketing functions */
+			FuncInfo *finfo = ts_func_cache_get_bucketing_func(fe->funcid);
+			if (finfo == NULL || !finfo->is_bucketing_func)
+			{
 				continue;
+			}
+
+			/* Do we have a bucketing function that is not allowed in the CAgg definition */
+			if (!function_allowed_in_cagg_definition(fe->funcid))
+			{
+				if (IS_DEPRECATED_BUCKET_FUNC(finfo))
+				{
+					ereport(ERROR,
+							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+							 errmsg("experimental bucket functions are not supported inside a CAgg "
+									"definition"),
+							 errhint("Use a function from the %s schema instead.",
+									 FUNCTIONS_SCHEMA_NAME)));
+				}
 
-			/*
-			 * Offset variants of time_bucket functions are not
-			 * supported at the moment.
-			 */
-			if (list_length(fe->args) >= 5 ||
-				(list_length(fe->args) == 4 && exprType(lfourth(fe->args)) == INTERVALOID))
 				continue;
+			}
 
 			if (found)
 				ereport(ERROR,
@@ -394,6 +416,15 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar
 		}
 	}
 
+	if (tbinfo->bucket_time_offset != NULL &&
+		TIMESTAMP_NOT_FINITE(tbinfo->bucket_time_origin) == false)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("using offset and origin in a time_bucket function at the same time is not "
+						"supported")));
+	}
+
 	if (!time_bucket_info_has_fixed_width(tbinfo))
 	{
 		/* Variable-sized buckets can be used only with intervals. */

tsl/test/expected/cagg_deprecated_bucket_ng.out

@@ -0,0 +1,49 @@
+-- This file and its contents are licensed under the Timescale License.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-TIMESCALE for a copy of the license.
+CREATE TABLE conditions(
+  time timestamptz NOT NULL,
+  city text NOT NULL,
+  temperature INT NOT NULL);
+SELECT create_hypertable(
+  'conditions', 'time',
+  chunk_time_interval => INTERVAL '1 day'
+);
+    create_hypertable    
+-------------------------
+ (1,public,conditions,t)
+(1 row)
+
+-- Ensure no CAgg using time_bucket_ng can be created
+\set ON_ERROR_STOP 0
+-- Regular CAgg
+CREATE MATERIALIZED VIEW conditions_summary_weekly
+WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
+SELECT city,
+       timescaledb_experimental.time_bucket_ng('7 days', time, 'UTC') AS bucket,
+       MIN(temperature),
+       MAX(temperature)
+FROM conditions
+GROUP BY city, bucket WITH NO DATA;
+ERROR:  experimental bucket functions are not supported inside a CAgg definition
+-- CAgg with origin
+CREATE MATERIALIZED VIEW conditions_summary_weekly
+WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
+SELECT city,
+       timescaledb_experimental.time_bucket_ng('7 days', time, '2024-01-16 18:00:00+00') AS bucket,
+       MIN(temperature),
+       MAX(temperature)
+FROM conditions
+GROUP BY city, bucket WITH NO DATA;
+ERROR:  experimental bucket functions are not supported inside a CAgg definition
+-- CAgg with origin and timezone
+CREATE MATERIALIZED VIEW conditions_summary_weekly
+WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
+SELECT city,
+       timescaledb_experimental.time_bucket_ng('7 days', time, '2024-01-16 18:00:00+00', 'UTC') AS bucket,
+       MIN(temperature),
+       MAX(temperature)
+FROM conditions
+GROUP BY city, bucket WITH NO DATA;
+ERROR:  experimental bucket functions are not supported inside a CAgg definition
+\set ON_ERROR_STOP 1

tsl/test/expected/cagg_query.out

@@ -1147,7 +1147,7 @@ CREATE MATERIALIZED VIEW cagg_4_hours_offset_and_origin
   SELECT time_bucket('4 hour', time, "offset"=>'30m'::interval, origin=>'2000-01-01 01:00:00 PST'::timestamptz, timezone=>'UTC'), max(value)
     FROM temperature
     GROUP BY 1 ORDER BY 1;
-ERROR:  continuous aggregate view must include a valid time bucket function
+ERROR:  using offset and origin in a time_bucket function at the same time is not supported
 \set ON_ERROR_STOP 1
 ---
 -- Tests with CAgg processing

tsl/test/expected/cagg_usage-13.out

@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
 ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 -- offset - not supported due to variable size
 CREATE MATERIALIZED VIEW cagg4 WITH (timescaledb.continuous,timescaledb.materialized_only=true) AS SELECT time_bucket('1 month', time, 'PST8PDT', "offset":= INTERVAL '15 day') FROM metrics GROUP BY 1;
-ERROR:  continuous aggregate view must include a valid time bucket function
+ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 \set ON_ERROR_STOP 1
 --
 -- drop chunks tests

tsl/test/expected/cagg_usage-14.out

@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
 ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 -- offset - not supported due to variable size
 CREATE MATERIALIZED VIEW cagg4 WITH (timescaledb.continuous,timescaledb.materialized_only=true) AS SELECT time_bucket('1 month', time, 'PST8PDT', "offset":= INTERVAL '15 day') FROM metrics GROUP BY 1;
-ERROR:  continuous aggregate view must include a valid time bucket function
+ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 \set ON_ERROR_STOP 1
 --
 -- drop chunks tests

tsl/test/expected/cagg_usage-15.out

@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
 ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 -- offset - not supported due to variable size
 CREATE MATERIALIZED VIEW cagg4 WITH (timescaledb.continuous,timescaledb.materialized_only=true) AS SELECT time_bucket('1 month', time, 'PST8PDT', "offset":= INTERVAL '15 day') FROM metrics GROUP BY 1;
-ERROR:  continuous aggregate view must include a valid time bucket function
+ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 \set ON_ERROR_STOP 1
 --
 -- drop chunks tests

tsl/test/expected/cagg_usage-16.out

@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
 ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 -- offset - not supported due to variable size
 CREATE MATERIALIZED VIEW cagg4 WITH (timescaledb.continuous,timescaledb.materialized_only=true) AS SELECT time_bucket('1 month', time, 'PST8PDT', "offset":= INTERVAL '15 day') FROM metrics GROUP BY 1;
-ERROR:  continuous aggregate view must include a valid time bucket function
+ERROR:  cannot create continuous aggregate with variable-width bucket using offset or origin.
 \set ON_ERROR_STOP 1
 --
 -- drop chunks tests

tsl/test/expected/cagg_utils.out

@@ -310,12 +310,6 @@ CREATE MATERIALIZED VIEW temperature_tz_4h_ts
     FROM timestamptz_ht
     GROUP BY 1 ORDER BY 1;
 NOTICE:  refreshing continuous aggregate "temperature_tz_4h_ts"
-CREATE MATERIALIZED VIEW temperature_tz_4h_ts_ng
-  WITH  (timescaledb.continuous) AS
-  SELECT timescaledb_experimental.time_bucket_ng('4 hour', time, 'Asia/Shanghai'), avg(value)
-    FROM timestamptz_ht
-    GROUP BY 1 ORDER BY 1;
-NOTICE:  refreshing continuous aggregate "temperature_tz_4h_ts_ng"
 CREATE TABLE integer_ht(a integer, b integer, c integer);
 SELECT table_name FROM create_hypertable('integer_ht', 'a', chunk_time_interval=> 10);
 NOTICE:  adding not-null constraint to column "a"
@@ -341,16 +335,15 @@ NOTICE:  continuous aggregate "integer_ht_cagg" is already up-to-date
 SELECT user_view_name,
        cagg_get_bucket_function(mat_hypertable_id)
        FROM _timescaledb_catalog.continuous_agg
-       WHERE user_view_name in('temperature_4h', 'temperature_tz_4h', 'temperature_tz_4h_ts', 'temperature_tz_4h_ts_ng', 'integer_ht_cagg')
+       WHERE user_view_name in('temperature_4h', 'temperature_tz_4h', 'temperature_tz_4h_ts', 'integer_ht_cagg')
        ORDER BY user_view_name;
-     user_view_name      |                               cagg_get_bucket_function                                
--------------------------+---------------------------------------------------------------------------------------
- integer_ht_cagg         | time_bucket(integer,integer)
- temperature_4h          | time_bucket(interval,timestamp without time zone)
- temperature_tz_4h       | time_bucket(interval,timestamp with time zone)
- temperature_tz_4h_ts    | time_bucket(interval,timestamp with time zone,text,timestamp with time zone,interval)
- temperature_tz_4h_ts_ng | timescaledb_experimental.time_bucket_ng(interval,timestamp with time zone,text)
-(5 rows)
+    user_view_name    |                               cagg_get_bucket_function                                
+----------------------+---------------------------------------------------------------------------------------
+ integer_ht_cagg      | time_bucket(integer,integer)
+ temperature_4h       | time_bucket(interval,timestamp without time zone)
+ temperature_tz_4h    | time_bucket(interval,timestamp with time zone)
+ temperature_tz_4h_ts | time_bucket(interval,timestamp with time zone,text,timestamp with time zone,interval)
+(4 rows)
 
 --- Cleanup
 \c :TEST_DBNAME :ROLE_SUPERUSER

tsl/test/expected/exp_cagg_monthly.out

@@ -505,7 +505,6 @@ ALTER TABLE conditions SET (
     timescaledb.compress,
     timescaledb.compress_segmentby = 'city'
 );
-NOTICE:  default order by for hypertable "conditions" is set to "day DESC"
 SELECT compress_chunk(ch) FROM show_chunks('conditions') AS ch;
               compress_chunk              
 ------------------------------------------