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

[Content Bug] Inconsistent use of api for compression #3248

Closed
billy-the-fish opened this issue Jun 13, 2024 · 2 comments · Fixed by #3250
Closed

[Content Bug] Inconsistent use of api for compression #3248

billy-the-fish opened this issue Jun 13, 2024 · 2 comments · Fixed by #3250
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@billy-the-fish
Copy link
Contributor

Hey, I found an inconsistency in the docs, but potentially also in the actual SQL API.

When enabling compression on a hypertable, we are asked to set, among other things, timescaledb.compress without explicitly giving it the value true. See https://docs.timescale.com/api/latest/compression/alter_table_compression/#alter-table-compression

When enabling compression on a continuous aggregate, we are asked to set timescaledb.compress with explicitly giving it the value true. See https://docs.timescale.com/api/latest/continuous-aggregates/alter_materialized_view/#sample-usage

This is indeed confusing me, and I am wondering what is the correct and best way to do it. I hope the documentation and, if necessary, the underlying API, can be changed to make both cases consistent

@billy-the-fish billy-the-fish added bug Something isn't working documentation Improvements or additions to documentation labels Jun 13, 2024
@billy-the-fish billy-the-fish self-assigned this Jun 13, 2024
@billy-the-fish billy-the-fish changed the title [Content Bug] [Content Bug] Inconsistent use of api for compression Jun 13, 2024
@antekresic
Copy link
Contributor

Both ways basically do the same things so cleaning up the docs by removing the = true part of the continuous aggs part seems the way to go.

@jonatas
Copy link
Contributor

jonatas commented Jun 13, 2024

@antekresic I'm curious if it is the same case for materialized_only?

I like when we use the = true or = false as it seems more like a configuration style.

So, it means that:

ALTER MATERIALIZED VIEW contagg_view SET (timescaledb.compress = true);

is the same as:

ALTER MATERIALIZED VIEW contagg_view SET (timescaledb.compress);

As a rubyist my brain recognizes timescaledb.compress as a method call but in sql it looks reading a property of a relation and not configuring it. But ok, let's say the hidden assignment to true is being done, but maybe users will get confused that it's a method, and it will also start thinking it will be compressing immediately. In fact they'll need to setup a compression policy or compress manually.

When we toggle a configuration, my brain thinks something else should be done to apply this configuration, when we call the method, we think it can be doing both things at once. Not sure if I'm contributing or not but just thinking loud.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants