Update generate_uri member functions to use the StorageFormat #4530
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
|
This pull request has been linked to Shortcut Story #37128: Migrate |
KiterLuc
reviewed
Nov 22, 2023
| const std::pair<uint64_t, uint64_t>& timestamp_range) { | ||
| std::string uuid; | ||
| RETURN_NOT_OK(uuid::generate_uuid(&uuid, false)); | ||
| void ArraySchema::generate_uri( |
There was a problem hiding this comment.
Note that we could get rid of this function altogether. It is only called in a few places where a new constructor would remove the need for this. But not necessary to do in this PR.
There was a problem hiding this comment.
I noticed the same thing but the clear() functions in ArraySchema and ArraySchemaEvolution introduce a slight complication to this change, particularly with serialization. It seems it would be better to make both classes C.41 compliant, and remove generate_uri as a part of that work.
KiterLuc
approved these changes
Nov 23, 2023
teo-tsirpanis
added a commit
that referenced
this pull request
Feb 19, 2024
Since #4530, the group details folders written have a version field even in V1 groups.
teo-tsirpanis
added a commit
that referenced
this pull request
Feb 19, 2024
Since #4530, the group details folders written have a version field even in V1 groups.
teo-tsirpanis
added a commit
that referenced
this pull request
Feb 19, 2024
Since #4530, the group details folders written have a version field even in V1 groups.
KiterLuc
pushed a commit
that referenced
this pull request
Feb 19, 2024
…4744) [SC-41275](https://app.shortcut.com/tiledb-inc/story/41275/fix-v1-groups-written-with-latest-version) When version 2 of the group details format was introduced in #3928 (2.16 and also backported to 2.15), the names of group details V2 files were of the format `__t1_t2_id_version`, and V1 kept using `__t1_t2_id` for compatibility. In #4530 (2.19), V1 files started being written with the new format as well. This confused the group reader, which considered group details files with names of the form `__t1_t2_id_1` as V2, merely because they had a version field in their name. This PR fixes the group reader, making it always consider the version if it is present in the name of details file. The change has been validated by writing to a V1 group and then reading from it. The test group was created locally by Core version 2.14 and was added in `test/inputs/groups` (its size is tiny so it can safely be stored in the repository). --- TYPE: BUG DESC: Support reading V1 group details with explicit version in the name.
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.
Member functions should not generate uuids to create their own timestamped fragment and uri names. Instead, they need only to use the StorageFormat's APIs.
TYPE: IMPROVEMENT
DESC: Use StorageFormat APIs to generate timestamped uris