Skip to content

Commit 626975f

Browse files
jnidzwetzkijnidzwetzki
committed
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.
1 parent 2a30ca4 commit 626975f

25 files changed

+354
-46
lines changed

‎.unreleased/pr_6798

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Implements: #6798 Prevent usage of deprecated time_bucket_ng in CAgg definition

‎src/func_cache.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ static FuncInfo funcinfo[] = {
366366
{
367367
.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
368368
.is_bucketing_func = true,
369-
.allowed_in_cagg_definition = true,
369+
.allowed_in_cagg_definition = false,
370370
.funcname = "time_bucket_ng",
371371
.nargs = 2,
372372
.arg_types = { INTERVALOID, DATEOID },
@@ -376,7 +376,7 @@ static FuncInfo funcinfo[] = {
376376
{
377377
.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
378378
.is_bucketing_func = true,
379-
.allowed_in_cagg_definition = true,
379+
.allowed_in_cagg_definition = false,
380380
.funcname = "time_bucket_ng",
381381
.nargs = 3,
382382
.arg_types = { INTERVALOID, DATEOID, DATEOID },
@@ -386,7 +386,7 @@ static FuncInfo funcinfo[] = {
386386
{
387387
.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
388388
.is_bucketing_func = true,
389-
.allowed_in_cagg_definition = true,
389+
.allowed_in_cagg_definition = false,
390390
.funcname = "time_bucket_ng",
391391
.nargs = 2,
392392
.arg_types = { INTERVALOID, TIMESTAMPOID },
@@ -396,7 +396,7 @@ static FuncInfo funcinfo[] = {
396396
{
397397
.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
398398
.is_bucketing_func = true,
399-
.allowed_in_cagg_definition = true,
399+
.allowed_in_cagg_definition = false,
400400
.funcname = "time_bucket_ng",
401401
.nargs = 3,
402402
.arg_types = { INTERVALOID, TIMESTAMPOID, TIMESTAMPOID },
@@ -406,7 +406,7 @@ static FuncInfo funcinfo[] = {
406406
{
407407
.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
408408
.is_bucketing_func = true,
409-
.allowed_in_cagg_definition = true,
409+
.allowed_in_cagg_definition = false,
410410
.funcname = "time_bucket_ng",
411411
.nargs = 3,
412412
.arg_types = { INTERVALOID, TIMESTAMPTZOID, TEXTOID },
@@ -416,7 +416,7 @@ static FuncInfo funcinfo[] = {
416416
{
417417
.origin = ORIGIN_TIMESCALE_EXPERIMENTAL,
418418
.is_bucketing_func = true,
419-
.allowed_in_cagg_definition = true,
419+
.allowed_in_cagg_definition = false,
420420
.funcname = "time_bucket_ng",
421421
.nargs = 4,
422422
.arg_types = { INTERVALOID, TIMESTAMPTZOID, TIMESTAMPTZOID, TEXTOID },

‎src/guc.c

+12
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ char *ts_last_tune_time = NULL;
9797
char *ts_last_tune_version = NULL;
9898

9999
bool ts_guc_debug_require_batch_sorted_merge = false;
100+
bool ts_guc_debug_allow_cagg_with_deprecated_funcs = false;
100101

101102
#ifdef TS_DEBUG
102103
bool ts_shutdown_bgw = false;
@@ -834,6 +835,17 @@ _guc_init(void)
834835
/* check_hook= */ NULL,
835836
/* assign_hook= */ NULL,
836837
/* show_hook= */ NULL);
838+
839+
DefineCustomBoolVariable(/* name= */ MAKE_EXTOPTION("debug_allow_cagg_with_deprecated_funcs"),
840+
/* short_desc= */ "allow new caggs using time_bucket_ng",
841+
/* long_desc= */ "this is for debugging/testing purposes",
842+
/* valueAddr= */ &ts_guc_debug_allow_cagg_with_deprecated_funcs,
843+
/* bootValue= */ false,
844+
/* context= */ PGC_USERSET,
845+
/* flags= */ 0,
846+
/* check_hook= */ NULL,
847+
/* assign_hook= */ NULL,
848+
/* show_hook= */ NULL);
837849
#endif
838850

839851
/* register feature flags */

‎src/guc.h

+2
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ extern TSDLLEXPORT bool ts_guc_debug_compression_path_info;
8484

8585
extern TSDLLEXPORT bool ts_guc_debug_require_batch_sorted_merge;
8686

87+
extern TSDLLEXPORT bool ts_guc_debug_allow_cagg_with_deprecated_funcs;
88+
8789
void _guc_init(void);
8890

8991
typedef enum

‎src/ts_catalog/continuous_agg.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -1375,7 +1375,9 @@ ts_continuous_agg_bucket_on_interval(Oid bucket_function)
13751375
FuncInfo *func_info = ts_func_cache_get(bucket_function);
13761376
Ensure(func_info != NULL, "unable to get function info for Oid %d", bucket_function);
13771377

1378-
Assert(func_info->allowed_in_cagg_definition);
1378+
/* The function has to be a currently allowed function or one of the deprecated bucketing
1379+
* functions */
1380+
Assert(func_info->allowed_in_cagg_definition || IS_DEPRECATED_BUCKET_FUNC(func_info));
13791381

13801382
Oid first_bucket_arg = func_info->arg_types[0];
13811383

‎src/ts_catalog/continuous_agg.h

+5
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@
4040
SetUserIdAndSecContext(saved_uid, saved_secctx); \
4141
} while (0);
4242

43+
/* Does the function belong to a bucket function that is no longer allowed in CAgg definitions? */
44+
#define IS_DEPRECATED_BUCKET_FUNC(funcinfo) \
45+
((funcinfo->origin == ORIGIN_TIMESCALE_EXPERIMENTAL) && \
46+
(strcmp("time_bucket_ng", funcinfo->funcname) == 0))
47+
4348
typedef enum ContinuousAggViewOption
4449
{
4550
ContinuousEnabled = 0,

‎tsl/src/continuous_aggs/common.c

+39-8
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include <utils/date.h>
1010
#include <utils/timestamp.h>
1111

12+
#include "guc.h"
13+
1214
static Const *check_time_bucket_argument(Node *arg, char *position);
1315
static void caggtimebucketinfo_init(CAggTimebucketInfo *src, int32 hypertable_id,
1416
Oid hypertable_oid, AttrNumber hypertable_partition_colno,
@@ -87,7 +89,15 @@ function_allowed_in_cagg_definition(Oid funcid)
8789
if (finfo == NULL)
8890
return false;
8991

90-
return finfo->allowed_in_cagg_definition;
92+
if (finfo->allowed_in_cagg_definition)
93+
return true;
94+
95+
/* Allow creation of CAggs with deprecated bucket function in debug builds for testing purposes
96+
*/
97+
if (ts_guc_debug_allow_cagg_with_deprecated_funcs && IS_DEPRECATED_BUCKET_FUNC(finfo))
98+
return true;
99+
100+
return false;
91101
}
92102

93103
/*
@@ -239,16 +249,28 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar
239249
Node *width_arg;
240250
Node *col_arg;
241251

242-
if (!function_allowed_in_cagg_definition(fe->funcid))
252+
/* Filter any non bucketing functions */
253+
FuncInfo *finfo = ts_func_cache_get_bucketing_func(fe->funcid);
254+
if (finfo == NULL || !finfo->is_bucketing_func)
255+
{
243256
continue;
257+
}
258+
259+
/* Do we have a bucketing function that is not allowed in the CAgg definition */
260+
if (!function_allowed_in_cagg_definition(fe->funcid))
261+
{
262+
if (IS_DEPRECATED_BUCKET_FUNC(finfo))
263+
{
264+
ereport(ERROR,
265+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
266+
errmsg("experimental bucket functions are not supported inside a CAgg "
267+
"definition"),
268+
errhint("Use a function from the %s schema instead.",
269+
FUNCTIONS_SCHEMA_NAME)));
270+
}
244271

245-
/*
246-
* Offset variants of time_bucket functions are not
247-
* supported at the moment.
248-
*/
249-
if (list_length(fe->args) >= 5 ||
250-
(list_length(fe->args) == 4 && exprType(lfourth(fe->args)) == INTERVALOID))
251272
continue;
273+
}
252274

253275
if (found)
254276
ereport(ERROR,
@@ -394,6 +416,15 @@ caggtimebucket_validate(CAggTimebucketInfo *tbinfo, List *groupClause, List *tar
394416
}
395417
}
396418

419+
if (tbinfo->bucket_time_offset != NULL &&
420+
TIMESTAMP_NOT_FINITE(tbinfo->bucket_time_origin) == false)
421+
{
422+
ereport(ERROR,
423+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
424+
errmsg("using offset and origin in a time_bucket function at the same time is not "
425+
"supported")));
426+
}
427+
397428
if (!time_bucket_info_has_fixed_width(tbinfo))
398429
{
399430
/* Variable-sized buckets can be used only with intervals. */

‎tsl/test/expected/cagg_deprecated_bucket_ng.out

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
-- This file and its contents are licensed under the Timescale License.
2+
-- Please see the included NOTICE for copyright information and
3+
-- LICENSE-TIMESCALE for a copy of the license.
4+
CREATE TABLE conditions(
5+
time timestamptz NOT NULL,
6+
city text NOT NULL,
7+
temperature INT NOT NULL);
8+
SELECT create_hypertable(
9+
'conditions', 'time',
10+
chunk_time_interval => INTERVAL '1 day'
11+
);
12+
create_hypertable
13+
-------------------------
14+
(1,public,conditions,t)
15+
(1 row)
16+
17+
-- Ensure no CAgg using time_bucket_ng can be created
18+
\set ON_ERROR_STOP 0
19+
-- Regular CAgg
20+
CREATE MATERIALIZED VIEW conditions_summary_weekly
21+
WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
22+
SELECT city,
23+
timescaledb_experimental.time_bucket_ng('7 days', time, 'UTC') AS bucket,
24+
MIN(temperature),
25+
MAX(temperature)
26+
FROM conditions
27+
GROUP BY city, bucket WITH NO DATA;
28+
ERROR: experimental bucket functions are not supported inside a CAgg definition
29+
-- CAgg with origin
30+
CREATE MATERIALIZED VIEW conditions_summary_weekly
31+
WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
32+
SELECT city,
33+
timescaledb_experimental.time_bucket_ng('7 days', time, '2024-01-16 18:00:00+00') AS bucket,
34+
MIN(temperature),
35+
MAX(temperature)
36+
FROM conditions
37+
GROUP BY city, bucket WITH NO DATA;
38+
ERROR: experimental bucket functions are not supported inside a CAgg definition
39+
-- CAgg with origin and timezone
40+
CREATE MATERIALIZED VIEW conditions_summary_weekly
41+
WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS
42+
SELECT city,
43+
timescaledb_experimental.time_bucket_ng('7 days', time, '2024-01-16 18:00:00+00', 'UTC') AS bucket,
44+
MIN(temperature),
45+
MAX(temperature)
46+
FROM conditions
47+
GROUP BY city, bucket WITH NO DATA;
48+
ERROR: experimental bucket functions are not supported inside a CAgg definition
49+
\set ON_ERROR_STOP 1

‎tsl/test/expected/cagg_query.out

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

‎tsl/test/expected/cagg_usage-13.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
473473
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
474474
-- offset - not supported due to variable size
475475
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;
476-
ERROR: continuous aggregate view must include a valid time bucket function
476+
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
477477
\set ON_ERROR_STOP 1
478478
--
479479
-- drop chunks tests

‎tsl/test/expected/cagg_usage-14.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
473473
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
474474
-- offset - not supported due to variable size
475475
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;
476-
ERROR: continuous aggregate view must include a valid time bucket function
476+
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
477477
\set ON_ERROR_STOP 1
478478
--
479479
-- drop chunks tests

‎tsl/test/expected/cagg_usage-15.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
473473
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
474474
-- offset - not supported due to variable size
475475
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;
476-
ERROR: continuous aggregate view must include a valid time bucket function
476+
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
477477
\set ON_ERROR_STOP 1
478478
--
479479
-- drop chunks tests

‎tsl/test/expected/cagg_usage-16.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ CREATE MATERIALIZED VIEW cagg3 WITH (timescaledb.continuous,timescaledb.material
473473
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
474474
-- offset - not supported due to variable size
475475
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;
476-
ERROR: continuous aggregate view must include a valid time bucket function
476+
ERROR: cannot create continuous aggregate with variable-width bucket using offset or origin.
477477
\set ON_ERROR_STOP 1
478478
--
479479
-- drop chunks tests

‎tsl/test/expected/cagg_utils.out

+8-15
Original file line numberDiff line numberDiff line change
@@ -310,12 +310,6 @@ CREATE MATERIALIZED VIEW temperature_tz_4h_ts
310310
FROM timestamptz_ht
311311
GROUP BY 1 ORDER BY 1;
312312
NOTICE: refreshing continuous aggregate "temperature_tz_4h_ts"
313-
CREATE MATERIALIZED VIEW temperature_tz_4h_ts_ng
314-
WITH (timescaledb.continuous) AS
315-
SELECT timescaledb_experimental.time_bucket_ng('4 hour', time, 'Asia/Shanghai'), avg(value)
316-
FROM timestamptz_ht
317-
GROUP BY 1 ORDER BY 1;
318-
NOTICE: refreshing continuous aggregate "temperature_tz_4h_ts_ng"
319313
CREATE TABLE integer_ht(a integer, b integer, c integer);
320314
SELECT table_name FROM create_hypertable('integer_ht', 'a', chunk_time_interval=> 10);
321315
NOTICE: adding not-null constraint to column "a"
@@ -341,16 +335,15 @@ NOTICE: continuous aggregate "integer_ht_cagg" is already up-to-date
341335
SELECT user_view_name,
342336
cagg_get_bucket_function(mat_hypertable_id)
343337
FROM _timescaledb_catalog.continuous_agg
344-
WHERE user_view_name in('temperature_4h', 'temperature_tz_4h', 'temperature_tz_4h_ts', 'temperature_tz_4h_ts_ng', 'integer_ht_cagg')
338+
WHERE user_view_name in('temperature_4h', 'temperature_tz_4h', 'temperature_tz_4h_ts', 'integer_ht_cagg')
345339
ORDER BY user_view_name;
346-
user_view_name | cagg_get_bucket_function
347-
-------------------------+---------------------------------------------------------------------------------------
348-
integer_ht_cagg | time_bucket(integer,integer)
349-
temperature_4h | time_bucket(interval,timestamp without time zone)
350-
temperature_tz_4h | time_bucket(interval,timestamp with time zone)
351-
temperature_tz_4h_ts | time_bucket(interval,timestamp with time zone,text,timestamp with time zone,interval)
352-
temperature_tz_4h_ts_ng | timescaledb_experimental.time_bucket_ng(interval,timestamp with time zone,text)
353-
(5 rows)
340+
user_view_name | cagg_get_bucket_function
341+
----------------------+---------------------------------------------------------------------------------------
342+
integer_ht_cagg | time_bucket(integer,integer)
343+
temperature_4h | time_bucket(interval,timestamp without time zone)
344+
temperature_tz_4h | time_bucket(interval,timestamp with time zone)
345+
temperature_tz_4h_ts | time_bucket(interval,timestamp with time zone,text,timestamp with time zone,interval)
346+
(4 rows)
354347

355348
--- Cleanup
356349
\c :TEST_DBNAME :ROLE_SUPERUSER

0 commit comments

Comments
 (0)