Storage manager: exposing methods to load/store generic tile. #3325
Conversation
This changes every paths in the storage manager to load/store generic tile to go through a common API that can also be used externally from the storage manager. --- TYPE: IMPROVEMENT DESC: Storage manager: exposing methods to load/store generic tile.
|
This pull request has been linked to Shortcut Story #18970: Deletes: add storage manager APIs to write and load deletes as a generic tile.. |
tiledb/sm/tile/generic_tile_io.h
Outdated
| */ | ||
| Status read_generic( | ||
| Tile** tile, | ||
| tuple<Status, optional<tdb_unique_ptr<Tile>>> read_generic( |
There was a problem hiding this comment.
Question: was there a reason to prefer optional<unique_ptr<Tile>> here over optional<Tile>? load_data_from_generic_tile returns optional<Buffer>, which seems like it should have the same cost as optional<Tile>. (IIUC Tile doesn't store anything inline, it either wraps a unique_ptr<void*>+deleter or a std::vector<char> via FilteredBuffer)
There was a problem hiding this comment.
Yes one caller has two branches calling this, and the resulting tile then gets used in common code. Having a unique pointer allows to not call the default constructor for Tile, which we'll want to remove at some point for C41 compliance.
There was a problem hiding this comment.
@ihnorton Actually by moving the common code in read_generic, and some other modifications, I got rid of the optional unique pointer.
This changes every paths in the storage manager to load/store generic
tile to go through a common API that can also be used externally from
the storage manager.
TYPE: IMPROVEMENT
DESC: Storage manager: exposing methods to load/store generic tile.