Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support CAGG names in hypertable_(detailed_)size #5159

Merged
merged 1 commit into from Feb 24, 2023

Conversation

noctarius
Copy link
Contributor

@noctarius noctarius commented Jan 9, 2023

This small patch adds support for continuous aggregates to the
hypertable_detailed_size (and with that hypertable_size).
It adds an additional check to see if a continuous aggregate exists
if a hypertable with the given regclass name isn't found.

Disable-check: force-changelog-changed

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

@shhnwz, @mahipv: please review this pull request.

Powered by pull-review

@noctarius noctarius force-pushed the noctarius-patch-1 branch 2 times, most recently from dd6be31 to bbf470b Compare January 9, 2023 16:46
Copy link
Contributor

@fabriziomello fabriziomello left a comment

Choose a reason for hiding this comment

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

Need to add regression tests

Copy link
Contributor

@mkindahl mkindahl left a comment

Choose a reason for hiding this comment

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

Title line is too long (we limit ourselves at 50 chars) and please have a look at the formatting of the commit message.

Going to look at it.

Copy link
Contributor

@mkindahl mkindahl left a comment

Choose a reason for hiding this comment

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

Looks good, but as @fabriziomello points out, you should have a test added as well to make sure that it works.

@noctarius noctarius changed the title Support for continuous aggregates in hypertable_(detailed_)size hypertable_detailed_size for continuous aggregates Jan 10, 2023
@noctarius noctarius force-pushed the noctarius-patch-1 branch 2 times, most recently from 533043e to d9c67fb Compare January 10, 2023 09:24
@noctarius noctarius changed the title hypertable_detailed_size for continuous aggregates Support for hypertable_detailed_size support in CAGG Jan 10, 2023
@codecov
Copy link

codecov bot commented Jan 10, 2023

Codecov Report

Merging #5159 (ac45569) into main (c8c50da) will decrease coverage by 0.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5159      +/-   ##
==========================================
- Coverage   91.08%   90.90%   -0.18%     
==========================================
  Files         225      225              
  Lines       46093    52296    +6203     
==========================================
+ Hits        41983    47539    +5556     
- Misses       4110     4757     +647     
Impacted Files Coverage Δ
tsl/test/src/test_chunk_stats.c 92.59% <0.00%> (-7.41%) ⬇️
src/uuid.c 78.57% <0.00%> (-6.05%) ⬇️
tsl/src/remote/healthcheck.c 63.76% <0.00%> (-6.05%) ⬇️
src/planner/add_hashagg.c 48.95% <0.00%> (-5.33%) ⬇️
src/histogram.c 83.83% <0.00%> (-4.80%) ⬇️
src/extension_utils.c 90.74% <0.00%> (-4.72%) ⬇️
tsl/src/fdw/fdw_utils.c 80.76% <0.00%> (-4.65%) ⬇️
src/time_utils.c 93.72% <0.00%> (-3.78%) ⬇️
tsl/src/remote/copy_fetcher.c 87.00% <0.00%> (-3.65%) ⬇️
src/utils.h 80.00% <0.00%> (-3.34%) ⬇️
... and 192 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@noctarius noctarius changed the title Support for hypertable_detailed_size support in CAGG Support CAGG names in hypertable_(detailed_)size Jan 10, 2023
@fabriziomello
Copy link
Contributor

@noctarius will you continue working on this PR?

@noctarius
Copy link
Contributor Author

Oh right, thanks for the reminder. Was on the road for a good chunk of the last two weeks. Let me look into it tomorrow and add the additional test.

@noctarius noctarius force-pushed the noctarius-patch-1 branch 2 times, most recently from 3b8b0b0 to 947a0ab Compare January 25, 2023 13:21
@noctarius
Copy link
Contributor Author

Can this be merged or is anything missing here? :)

@fabriziomello
Copy link
Contributor

Can this be merged or is anything missing here? :)

Ooops... I was on vacations... doing it right now!!

@noctarius
Copy link
Contributor Author

No worries, just saw it didn't make it into 2.10.0 😋

@fabriziomello
Copy link
Contributor

No worries, just saw it didn't make it into 2.10.0 yum

Sorry about that... in any case we need two last things:

  1. remove the signed off in the commit (you can sign with your gpg key but we don't put the "signed of" message in our commits), can you please amend your commit message and do a push --force
  2. we need to update the API docs: https://docs.timescale.com/api/latest/hypertable/hypertable_detailed_size/ (you can sync with docs team about it or send a PR to https://github.com/timescale/docs by yourself

@noctarius
Copy link
Contributor Author

Gone :) Good catch with the signed off. Many projects require it so I eventually enabled it by default :P

I'll send a PR for the docs myself tomorrow with a link to this PR.

This small patch adds support for continuous aggregates to the
`hypertable_detailed_size` (and with that `hypertable_size`).
It adds an additional check to see if a continuous aggregate exists
if a hypertable with the given regclass name isn't found.
@fabriziomello fabriziomello merged commit 0118e6b into timescale:main Feb 24, 2023
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Mar 6, 2023
This release contains bug fixes since the 2.10.0 release.
We recommend that you upgrade at the next available opportunity.

