Merge 'master' from upstream #29
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… of the low level numbers. (cmu-db#361)
jrolli
added
ReadyForReview
yashNaN
reviewed
Apr 25, 2019
| * | ... | | ||
| * ----------------------------------------------------------------------- | ||
| * ------------------------------------------------------------------------------------- | ||
| * | size | max_tuples | num_tuples | num_cols | attr_end[4] | col_id1 | col_id2 | ... | |
There was a problem hiding this comment.
is attr_end supposed to be the array of attribute size? Why isn't this documented as attr_sizes?
There was a problem hiding this comment.
Nvm I get it, it's storing where the boundaries are.
yashNaN
reviewed
Apr 25, 2019
| @@ -5,6 +5,17 @@ | |||
| #include <utility> | |||
| #include <vector> | |||
| namespace terrier::storage { | |||
| const uint32_t ProjectedColumns::AttrSizeForColumn(uint16_t col_id) { | |||
There was a problem hiding this comment.
attr_ends_ seems to be only for getting attr sizes, so why not directly just store attr size? Or would attribute size potentially be too large to fit them all in the header?
There was a problem hiding this comment.
Ignore this, John you're a genius. It make sense
yashNaN
reviewed
Apr 25, 2019
| // columns size by tracking the locations of the attribute size boundaries. | ||
| // Therefore, we only need to track 4 locations because the exterior bounds | ||
| // are implicit. | ||
| #define NUM_ATTR_BOUNDARIES 4 |
There was a problem hiding this comment.
Everything makes sense now regarding all the questions I had about attr_ends_, you're a genius.
yashNaN
approved these changes
Apr 25, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR merges in the latest changes in the master branch from upstream and resolves several API conflicts in the tests that utilize SqlTable (versioning is not upstream).