Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Save compression metadata settings on access node #2662
Save compression metadata settings on access node #2662
Changes from all commits
4cf560d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
What is the difference between
TS_HYPERTABLE_HAS_COMPRESSION
andTS_HYPERTABLE_HAS_COMPRESSION_ENABLED
?This is somewhat confusing and it is difficult to tell these two macros apart.
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.
TS_HYPERTABLE_HAS_COMPRESSION is used in places where we need to know that the internal table exists for the user table. The action is done on the internal table.
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.
Sorry, I should have been more clear/specific. The issue here is that the macros are hard to tell apart. I understand that they are used differently, although it was initially hard for me to realize the difference. This leads me to believe that this will be the case for others too, so just explaining in the PR is not enough.
If I look at the implementation of
ts_hypertable_has_compression
it is asserted thatht->fd.compression_state == HypertableCompressionEnabled
is true when the function returns true. This is the same condition used inTS_HYPERTABLE_HAS_COMPRESSION_ENABLED
. So it is really unclear what the difference is.I think what we actually want to do here is distinguish between distributed and non-distributed compressed hypertables:
This would make the naming more clear and also, looking at the definition, that this is a stricter condition than just having compression enabled.
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.
The check is for the existence of a "compressed" hypertable. I think a more appropriate rename is TS_HYPERTABLE_HAS_COMPRESSION_TABLE. If it is distributed, compressed_hypertable_id would be null.