**Bugfixes**
* timescale#5159 Support Continuous Aggregates names in hypertable_(detailed_)size
* timescale#5226 Fix concurrent locking with chunk_data_node table
* timescale#5317 Fix some incorrect memory handling
* timescale#5336 Use NameData and namestrcpy for names
* timescale#5343 Set PortalContext when starting job
* timescale#5360 Fix uninitialized bucket_info variable
* timescale#5362 Make copy fetcher more async
* timescale#5364 Fix num_chunks inconsistency in hypertables view
* timescale#5367 Fix column name handling in old-style continuous aggregates
* timescale#5378 Fix multinode DML HA performance regression
* timescale#5384 Fix Hierarchical Continuous Aggregates chunk_interval_size

**Thanks**
* @justinozavala for reporting an issue with PL/Python procedures in the background worker
* @Medvecrab for discovering an issue with copying NameData when forming heap tuples.
* @pushpeepkmonroe for discovering an issue in upgrading old-style
  continuous aggregates with renamed columns
* @pushpeepkmonroe for discovering an issue in upgrading old-style continuous aggregates with renamed columns
svenklemm added a commit that referenced this pull request Mar 7, 2023
This release contains bug fixes since the 2.10.0 release.
We recommend that you upgrade at the next available opportunity.

**Bugfixes**
* #5159 Support Continuous Aggregates names in hypertable_(detailed_)size
* #5226 Fix concurrent locking with chunk_data_node table
* #5317 Fix some incorrect memory handling
* #5336 Use NameData and namestrcpy for names
* #5343 Set PortalContext when starting job
* #5360 Fix uninitialized bucket_info variable
* #5362 Make copy fetcher more async
* #5364 Fix num_chunks inconsistency in hypertables view
* #5367 Fix column name handling in old-style continuous aggregates
* #5378 Fix multinode DML HA performance regression
* #5384 Fix Hierarchical Continuous Aggregates chunk_interval_size

**Thanks**
* @justinozavala for reporting an issue with PL/Python procedures in the background worker
* @Medvecrab for discovering an issue with copying NameData when forming heap tuples.
* @pushpeepkmonroe for discovering an issue in upgrading old-style
  continuous aggregates with renamed columns
* @pushpeepkmonroe for discovering an issue in upgrading old-style continuous aggregates with renamed columns
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Mar 7, 2023
This release contains bug fixes since the 2.10.0 release.
We recommend that you upgrade at the next available opportunity.

**Bugfixes**
* timescale#5159 Support Continuous Aggregates names in hypertable_(detailed_)size
* timescale#5226 Fix concurrent locking with chunk_data_node table
* timescale#5317 Fix some incorrect memory handling
* timescale#5336 Use NameData and namestrcpy for names
* timescale#5343 Set PortalContext when starting job
* timescale#5360 Fix uninitialized bucket_info variable
* timescale#5362 Make copy fetcher more async
* timescale#5364 Fix num_chunks inconsistency in hypertables view
* timescale#5367 Fix column name handling in old-style continuous aggregates
* timescale#5378 Fix multinode DML HA performance regression
* timescale#5384 Fix Hierarchical Continuous Aggregates chunk_interval_size

**Thanks**
* @justinozavala for reporting an issue with PL/Python procedures in the background worker
* @Medvecrab for discovering an issue with copying NameData when forming heap tuples.
* @pushpeepkmonroe for discovering an issue in upgrading old-style
  continuous aggregates with renamed columns
* @pushpeepkmonroe for discovering an issue in upgrading old-style continuous aggregates with renamed columns
svenklemm added a commit that referenced this pull request Mar 7, 2023
This release contains bug fixes since the 2.10.0 release.
We recommend that you upgrade at the next available opportunity.

**Bugfixes**
* #5159 Support Continuous Aggregates names in hypertable_(detailed_)size
* #5226 Fix concurrent locking with chunk_data_node table
* #5317 Fix some incorrect memory handling
* #5336 Use NameData and namestrcpy for names
* #5343 Set PortalContext when starting job
* #5360 Fix uninitialized bucket_info variable
* #5362 Make copy fetcher more async
* #5364 Fix num_chunks inconsistency in hypertables view
* #5367 Fix column name handling in old-style continuous aggregates
* #5378 Fix multinode DML HA performance regression
* #5384 Fix Hierarchical Continuous Aggregates chunk_interval_size

**Thanks**
* @justinozavala for reporting an issue with PL/Python procedures in the background worker
* @Medvecrab for discovering an issue with copying NameData when forming heap tuples.
* @pushpeepkmonroe for discovering an issue in upgrading old-style
  continuous aggregates with renamed columns
* @pushpeepkmonroe for discovering an issue in upgrading old-style continuous aggregates with renamed columns
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants