Skip to content

Commit

Permalink
Fix CAgg on CAgg variable bucket size validation
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@fabriziomello
fabriziomello committed Jan 11, 2023
1 parent 396bc6d commit acffcc6
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 51 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ accidentally triggering the load of a previous DB version.**

**Bugfixes**
* #4926 Fix corruption when inserting into compressed chunks
* #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)

Expand Down
65 changes: 49 additions & 16 deletions tsl/src/continuous_aggs/create.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1099,24 +1099,51 @@ 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)
/* calculate the width */
switch (bucket_info.bucket_width_type)
{
/* calculate the width */
return ts_interval_value_to_internal(IntervalPGetDatum(bucket_info.interval),
bucket_info.bucket_width_type);
case INT8OID:
case INT4OID:
case INT2OID:
return 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 */
return DatumGetInt64(DirectFunctionCall1(dtoi8, epoch));
break;
}
default:
Assert(false);
}

return bucket_info.bucket_width;
pg_unreachable();

return -1;
}

static inline Datum
get_bucket_width_datum(CAggTimebucketInfo bucket_info)
{
if (bucket_info.bucket_width == BUCKET_WIDTH_VARIABLE)
return IntervalPGetDatum(bucket_info.interval);
switch (bucket_info.bucket_width_type)
{
case INT8OID:
case INT4OID:
case INT2OID:
return ts_internal_to_interval_value(bucket_info.bucket_width,
bucket_info.bucket_width_type);
case INTERVALOID:
return IntervalPGetDatum(bucket_info.interval);
default:
Assert(false);
}

pg_unreachable();

return ts_internal_to_interval_value(get_bucket_width(bucket_info),
bucket_info.bucket_width_type);
return (Datum) -1;
}

static CAggTimebucketInfo
Expand Down Expand Up @@ -1320,8 +1347,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;
bool is_greater_or_equal_than_parent, is_multiple_of_parent;
int64 bucket_width = 0, bucket_width_parent = 0;
bool is_greater_or_equal_than_parent = true, is_multiple_of_parent = true;

Assert(prev_query->groupClause);
caggtimebucket_validate(&bucket_info_parent,
Expand Down Expand Up @@ -1354,7 +1381,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)
Expand All @@ -1373,14 +1406,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"),
Expand Down
32 changes: 16 additions & 16 deletions tsl/test/expected/cagg_on_cagg.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [5] should be multiple of the time bucket width of "public.conditions_summary_1" [2].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -860,7 +860,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -914,8 +914,8 @@ FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [2] should be multiple of the time bucket width of "public.conditions_summary_1" [4].
\set ON_ERROR_STOP 0
DETAIL: Time bucket width of "public.conditions_summary_2" [2] should be greater or equal than the time bucket width of "public.conditions_summary_1" [4].
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1718,7 +1718,7 @@ WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with fixed-width bucket on top of one using variable-width bucket
DETAIL: Continuous aggregate with a fixed time bucket width (e.g. 61 days) cannot be created on top of one using variable time bucket width (e.g. 1 month).
The variance can lead to the fixed width one not being a multiple of the variable width one.
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1774,7 +1774,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 3 hours] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1828,7 +1828,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1882,8 +1882,8 @@ FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be greater or equal than the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2684,7 +2684,7 @@ WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with fixed-width bucket on top of one using variable-width bucket
DETAIL: Continuous aggregate with a fixed time bucket width (e.g. 61 days) cannot be created on top of one using variable time bucket width (e.g. 1 month).
The variance can lead to the fixed width one not being a multiple of the variable width one.
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2740,7 +2740,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 3 hours] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2794,7 +2794,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2848,8 +2848,8 @@ FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be greater or equal than the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2908,7 +2908,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2961,7 +2961,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 16 mins] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 5 mins].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down
32 changes: 16 additions & 16 deletions tsl/test/expected/cagg_on_cagg_dist_ht.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [5] should be multiple of the time bucket width of "public.conditions_summary_1" [2].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -894,7 +894,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -948,8 +948,8 @@ FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [2] should be multiple of the time bucket width of "public.conditions_summary_1" [4].
\set ON_ERROR_STOP 0
DETAIL: Time bucket width of "public.conditions_summary_2" [2] should be greater or equal than the time bucket width of "public.conditions_summary_1" [4].
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1755,7 +1755,7 @@ WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with fixed-width bucket on top of one using variable-width bucket
DETAIL: Continuous aggregate with a fixed time bucket width (e.g. 61 days) cannot be created on top of one using variable time bucket width (e.g. 1 month).
The variance can lead to the fixed width one not being a multiple of the variable width one.
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1811,7 +1811,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 3 hours] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1865,7 +1865,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -1919,8 +1919,8 @@ FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be greater or equal than the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2721,7 +2721,7 @@ WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with fixed-width bucket on top of one using variable-width bucket
DETAIL: Continuous aggregate with a fixed time bucket width (e.g. 61 days) cannot be created on top of one using variable time bucket width (e.g. 1 month).
The variance can lead to the fixed width one not being a multiple of the variable width one.
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2777,7 +2777,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 3 hours] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2831,7 +2831,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2885,8 +2885,8 @@ FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 0
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 1 hour] should be greater or equal than the time bucket width of "public.conditions_summary_1" [@ 2 hours].
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2945,7 +2945,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down Expand Up @@ -2998,7 +2998,7 @@ GROUP BY 1
WITH NO DATA;
psql:include/cagg_on_cagg_validations.sql:41: ERROR: cannot create continuous aggregate with incompatible bucket width
DETAIL: Time bucket width of "public.conditions_summary_2" [@ 16 mins] should be multiple of the time bucket width of "public.conditions_summary_1" [@ 5 mins].
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse
--
-- Cleanup
Expand Down
2 changes: 1 addition & 1 deletion tsl/test/sql/include/cagg_on_cagg_validations.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ SELECT
FROM :CAGG_NAME_1ST_LEVEL
GROUP BY 1
WITH NO DATA;
\set ON_ERROR_STOP 0
\set ON_ERROR_STOP 1
\set VERBOSITY terse

--
Expand Down

0 comments on commit acffcc6

Please sign in to comment.