Move compute_new_fragment_name into StorageFormat #4520
Conversation
|
This pull request has been linked to Shortcut Story #35770: Move Group/ArrayDirectory::compute_new_fragment_name to storage format.. |
| std::string compute_new_fragment_name( | ||
| const URI& first, const URI& last, format_version_t format_version); | ||
|
|
||
| /** |
There was a problem hiding this comment.
I did some more research here... This is used for things other than fragment names... What I would suggest is to move this back to generate_uri.cc. The function would also be renamed to generate_timestamped_name.
The function currently called generate_uri (in generate_uri.cc) would also be renamed generate_timestamped_name. It actually does mostly the same thing as the current compute_new_fragment_name, but with a different input set.
We might be able to move part of Status ArraySchema::generate_uri as part of this PR.
We might also be able to move part of Metadata::generate_uri as part of this PR.
There was a problem hiding this comment.
| // Create fragment at timestamp 1-2 | ||
| auto frag = tiledb::sm::URI{ | ||
| "file:///array_name/__fragments/" | ||
| "__44318efd44f546b18db13edc8d10805b_1{_2}"}; |
There was a problem hiding this comment.
Why do we have braces here?
There was a problem hiding this comment.
Now that you have looked at the format for all name versions, it might be time to document them... Can you file a shortcut story that details all the timestamped name versions. The story should be about adding this history to the format spec (<timestamped_name>).
There was a problem hiding this comment.
There was a problem hiding this comment.
Comment for future work.
ArraySchema::generate_uriis using a generated URI, but isn't a fragment. It needs similar treatment, though not in this PR.- It seems as though we ought to move
uuid.*out ofmiscand intostorage_format, which is the only place we should be using that code. This item would make a good, quick immediate followup to the present one.
Move
compute_new_fragment_namefromtiledb/sminto theStorageFormat.A new file is added to maintain storage format of
fragment_name, and containscompute_new_fragment_nameandgenerate_fragment_name. Tests are added for these APIs, as well asget_timestamp_range.TYPE: FORMAT
DESC: Migrate fragment_name parsing into StorageFormat