Skip to content

Refactor cagg bucket alignment code - #9729

Merged
pnthao merged 1 commit into
timescale:mainfrom
pnthao:refactor_cagg_bucket_alignment_code
May 8, 2026
Merged

Refactor cagg bucket alignment code#9729
pnthao merged 1 commit into
timescale:mainfrom
pnthao:refactor_cagg_bucket_alignment_code

Conversation

@pnthao

@pnthao pnthao commented May 6, 2026

Copy link
Copy Markdown
Member

Refactor the bucket alignment code

Refactor the bucket alignment code to reduce duplication
- Extract the logics that computes the bucket start and next bucket
start into corresponding functions to use at a few places
- Consolidate functions that compute bucket start for both fix and
variable bucket.
- Clean up the inscribe/circumscribe functions

Disable-check: force-changelog-file

@pnthao
pnthao requested review from a team, gayyappan, kpan2034 and melihmutlu May 6, 2026 13:13
@codecov

codecov Bot commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@gayyappan gayyappan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup of the code. A lot more readable now.
some minor comments.

Comment thread tsl/src/continuous_aggs/refresh.c
Comment thread tsl/src/continuous_aggs/refresh.c Outdated
@@ -155,12 +155,24 @@ get_largest_bucketed_window(Oid timetype, int64 bucket_width)
*/
static InternalTimeRange
compute_inscribed_bucketed_refresh_window(const ContinuousAgg *cagg,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the change in the parameters here:
either use cagg->bucket_function.
Or remove cagg from the parameters?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I added bucket_function to replace bucket_width, but didn't realize cagg is there for the same purpose. I will remove cagg.

Comment thread tsl/src/continuous_aggs/refresh.c Outdated
@pnthao

pnthao commented May 8, 2026

Copy link
Copy Markdown
Member Author

Nice cleanup of the code. A lot more readable now. some minor comments.

Thank you!

@kpan2034 kpan2034 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Refactor the bucket alignment code to reduce duplication
- Extract the logics that computes the bucket start and next bucket
start into corresponding functions to use at a few places
- Consolidate functions that compute bucket start for both fix and
variable bucket.
- Clean up the inscribe/circumscribe functions
@pnthao
pnthao force-pushed the refactor_cagg_bucket_alignment_code branch from 6f0da79 to da3e892 Compare May 8, 2026 15:19
@pnthao
pnthao merged commit 85f312f into timescale:main May 8, 2026
65 of 67 checks passed
@pnthao
pnthao deleted the refactor_cagg_bucket_alignment_code branch May 8, 2026 16:24
@timescale-automation timescale-automation added the released-2.28.0 Released in 2.28.0 label Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released-2.28.0 Released in 2.28.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants