-
Notifications
You must be signed in to change notification settings - Fork 23
Remove id column #94
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
Remove id column #94
Conversation
Thanks @nwerter ! I will try to have an in-depth look at this within the next couple of weeks. However, I can already now say that I would prefer to keep this PR to only address the removal of the ID column. If you also want to propose to change the default value of the chunk time interval, please open a separate PR for that and argue for why. |
@freol35241 No worries, I'll try to update the PR this weekend accordingly. The main reason for changing the default value of the chunk time interval is that it allows for compression of the database after a shorter time period (e.g. two weeks) and thereby more efficient storage. Timescale only allows compression of chunks that are not longer updated. |
@freol35241 I cleaned-up the PR such that it now only removes the ID column. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, thank you!
One thing to fix and one thing that I would appreciate your input on. See comments below.
Thanks for the explanation. Still, changing the default value in ltss means that all existing installations (opting for an update) relying on the default value would get silently pushed to another chunk time interval. |
Understood, if you would be interested, I was thinking of including a separate option to enable compression after a certain time period. The default values I would then propose (if compression is enabled) is compression after 14 days with a default chunk interval of 7 days. I'm running this on my own installation now and it significantly reduces the database size. Do you have any thoughts on this? |
This looks good, thank you again @nwerter! Regarding the compression feature, I would be happy to receive a PR which includes the possibility to configure the compression policy. For default values however, I would like to stick with the current one for the chunk time interval (for the above explained reason) and have compression disabled by default (to not force new features on to existing users). But I think it would be very good to document your specific suggestion in the readme to provide som guidance to other users. |
One comment in case others stumble across this as well: in case you have already enabled compression manually, the migration to 2.1.0 will fail when trying to add the new primary key. It seems adding primary keys is not allowed for compressed hypertables. |
#25
Includes the migration from the old primary keys to the new primary keys.
Removes the id column, since it is no longer tied to the primary key.