Skip to content

Commit

Permalink
Support sub-second intervals in hierarchical caggs
Browse files Browse the repository at this point in the history
Previously we used date_part("epoch", interval) and integer division
internally to determine whether the top cagg's interval is a
multiple of its parent's.
This led to precision loss and wrong results
in the case of intervals with sub-second components.

Fixed by using the `ts_interval_value_to_internal` function to convert
intervals to appropriate integer representation for division.

Fixes #5277
  • Loading branch information
konskov committed Mar 6, 2023
1 parent 474b09b commit 0aec692
Show file tree
Hide file tree
Showing 6 changed files with 895 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -13,6 +13,7 @@ accidentally triggering the load of a previous DB version.**
* #5317 Fix some incorrect memory handling
* #5367 Rename columns in old-style continuous aggregates
* #5384 Fix Hierarchical Continuous Aggregates chunk_interval_size
* #5304 Support sub-second intervals in hierarchical caggs

**Thanks**
* @Medvecrab for discovering an issue with copying NameData when forming heap tuples.
Expand Down
14 changes: 6 additions & 8 deletions tsl/src/continuous_aggs/create.c
Expand Up @@ -1128,7 +1128,7 @@ cagg_query_supported(const Query *query, StringInfo hint, StringInfo detail, con
static inline int64
get_bucket_width(CAggTimebucketInfo bucket_info)
{
int64 width = 0;
float8 width = 0;

/* Calculate the width. */
switch (bucket_info.bucket_width_type)
Expand All @@ -1151,11 +1151,9 @@ get_bucket_width(CAggTimebucketInfo bucket_info)
bucket_info.interval->day = bucket_info.interval->month * DAYS_PER_MONTH;
bucket_info.interval->month = 0;
}
Datum epoch = DirectFunctionCall2(interval_part,
PointerGetDatum(cstring_to_text("epoch")),
IntervalPGetDatum(bucket_info.interval));
/* Cast float8 to int8. */
width = DatumGetInt64(DirectFunctionCall1(dtoi8, epoch));

/* Convert Interval to int64 */
width = ts_interval_value_to_internal(IntervalPGetDatum(bucket_info.interval), INTERVALOID);
break;
}
default:
Expand Down Expand Up @@ -1546,9 +1544,9 @@ cagg_validate_query(const Query *query, const bool finalized, const char *cagg_s
if (bucket_width_parent != 0)
{
if (bucket_width_parent > bucket_width && bucket_width != 0)
is_multiple_of_parent = ((bucket_width_parent % bucket_width) == 0);
is_multiple_of_parent = (fmod(bucket_width_parent, bucket_width) == 0);
else
is_multiple_of_parent = ((bucket_width % bucket_width_parent) == 0);
is_multiple_of_parent = (fmod(bucket_width, bucket_width_parent) == 0);
}

/* Proceed with validation errors. */
Expand Down

0 comments on commit 0aec692

Please sign in to comment.