diff --git a/sql/updates/latest-dev.sql b/sql/updates/latest-dev.sql index 5763c6a3572..b3e75e27d1c 100644 --- a/sql/updates/latest-dev.sql +++ b/sql/updates/latest-dev.sql @@ -146,3 +146,8 @@ DROP FUNCTION IF EXISTS _timescaledb_functions.cagg_get_bucket_function(INTEGER) UPDATE _timescaledb_catalog.continuous_aggs_bucket_function SET bucket_origin = bucket_origin::timestamp::timestamptz::text WHERE length(bucket_origin) > 1; + +-- Historically, we have used empty strings for undefined bucket_origin and timezone +-- attributes. This is now replaced by proper NULL values. We use TRIM() to ensure we handle empty string well. +UPDATE _timescaledb_catalog.continuous_aggs_bucket_function SET bucket_origin = NULL WHERE TRIM(bucket_origin) = ''; +UPDATE _timescaledb_catalog.continuous_aggs_bucket_function SET bucket_timezone = NULL WHERE TRIM(bucket_timezone) = ''; diff --git a/sql/updates/reverse-dev.sql b/sql/updates/reverse-dev.sql index 1b580190b8c..9f9a53f77b5 100644 --- a/sql/updates/reverse-dev.sql +++ b/sql/updates/reverse-dev.sql @@ -3,6 +3,9 @@ DROP FUNCTION IF EXISTS _timescaledb_functions.remove_dropped_chunk_metadata(INT -- -- Rebuild the catalog table `_timescaledb_catalog.continuous_aggs_bucket_function` -- +UPDATE _timescaledb_catalog.continuous_aggs_bucket_function SET bucket_origin = '' WHERE bucket_origin IS NULL; +UPDATE _timescaledb_catalog.continuous_aggs_bucket_function SET bucket_timezone = '' WHERE bucket_timezone IS NULL; + CREATE TABLE _timescaledb_catalog._tmp_continuous_aggs_bucket_function AS SELECT mat_hypertable_id, diff --git a/src/ts_catalog/continuous_agg.c b/src/ts_catalog/continuous_agg.c index 73cca4edcbe..c5c7551f0a0 100644 --- a/src/ts_catalog/continuous_agg.c +++ b/src/ts_catalog/continuous_agg.c @@ -413,12 +413,10 @@ continuous_agg_fill_bucket_function(int32 mat_hypertable_id, ContinuousAggsBucke init_scan_cagg_bucket_function_by_mat_hypertable_id(&iterator, mat_hypertable_id); ts_scanner_foreach(&iterator) { - const char *bucket_width_str; - const char *origin_str; Datum values[Natts_continuous_aggs_bucket_function]; bool isnull[Natts_continuous_aggs_bucket_function]; - bool should_free; + HeapTuple tuple = ts_scan_iterator_fetch_heap_tuple(&iterator, false, &should_free); /* @@ -427,6 +425,7 @@ continuous_agg_fill_bucket_function(int32 mat_hypertable_id, ContinuousAggsBucke */ heap_deform_tuple(tuple, ts_scan_iterator_tupledesc(&iterator), values, isnull); + /* Bucket function */ Assert(!isnull[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_function)]); bf->bucket_function = DatumGetObjectId( values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_function)]); @@ -434,34 +433,41 @@ continuous_agg_fill_bucket_function(int32 mat_hypertable_id, ContinuousAggsBucke Assert(OidIsValid(bf->bucket_function)); /* - * So far bucket_width is stored as TEXT for flexibility, but it's type - * most likely is going to change to Interval when the variable-sized - * buckets feature will stabilize. + * bucket_width + * + * The value is stored as TEXT since we have to store the interval value of time + * buckets and also the number value of integer based buckets. */ Assert(!isnull[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_width)]); - bucket_width_str = TextDatumGetCString( + const char *bucket_width_str = TextDatumGetCString( values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_width)]); Assert(strlen(bucket_width_str) > 0); bf->bucket_width = DatumGetIntervalP( DirectFunctionCall3(interval_in, CStringGetDatum(bucket_width_str), InvalidOid, -1)); - Assert( - !isnull[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_origin)]); - origin_str = TextDatumGetCString( - values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_origin)]); - if (strlen(origin_str) == 0) - TIMESTAMP_NOBEGIN(bf->bucket_origin); - else + /* Bucket origin */ + if (!isnull[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_origin)]) + { + const char *origin_str = TextDatumGetCString(values[AttrNumberGetAttrOffset( + Anum_continuous_aggs_bucket_function_bucket_origin)]); bf->bucket_origin = DatumGetTimestamp(DirectFunctionCall3(timestamptz_in, CStringGetDatum(origin_str), ObjectIdGetDatum(InvalidOid), Int32GetDatum(-1))); + } + else + { + TIMESTAMP_NOBEGIN(bf->bucket_origin); + } - Assert( - !isnull[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_timezone)]); - bf->timezone = TextDatumGetCString( - values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_timezone)]); + /* Bucket timezone */ + if (!isnull[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_timezone)]) + { + bf->timezone = TextDatumGetCString(values[AttrNumberGetAttrOffset( + Anum_continuous_aggs_bucket_function_bucket_timezone)]); + } + /* Bucket fixed width */ Assert(!isnull[AttrNumberGetAttrOffset( Anum_continuous_aggs_bucket_function_bucket_fixed_width)]); bf->bucket_fixed_interval = DatumGetBool(values[AttrNumberGetAttrOffset( @@ -1351,16 +1357,13 @@ ts_continuous_agg_bucket_width(const ContinuousAgg *agg) static Datum generic_time_bucket(const ContinuousAggsBucketFunction *bf, Datum timestamp) { - /* bf->timezone can't be NULL. If timezone is not specified, "" is stored */ - Assert(bf->timezone != NULL); - FuncInfo *func_info = ts_func_cache_get_bucketing_func(bf->bucket_function); Ensure(func_info != NULL, "unable to get bucket function for Oid %d", bf->bucket_function); bool is_experimental = func_info->origin == ORIGIN_TIMESCALE_EXPERIMENTAL; if (!is_experimental) { - if (strlen(bf->timezone) > 0) + if (bf->timezone != NULL) { if (TIMESTAMP_NOT_FINITE(bf->bucket_origin)) { @@ -1399,7 +1402,7 @@ generic_time_bucket(const ContinuousAggsBucketFunction *bf, Datum timestamp) } else { - if (strlen(bf->timezone) > 0) + if (bf->timezone != NULL) { if (TIMESTAMP_NOT_FINITE(bf->bucket_origin)) { @@ -1449,12 +1452,7 @@ static Datum generic_add_interval(const ContinuousAggsBucketFunction *bf, Datum timestamp) { Datum tzname = 0; - bool has_timezone; - - /* bf->timezone can't be NULL. If timezone is not specified, "" is stored */ - Assert(bf->timezone != NULL); - - has_timezone = (strlen(bf->timezone) > 0); + bool has_timezone = (bf->timezone != NULL); if (has_timezone) { diff --git a/src/ts_catalog/continuous_agg.h b/src/ts_catalog/continuous_agg.h index 3d9e869f50a..7f2ccd8173d 100644 --- a/src/ts_catalog/continuous_agg.h +++ b/src/ts_catalog/continuous_agg.h @@ -93,7 +93,7 @@ typedef struct ContinuousAggsBucketFunction TimestampTz bucket_origin; /* `bucket_offset` argument of the function. */ - Interval *bucket_offest; + Interval *bucket_offset; /* `timezone` argument of the function provided by the user. */ char *timezone; diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c index 9f9c8cf31a7..355497b03fd 100644 --- a/tsl/src/continuous_aggs/create.c +++ b/tsl/src/continuous_aggs/create.c @@ -188,8 +188,8 @@ create_cagg_catalog_entry(int32 matht_id, int32 rawht_id, const char *user_schem */ static void create_bucket_function_catalog_entry(int32 matht_id, Oid bucket_function, const char *bucket_width, - const char *origin, const char *offset, const char *timezone, - const bool bucket_fixed_width) + const char *bucket_origin, const char *bucket_offset, + const char *bucket_timezone, const bool bucket_fixed_width) { Catalog *catalog = ts_catalog_get(); Relation rel; @@ -203,18 +203,53 @@ create_bucket_function_catalog_entry(int32 matht_id, Oid bucket_function, const desc = RelationGetDescr(rel); memset(values, 0, sizeof(values)); + + /* Hypertable ID */ values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_mat_hypertable_id)] = matht_id; + + /* Bucket function */ values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_function)] = ObjectIdGetDatum(bucket_function); + + /* Bucket width */ values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_width)] = CStringGetTextDatum(bucket_width); - values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_origin)] = - CStringGetTextDatum(origin); - values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_offset)] = - CStringGetTextDatum(offset); - values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_timezone)] = - CStringGetTextDatum(timezone ? timezone : ""); + + /* Bucket origin */ + if (bucket_origin != NULL) + { + values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_origin)] = + CStringGetTextDatum(bucket_origin); + } + else + { + nulls[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_origin)] = true; + } + + /* Bucket offset */ + if (bucket_offset != NULL) + { + values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_offset)] = + CStringGetTextDatum(bucket_offset); + } + else + { + nulls[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_offset)] = true; + } + + /* Bucket timezone */ + if (bucket_timezone != NULL) + { + values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_timezone)] = + CStringGetTextDatum(bucket_timezone); + } + else + { + nulls[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_timezone)] = true; + } + + /* Bucket fixed width */ values[AttrNumberGetAttrOffset(Anum_continuous_aggs_bucket_function_bucket_fixed_width)] = BoolGetDatum(bucket_fixed_width); @@ -784,28 +819,27 @@ cagg_create(const CreateTableAsStmt *create_stmt, ViewStmt *stmt, Query *panquer if (bucket_info->bucket_width == BUCKET_WIDTH_VARIABLE) { - const char *bucket_width; - const char *origin = ""; - const char *offset = ""; + const char *bucket_origin = NULL; + const char *bucket_offset = NULL; /* * Variable-sized buckets work only with intervals. */ Assert(bucket_info->interval != NULL); - bucket_width = DatumGetCString( + const char *bucket_width = DatumGetCString( DirectFunctionCall1(interval_out, IntervalPGetDatum(bucket_info->interval))); if (!TIMESTAMP_NOT_FINITE(bucket_info->origin)) { - origin = DatumGetCString( + bucket_origin = DatumGetCString( DirectFunctionCall1(timestamptz_out, TimestampTzGetDatum(bucket_info->origin))); } create_bucket_function_catalog_entry(materialize_hypertable_id, bucket_info->bucket_func->funcid, bucket_width, - origin, - offset, + bucket_origin, + bucket_offset, bucket_info->timezone, bucket_info->bucket_width != BUCKET_WIDTH_VARIABLE); } diff --git a/tsl/test/expected/exp_cagg_monthly.out b/tsl/test/expected/exp_cagg_monthly.out index 07da250cdf7..a6579b528be 100644 --- a/tsl/test/expected/exp_cagg_monthly.out +++ b/tsl/test/expected/exp_cagg_monthly.out @@ -70,14 +70,16 @@ WHERE mat_hypertable_id = :cagg_id; -1 (1 row) -SELECT bucket_func, bucket_width, bucket_origin, bucket_timezone, bucket_fixed_width +\pset null +SELECT * FROM _timescaledb_catalog.continuous_aggs_bucket_function WHERE mat_hypertable_id = :cagg_id; - bucket_func | bucket_width | bucket_origin | bucket_timezone | bucket_fixed_width ---------------------------------------------------------+--------------+---------------+-----------------+-------------------- - timescaledb_experimental.time_bucket_ng(interval,date) | @ 1 mon | | | f + mat_hypertable_id | bucket_func | bucket_width | bucket_origin | bucket_offset | bucket_timezone | bucket_fixed_width +-------------------+--------------------------------------------------------+--------------+---------------+---------------+-----------------+-------------------- + 2 | timescaledb_experimental.time_bucket_ng(interval,date) | @ 1 mon | | | | f (1 row) +\pset null "" -- Check that the saved invalidation threshold is -infinity SELECT _timescaledb_functions.to_timestamp(watermark) FROM _timescaledb_catalog.continuous_aggs_invalidation_threshold diff --git a/tsl/test/sql/exp_cagg_monthly.sql b/tsl/test/sql/exp_cagg_monthly.sql index 71c92192619..1cab569b908 100644 --- a/tsl/test/sql/exp_cagg_monthly.sql +++ b/tsl/test/sql/exp_cagg_monthly.sql @@ -70,9 +70,11 @@ SELECT bucket_width FROM _timescaledb_catalog.continuous_agg WHERE mat_hypertable_id = :cagg_id; -SELECT bucket_func, bucket_width, bucket_origin, bucket_timezone, bucket_fixed_width +\pset null +SELECT * FROM _timescaledb_catalog.continuous_aggs_bucket_function WHERE mat_hypertable_id = :cagg_id; +\pset null "" -- Check that the saved invalidation threshold is -infinity SELECT _timescaledb_functions.to_timestamp(watermark)