From acffcc61dba62896c35c5cdb14932fe9d9a0f32e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabr=C3=ADzio=20de=20Royes=20Mello?= Date: Tue, 10 Jan 2023 15:19:18 -0300 Subject: [PATCH] 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 | 4 +- tsl/src/continuous_aggs/create.c | 65 ++++++++++++++----- tsl/test/expected/cagg_on_cagg.out | 32 ++++----- tsl/test/expected/cagg_on_cagg_dist_ht.out | 32 ++++----- .../sql/include/cagg_on_cagg_validations.sql | 2 +- 5 files changed, 84 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 055df8cad67..56cf6b4f775 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/tsl/src/continuous_aggs/create.c b/tsl/src/continuous_aggs/create.c index b03eae35e54..69abf9aeb49 100644 --- a/tsl/src/continuous_aggs/create.c +++ b/tsl/src/continuous_aggs/create.c @@ -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 @@ -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, @@ -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) @@ -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"), diff --git a/tsl/test/expected/cagg_on_cagg.out b/tsl/test/expected/cagg_on_cagg.out index 338b5d3a674..7bb562f6637 100644 --- a/tsl/test/expected/cagg_on_cagg.out +++ b/tsl/test/expected/cagg_on_cagg.out @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/tsl/test/expected/cagg_on_cagg_dist_ht.out b/tsl/test/expected/cagg_on_cagg_dist_ht.out index 6851344b8cf..6578467946a 100644 --- a/tsl/test/expected/cagg_on_cagg_dist_ht.out +++ b/tsl/test/expected/cagg_on_cagg_dist_ht.out @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/tsl/test/sql/include/cagg_on_cagg_validations.sql b/tsl/test/sql/include/cagg_on_cagg_validations.sql index 519776bfb26..7b0df5a1a8f 100644 --- a/tsl/test/sql/include/cagg_on_cagg_validations.sql +++ b/tsl/test/sql/include/cagg_on_cagg_validations.sql @@ -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 --