From 2c99613f8bf6193a8ca48ac19d60de0c13a22bdf Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Wed, 4 Oct 2023 09:38:12 +0300 Subject: [PATCH] Remove statuses from the Metadata code. (#4012) * Unstatus the `Metadata` class. * Unstatus metadata-related methods of the `Array` class. * Unstatus metadata-related methods of the `Group` class. * Change `Metadata::has_metadata` to return an `std::optional`. Applies suggestion from https://github.com/TileDB-Inc/TileDB/pull/3972#discussion_r1156121140. * Remove status. * Fix error message spacing. * Fix standalone test. --------- Co-authored-by: KiterLuc <67824247+KiterLuc@users.noreply.github.com> Co-authored-by: Luc Rancourt --- test/src/unit-capi-array.cc | 4 +- tiledb/api/c_api/group/group_api.cc | 23 ++--- tiledb/sm/array/array.cc | 114 +++++++++-------------- tiledb/sm/array/array.h | 18 ++-- tiledb/sm/c_api/tiledb.cc | 23 ++--- tiledb/sm/group/group.cc | 110 +++++++++------------- tiledb/sm/group/group.h | 21 ++--- tiledb/sm/metadata/metadata.cc | 41 ++++---- tiledb/sm/metadata/metadata.h | 25 ++--- tiledb/sm/metadata/test/unit_metadata.cc | 24 ++--- tiledb/sm/serialization/array.cc | 4 +- 11 files changed, 157 insertions(+), 250 deletions(-) diff --git a/test/src/unit-capi-array.cc b/test/src/unit-capi-array.cc index a088d2a7614..94cee2f7a3c 100644 --- a/test/src/unit-capi-array.cc +++ b/test/src/unit-capi-array.cc @@ -2349,12 +2349,12 @@ TEST_CASE_METHOD( const void* v_r; uint32_t v_num; auto new_metadata = new_array->array_->unsafe_metadata(); - Status st = new_metadata->get("aaa", &type, &v_num, &v_r); + new_metadata->get("aaa", &type, &v_num, &v_r); CHECK(static_cast(type) == TILEDB_INT32); CHECK(v_num == 1); CHECK(*((const int32_t*)v_r) == 5); - st = new_metadata->get("bb", &type, &v_num, &v_r); + new_metadata->get("bb", &type, &v_num, &v_r); CHECK(static_cast(type) == TILEDB_FLOAT32); CHECK(v_num == 2); CHECK(((const float*)v_r)[0] == 1.1f); diff --git a/tiledb/api/c_api/group/group_api.cc b/tiledb/api/c_api/group/group_api.cc index 313b9790041..483040b2d96 100644 --- a/tiledb/api/c_api/group/group_api.cc +++ b/tiledb/api/c_api/group/group_api.cc @@ -156,8 +156,8 @@ capi_return_t tiledb_group_put_metadata( ensure_group_is_valid(group); ensure_key_argument_is_valid(key); - throw_if_not_ok(group->group().put_metadata( - key, static_cast(value_type), value_num, value)); + group->group().put_metadata( + key, static_cast(value_type), value_num, value); return TILEDB_OK; } @@ -176,7 +176,7 @@ capi_return_t tiledb_group_delete_metadata( ensure_group_is_valid(group); ensure_key_argument_is_valid(key); - throw_if_not_ok(group->group().delete_metadata(key)); + group->group().delete_metadata(key); return TILEDB_OK; } @@ -194,7 +194,7 @@ capi_return_t tiledb_group_get_metadata( ensure_output_pointer_is_valid(value); tiledb::sm::Datatype type; - throw_if_not_ok(group->group().get_metadata(key, &type, value_num, value)); + group->group().get_metadata(key, &type, value_num, value); *value_type = static_cast(type); @@ -206,7 +206,7 @@ capi_return_t tiledb_group_get_metadata_num( ensure_group_is_valid(group); ensure_output_pointer_is_valid(num); - throw_if_not_ok(group->group().get_metadata_num(num)); + *num = group->group().get_metadata_num(); return TILEDB_OK; } @@ -227,8 +227,7 @@ capi_return_t tiledb_group_get_metadata_from_index( ensure_output_pointer_is_valid(value); tiledb::sm::Datatype type; - throw_if_not_ok(group->group().get_metadata( - index, key, key_len, &type, value_num, value)); + group->group().get_metadata(index, key, key_len, &type, value_num, value); *value_type = static_cast(type); @@ -245,13 +244,11 @@ capi_return_t tiledb_group_has_metadata_key( ensure_output_pointer_is_valid(value_type); ensure_output_pointer_is_valid(has_key); - tiledb::sm::Datatype type; - bool has_the_key; - throw_if_not_ok(group->group().has_metadata_key(key, &type, &has_the_key)); + std::optional type = group->group().metadata_type(key); - *has_key = has_the_key ? 1 : 0; - if (has_the_key) { - *value_type = static_cast(type); + *has_key = type.has_value(); + if (*has_key) { + *value_type = static_cast(type.value()); } return TILEDB_OK; diff --git a/tiledb/sm/array/array.cc b/tiledb/sm/array/array.cc index 5d1bacb8c17..65a7fb56eca 100644 --- a/tiledb/sm/array/array.cc +++ b/tiledb/sm/array/array.cc @@ -615,8 +615,7 @@ std::vector> Array::get_enumerations( auto rest_client = resources_.rest_client(); if (rest_client == nullptr) { throw ArrayException( - "Error loading enumerations; " - "Remote array with no REST client."); + "Error loading enumerations; Remote array with no REST client."); } std::vector names_to_load; @@ -913,103 +912,89 @@ void Array::set_config(Config config) { config_.inherit(config); } -Status Array::delete_metadata(const char* key) { +void Array::delete_metadata(const char* key) { // Check if array is open if (!is_open_) { - return LOG_STATUS( - Status_ArrayError("Cannot delete metadata. Array is not open")); + throw ArrayException("Cannot delete metadata. Array is not open"); } // Check mode if (query_type_ != QueryType::WRITE && query_type_ != QueryType::MODIFY_EXCLUSIVE) { - return LOG_STATUS( - Status_ArrayError("Cannot delete metadata. Array was " - "not opened in write or modify_exclusive mode")); + throw ArrayException( + "Cannot delete metadata. Array was not opened in write or " + "modify_exclusive mode"); } // Check if key is null if (key == nullptr) { - return LOG_STATUS( - Status_ArrayError("Cannot delete metadata. Key cannot be null")); + throw ArrayException("Cannot delete metadata. Key cannot be null"); } - RETURN_NOT_OK(metadata_.del(key)); - - return Status::Ok(); + metadata_.del(key); } -Status Array::put_metadata( +void Array::put_metadata( const char* key, Datatype value_type, uint32_t value_num, const void* value) { // Check if array is open if (!is_open_) { - return LOG_STATUS( - Status_ArrayError("Cannot put metadata; Array is not open")); + throw ArrayException("Cannot put metadata; Array is not open"); } // Check mode if (query_type_ != QueryType::WRITE && query_type_ != QueryType::MODIFY_EXCLUSIVE) { - return LOG_STATUS( - Status_ArrayError("Cannot put metadata; Array was " - "not opened in write or modify_exclusive mode")); + throw ArrayException( + "Cannot put metadata; Array was not opened in write or " + "modify_exclusive mode"); } // Check if key is null if (key == nullptr) { - return LOG_STATUS( - Status_ArrayError("Cannot put metadata; Key cannot be null")); + throw ArrayException("Cannot put metadata; Key cannot be null"); } // Check if value type is ANY if (value_type == Datatype::ANY) { - return LOG_STATUS( - Status_ArrayError("Cannot put metadata; Value type cannot be ANY")); + throw ArrayException("Cannot put metadata; Value type cannot be ANY"); } - RETURN_NOT_OK(metadata_.put(key, value_type, value_num, value)); - - return Status::Ok(); + metadata_.put(key, value_type, value_num, value); } -Status Array::get_metadata( +void Array::get_metadata( const char* key, Datatype* value_type, uint32_t* value_num, const void** value) { // Check if array is open if (!is_open_) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Array is not open")); + throw ArrayException("Cannot get metadata; Array is not open"); } // Check mode if (query_type_ != QueryType::READ) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Array was " - "not opened in read mode")); + throw ArrayException( + "Cannot get metadata; Array was not opened in read mode"); } // Check if key is null if (key == nullptr) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Key cannot be null")); + throw ArrayException("Cannot get metadata; Key cannot be null"); } // Load array metadata, if not loaded yet if (!metadata_loaded_) { - RETURN_NOT_OK(load_metadata()); + throw_if_not_ok(load_metadata()); } - RETURN_NOT_OK(metadata_.get(key, value_type, value_num, value)); - - return Status::Ok(); + metadata_.get(key, value_type, value_num, value); } -Status Array::get_metadata( +void Array::get_metadata( uint64_t index, const char** key, uint32_t* key_len, @@ -1018,81 +1003,66 @@ Status Array::get_metadata( const void** value) { // Check if array is open if (!is_open_) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Array is not open")); + throw ArrayException("Cannot get metadata; Array is not open"); } // Check mode if (query_type_ != QueryType::READ) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Array was " - "not opened in read mode")); + throw ArrayException( + "Cannot get metadata; Array was not opened in read mode"); } // Load array metadata, if not loaded yet if (!metadata_loaded_) { - RETURN_NOT_OK(load_metadata()); + throw_if_not_ok(load_metadata()); } - RETURN_NOT_OK( - metadata_.get(index, key, key_len, value_type, value_num, value)); - - return Status::Ok(); + metadata_.get(index, key, key_len, value_type, value_num, value); } -Status Array::get_metadata_num(uint64_t* num) { +uint64_t Array::metadata_num() { // Check if array is open if (!is_open_) { - return LOG_STATUS( - Status_ArrayError("Cannot get number of metadata; Array is not open")); + throw ArrayException("Cannot get number of metadata; Array is not open"); } // Check mode if (query_type_ != QueryType::READ) { - return LOG_STATUS( - Status_ArrayError("Cannot get number of metadata; Array was " - "not opened in read mode")); + throw ArrayException( + "Cannot get number of metadata; Array was not opened in read mode"); } // Load array metadata, if not loaded yet if (!metadata_loaded_) { - RETURN_NOT_OK(load_metadata()); + throw_if_not_ok(load_metadata()); } - *num = metadata_.num(); - - return Status::Ok(); + return metadata_.num(); } -Status Array::has_metadata_key( - const char* key, Datatype* value_type, bool* has_key) { +std::optional Array::metadata_type(const char* key) { // Check if array is open if (!is_open_) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Array is not open")); + throw ArrayException("Cannot get metadata; Array is not open"); } // Check mode if (query_type_ != QueryType::READ) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Array was " - "not opened in read mode")); + throw ArrayException( + "Cannot get metadata; Array was not opened in read mode"); } // Check if key is null if (key == nullptr) { - return LOG_STATUS( - Status_ArrayError("Cannot get metadata; Key cannot be null")); + throw ArrayException("Cannot get metadata; Key cannot be null"); } // Load array metadata, if not loaded yet if (!metadata_loaded_) { - RETURN_NOT_OK(load_metadata()); + throw_if_not_ok(load_metadata()); } - RETURN_NOT_OK(metadata_.has_key(key, value_type, has_key)); - - return Status::Ok(); + return metadata_.metadata_type(key); } Metadata* Array::unsafe_metadata() { diff --git a/tiledb/sm/array/array.h b/tiledb/sm/array/array.h index d859675cb63..7c03262d42c 100644 --- a/tiledb/sm/array/array.h +++ b/tiledb/sm/array/array.h @@ -414,9 +414,8 @@ class Array { * Deletes metadata from an array opened in WRITE mode. * * @param key The key of the metadata item to be deleted. - * @return Status */ - Status delete_metadata(const char* key); + void delete_metadata(const char* key); /** * Puts metadata into an array opened in WRITE mode. @@ -428,9 +427,8 @@ class Array { * same datatype. This argument indicates the number of items in the * value component of the metadata. * @param value The metadata value in binary form. - * @return Status */ - Status put_metadata( + void put_metadata( const char* key, Datatype value_type, uint32_t value_num, @@ -447,9 +445,8 @@ class Array { * same datatype. This argument indicates the number of items in the * value component of the metadata. * @param value The metadata value in binary form. - * @return Status */ - Status get_metadata( + void get_metadata( const char* key, Datatype* value_type, uint32_t* value_num, @@ -466,9 +463,8 @@ class Array { * same datatype. This argument indicates the number of items in the * value component of the metadata. * @param value The metadata value in binary form. - * @return Status */ - Status get_metadata( + void get_metadata( uint64_t index, const char** key, uint32_t* key_len, @@ -477,10 +473,10 @@ class Array { const void** value); /** Returns the number of array metadata items. */ - Status get_metadata_num(uint64_t* num); + uint64_t metadata_num(); - /** Sets has_key == 1 and corresponding value_type if the array has key. */ - Status has_metadata_key(const char* key, Datatype* value_type, bool* has_key); + /** Gets the type of the given metadata or nullopt if it does not exist. */ + std::optional metadata_type(const char* key); /** Retrieves the array metadata object. */ Status metadata(Metadata** metadata); diff --git a/tiledb/sm/c_api/tiledb.cc b/tiledb/sm/c_api/tiledb.cc index 95e4d4667c8..067a2d2856f 100644 --- a/tiledb/sm/c_api/tiledb.cc +++ b/tiledb/sm/c_api/tiledb.cc @@ -3023,8 +3023,8 @@ int32_t tiledb_array_put_metadata( return TILEDB_ERR; // Put metadata - throw_if_not_ok(array->array_->put_metadata( - key, static_cast(value_type), value_num, value)); + array->array_->put_metadata( + key, static_cast(value_type), value_num, value); return TILEDB_OK; } @@ -3035,7 +3035,7 @@ int32_t tiledb_array_delete_metadata( return TILEDB_ERR; // Put metadata - throw_if_not_ok(array->array_->delete_metadata(key)); + array->array_->delete_metadata(key); return TILEDB_OK; } @@ -3052,7 +3052,7 @@ int32_t tiledb_array_get_metadata( // Get metadata tiledb::sm::Datatype type; - throw_if_not_ok(array->array_->get_metadata(key, &type, value_num, value)); + array->array_->get_metadata(key, &type, value_num, value); *value_type = static_cast(type); @@ -3065,7 +3065,7 @@ int32_t tiledb_array_get_metadata_num( return TILEDB_ERR; // Get metadata num - throw_if_not_ok(array->array_->get_metadata_num(num)); + *num = array->array_->metadata_num(); return TILEDB_OK; } @@ -3084,8 +3084,7 @@ int32_t tiledb_array_get_metadata_from_index( // Get metadata tiledb::sm::Datatype type; - throw_if_not_ok(array->array_->get_metadata( - index, key, key_len, &type, value_num, value)); + array->array_->get_metadata(index, key, key_len, &type, value_num, value); *value_type = static_cast(type); @@ -3102,13 +3101,11 @@ int32_t tiledb_array_has_metadata_key( return TILEDB_ERR; // Check whether metadata has_key - bool has_the_key; - tiledb::sm::Datatype type; - throw_if_not_ok(array->array_->has_metadata_key(key, &type, &has_the_key)); + std::optional type = array->array_->metadata_type(key); - *has_key = has_the_key ? 1 : 0; - if (has_the_key) { - *value_type = static_cast(type); + *has_key = type.has_value(); + if (*has_key) { + *value_type = static_cast(type.value()); } return TILEDB_OK; } diff --git a/tiledb/sm/group/group.cc b/tiledb/sm/group/group.cc index 92fd1a52b41..fddea6ca412 100644 --- a/tiledb/sm/group/group.cc +++ b/tiledb/sm/group/group.cc @@ -352,85 +352,78 @@ void Group::delete_group(const URI& uri, bool recursive) { throw_if_not_ok(this->close()); } -Status Group::delete_metadata(const char* key) { +void Group::delete_metadata(const char* key) { // Check if group is open if (!is_open_) - return Status_GroupError("Cannot delete metadata. Group is not open"); + throw GroupStatusException("Cannot delete metadata. Group is not open"); // Check mode if (query_type_ != QueryType::WRITE && query_type_ != QueryType::MODIFY_EXCLUSIVE) - return Status_GroupError( - "Cannot delete metadata. Group was " - "not opened in write or modify_exclusive mode"); + throw GroupStatusException( + "Cannot delete metadata. Group was not opened in write or " + "modify_exclusive mode"); // Check if key is null if (key == nullptr) - return Status_GroupError("Cannot delete metadata. Key cannot be null"); + throw GroupStatusException("Cannot delete metadata. Key cannot be null"); - RETURN_NOT_OK(metadata_.del(key)); - - return Status::Ok(); + metadata_.del(key); } -Status Group::put_metadata( +void Group::put_metadata( const char* key, Datatype value_type, uint32_t value_num, const void* value) { // Check if group is open if (!is_open_) - return Status_GroupError("Cannot put metadata; Group is not open"); + throw GroupStatusException("Cannot put metadata; Group is not open"); // Check mode if (query_type_ != QueryType::WRITE && query_type_ != QueryType::MODIFY_EXCLUSIVE) - return Status_GroupError( - "Cannot put metadata; Group was " - "not opened in write or modify_exclusive mode"); + throw GroupStatusException( + "Cannot put metadata; Group was not opened in write or " + "modify_exclusive mode"); // Check if key is null if (key == nullptr) - return Status_GroupError("Cannot put metadata; Key cannot be null"); + throw GroupStatusException("Cannot put metadata; Key cannot be null"); // Check if value type is ANY if (value_type == Datatype::ANY) - return Status_GroupError("Cannot put metadata; Value type cannot be ANY"); + throw GroupStatusException("Cannot put metadata; Value type cannot be ANY"); - RETURN_NOT_OK(metadata_.put(key, value_type, value_num, value)); - - return Status::Ok(); + metadata_.put(key, value_type, value_num, value); } -Status Group::get_metadata( +void Group::get_metadata( const char* key, Datatype* value_type, uint32_t* value_num, const void** value) { // Check if group is open if (!is_open_) - return Status_GroupError("Cannot get metadata; Group is not open"); + throw GroupStatusException("Cannot get metadata; Group is not open"); // Check mode if (query_type_ != QueryType::READ) - return Status_GroupError( - "Cannot get metadata; Group was " - "not opened in read mode"); + throw GroupStatusException( + "Cannot get metadata; Group was not opened in read mode"); // Check if key is null if (key == nullptr) - return Status_GroupError("Cannot get metadata; Key cannot be null"); + throw GroupStatusException("Cannot get metadata; Key cannot be null"); // Load group metadata, if not loaded yet if (!metadata_loaded_) - RETURN_NOT_OK(load_metadata()); + load_metadata(); - RETURN_NOT_OK(metadata_.get(key, value_type, value_num, value)); - - return Status::Ok(); + metadata_.get(key, value_type, value_num, value); } -Status Group::get_metadata( +void Group::get_metadata( uint64_t index, const char** key, uint32_t* key_len, @@ -439,68 +432,57 @@ Status Group::get_metadata( const void** value) { // Check if group is open if (!is_open_) - return Status_GroupError("Cannot get metadata; Group is not open"); + throw GroupStatusException("Cannot get metadata; Group is not open"); // Check mode if (query_type_ != QueryType::READ) - return Status_GroupError( - "Cannot get metadata; Group was " - "not opened in read mode"); + throw GroupStatusException( + "Cannot get metadata; Group was not opened in read mode"); // Load group metadata, if not loaded yet if (!metadata_loaded_) - RETURN_NOT_OK(load_metadata()); - - RETURN_NOT_OK( - metadata_.get(index, key, key_len, value_type, value_num, value)); + load_metadata(); - return Status::Ok(); + metadata_.get(index, key, key_len, value_type, value_num, value); } -Status Group::get_metadata_num(uint64_t* num) { +uint64_t Group::get_metadata_num() { // Check if group is open if (!is_open_) - return Status_GroupError( + throw GroupStatusException( "Cannot get number of metadata; Group is not open"); // Check mode if (query_type_ != QueryType::READ) - return Status_GroupError( - "Cannot get number of metadata; Group was " - "not opened in read mode"); + throw GroupStatusException( + "Cannot get number of metadata; Group was not opened in read mode"); // Load group metadata, if not loaded yet if (!metadata_loaded_) - RETURN_NOT_OK(load_metadata()); + load_metadata(); - *num = metadata_.num(); - - return Status::Ok(); + return metadata_.num(); } -Status Group::has_metadata_key( - const char* key, Datatype* value_type, bool* has_key) { +std::optional Group::metadata_type(const char* key) { // Check if group is open if (!is_open_) - return Status_GroupError("Cannot get metadata; Group is not open"); + throw GroupStatusException("Cannot get metadata; Group is not open"); // Check mode if (query_type_ != QueryType::READ) - return Status_GroupError( - "Cannot get metadata; Group was " - "not opened in read mode"); + throw GroupStatusException( + "Cannot get metadata; Group was not opened in read mode"); // Check if key is null if (key == nullptr) - return Status_GroupError("Cannot get metadata; Key cannot be null"); + throw GroupStatusException("Cannot get metadata; Key cannot be null"); // Load group metadata, if not loaded yet if (!metadata_loaded_) - RETURN_NOT_OK(load_metadata()); + load_metadata(); - RETURN_NOT_OK(metadata_.has_key(key, value_type, has_key)); - - return Status::Ok(); + return metadata_.metadata_type(key); } Metadata* Group::unsafe_metadata() { @@ -514,7 +496,7 @@ const Metadata* Group::metadata() const { Status Group::metadata(Metadata** metadata) { // Load group metadata, if not loaded yet if (!metadata_loaded_) - RETURN_NOT_OK(load_metadata()); + load_metadata(); *metadata = &metadata_; @@ -749,20 +731,20 @@ tuple> Group::generate_name() const { return {Status::Ok(), ss.str()}; } -Status Group::load_metadata() { +void Group::load_metadata() { if (remote_) { auto rest_client = storage_manager_->rest_client(); if (rest_client == nullptr) - return Status_GroupError( + throw GroupStatusException( "Cannot load metadata; remote group with no REST client."); - RETURN_NOT_OK(rest_client->post_group_metadata_from_rest(group_uri_, this)); + throw_if_not_ok( + rest_client->post_group_metadata_from_rest(group_uri_, this)); } else { assert(group_dir_->loaded()); storage_manager_->load_group_metadata( group_dir_, *encryption_key_, &metadata_); } metadata_loaded_ = true; - return Status::Ok(); } } // namespace sm diff --git a/tiledb/sm/group/group.h b/tiledb/sm/group/group.h index b558481a6ea..ad2af079041 100644 --- a/tiledb/sm/group/group.h +++ b/tiledb/sm/group/group.h @@ -108,9 +108,8 @@ class Group { * Deletes metadata from an group opened in WRITE mode. * * @param key The key of the metadata item to be deleted. - * @return Status */ - Status delete_metadata(const char* key); + void delete_metadata(const char* key); /** * Puts metadata into an group opened in WRITE mode. @@ -122,9 +121,8 @@ class Group { * same datatype. This argument indicates the number of items in the * value component of the metadata. * @param value The metadata value in binary form. - * @return Status */ - Status put_metadata( + void put_metadata( const char* key, Datatype value_type, uint32_t value_num, @@ -141,9 +139,8 @@ class Group { * same datatype. This argument indicates the number of items in the * value component of the metadata. * @param value The metadata value in binary form. - * @return Status */ - Status get_metadata( + void get_metadata( const char* key, Datatype* value_type, uint32_t* value_num, @@ -160,9 +157,8 @@ class Group { * same datatype. This argument indicates the number of items in the * value component of the metadata. * @param value The metadata value in binary form. - * @return Status */ - Status get_metadata( + void get_metadata( uint64_t index, const char** key, uint32_t* key_len, @@ -171,10 +167,10 @@ class Group { const void** value); /** Returns the number of group metadata items. */ - Status get_metadata_num(uint64_t* num); + uint64_t get_metadata_num(); - /** Sets has_key == 1 and corresponding value_type if the group has key. */ - Status has_metadata_key(const char* key, Datatype* value_type, bool* has_key); + /** Gets the type of the given metadata or nullopt if it does not exist. */ + std::optional metadata_type(const char* key); /** Retrieves the group metadata object. */ Status metadata(Metadata** metadata); @@ -454,9 +450,8 @@ class Group { /** * Load group metadata, handles remote groups vs non-remote groups - * @return Status */ - Status load_metadata(); + void load_metadata(); /** * Generate new name in the form of timestmap_timestamp_uuid diff --git a/tiledb/sm/metadata/metadata.cc b/tiledb/sm/metadata/metadata.cc index f28bc170c32..ff726690c12 100644 --- a/tiledb/sm/metadata/metadata.cc +++ b/tiledb/sm/metadata/metadata.cc @@ -31,6 +31,7 @@ */ #include "tiledb/sm/metadata/metadata.h" +#include "tiledb/common/exception/exception.h" #include "tiledb/common/logger.h" #include "tiledb/sm/buffer/buffer.h" #include "tiledb/sm/enums/datatype.h" @@ -46,10 +47,12 @@ using namespace tiledb::common; namespace tiledb { namespace sm { -/** Return a Metadata error class Status with a given message **/ -inline Status Status_MetadataError(const std::string& msg) { - return {"[TileDB::Metadata] Error", msg}; -} +class MetadataException : public StatusException { + public: + explicit MetadataException(const std::string& message) + : StatusException("Metadata", message) { + } +}; /* ********************************* */ /* CONSTRUCTORS & DESTRUCTORS */ @@ -191,7 +194,7 @@ const std::pair& Metadata::timestamp_range() const { return timestamp_range_; } -Status Metadata::del(const char* key) { +void Metadata::del(const char* key) { assert(key != nullptr); std::unique_lock lck(mtx_); @@ -200,11 +203,9 @@ Status Metadata::del(const char* key) { value.del_ = 1; metadata_map_.insert_or_assign(key, std::move(value)); build_metadata_index(); - - return Status::Ok(); } -Status Metadata::put( +void Metadata::put( const char* key, Datatype value_type, uint32_t value_num, @@ -228,11 +229,9 @@ Status Metadata::put( } metadata_map_.insert_or_assign(key, std::move(value_struct)); build_metadata_index(); - - return Status::Ok(); } -Status Metadata::get( +void Metadata::get( const char* key, Datatype* value_type, uint32_t* value_num, @@ -243,7 +242,7 @@ Status Metadata::get( if (it == metadata_map_.end()) { // Key not found *value = nullptr; - return Status::Ok(); + return; } // Key found @@ -257,11 +256,9 @@ Status Metadata::get( *value_num = value_struct.num_; *value = (const void*)(value_struct.value_.data()); } - - return Status::Ok(); } -Status Metadata::get( +void Metadata::get( uint64_t index, const char** key, uint32_t* key_len, @@ -272,8 +269,7 @@ Status Metadata::get( build_metadata_index(); if (index >= metadata_index_.size()) - return LOG_STATUS( - Status_MetadataError("Cannot get metadata; index out of bounds")); + throw MetadataException("Cannot get metadata; index out of bounds"); // Get key auto& key_str = *(metadata_index_[index].first); @@ -291,25 +287,20 @@ Status Metadata::get( *value_num = value_struct.num_; *value = (const void*)(value_struct.value_.data()); } - return Status::Ok(); } -Status Metadata::has_key(const char* key, Datatype* value_type, bool* has_key) { +std::optional Metadata::metadata_type(const char* key) { assert(key != nullptr); auto it = metadata_map_.find(key); if (it == metadata_map_.end()) { // Key not found - *has_key = false; - return Status::Ok(); + return nullopt; } // Key found auto& value_struct = it->second; - *value_type = static_cast(value_struct.type_); - *has_key = true; - - return Status::Ok(); + return static_cast(value_struct.type_); } uint64_t Metadata::num() const { diff --git a/tiledb/sm/metadata/metadata.h b/tiledb/sm/metadata/metadata.h index 13da9c0999b..4da002b92db 100644 --- a/tiledb/sm/metadata/metadata.h +++ b/tiledb/sm/metadata/metadata.h @@ -40,7 +40,6 @@ #include "tiledb/common/common.h" #include "tiledb/common/heap_memory.h" -#include "tiledb/common/status.h" #include "tiledb/sm/filesystem/uri.h" #include "tiledb/sm/tile/tile.h" #include "tiledb/storage_format/serialization/serializers.h" @@ -135,9 +134,8 @@ class Metadata { * Deletes a metadata item. * * @param key The key of the metadata to be deleted. - * @return Status */ - Status del(const char* key); + void del(const char* key); /** * Puts a metadata item as a key-value pair. @@ -147,9 +145,8 @@ class Metadata { * @param value_num The number of items in the value part (they could be more * than one). * @param value The metadata value. - * @return Status */ - Status put( + void put( const char* key, Datatype value_type, uint32_t value_num, @@ -164,9 +161,8 @@ class Metadata { * more than one). * @param[out] value The metadata value. It will be `nullptr` if the key does * not exist - * @return Status */ - Status get( + void get( const char* key, Datatype* value_type, uint32_t* value_num, @@ -183,9 +179,8 @@ class Metadata { * more than one). * @param[out] value The metadata value. It will be `nullptr` if the key does * not exist - * @return Status */ - Status get( + void get( uint64_t index, const char** key, uint32_t* key_len, @@ -196,16 +191,8 @@ class Metadata { /** Returns the number of metadata items. */ uint64_t num() const; - /** - * Checks if metadata has specified key. - * - * @param key The metadata key. - * @param value_type The datatype of the value. - * @param value Set to `1` if the array metadata has a key of the - * given name, else `0`. - * @return Status - */ - Status has_key(const char* key, Datatype* value_type, bool* has_key); + /** Gets the type of the given metadata or nullopt if it does not exist. */ + std::optional metadata_type(const char* key); /** * Sets the URIs of the metadata files that have been loaded diff --git a/tiledb/sm/metadata/test/unit_metadata.cc b/tiledb/sm/metadata/test/unit_metadata.cc index c98cd50303d..cf606da4de3 100644 --- a/tiledb/sm/metadata/test/unit_metadata.cc +++ b/tiledb/sm/metadata/test/unit_metadata.cc @@ -70,9 +70,8 @@ TEST_CASE( std::string value_3 = "strmetadata"; uint32_t value_3_size = static_cast(value_3.size()); - CHECK(metadata_to_serialize1 - .put(key_1.c_str(), Datatype::INT32, 2, value_1_vector.data()) - .ok()); + metadata_to_serialize1.put( + key_1.c_str(), Datatype::INT32, 2, value_1_vector.data()); SizeComputationSerializer size_computation_serializer1; metadata_to_serialize1.serialize(size_computation_serializer1); @@ -82,9 +81,7 @@ TEST_CASE( Serializer serializer1(tile1.data(), tile1.size()); metadata_to_serialize1.serialize(serializer1); - CHECK( - metadata_to_serialize2.put(key_2.c_str(), Datatype::FLOAT64, 1, &value_2) - .ok()); + metadata_to_serialize2.put(key_2.c_str(), Datatype::FLOAT64, 1, &value_2); SizeComputationSerializer size_computation_serializer2; metadata_to_serialize2.serialize(size_computation_serializer2); @@ -94,13 +91,8 @@ TEST_CASE( Serializer serializer2(tile2.data(), tile2.size()); metadata_to_serialize2.serialize(serializer2); - CHECK(metadata_to_serialize3 - .put( - key_3.c_str(), - Datatype::STRING_ASCII, - value_3_size, - value_3.data()) - .ok()); + metadata_to_serialize3.put( + key_3.c_str(), Datatype::STRING_ASCII, value_3_size, value_3.data()); SizeComputationSerializer size_computation_serializer3; metadata_to_serialize3.serialize(size_computation_serializer3); @@ -151,7 +143,7 @@ TEST_CASE( // Read key_1 metadata const int32_t* v_1; - CHECK(meta.get("key1", &type, &v_num, (const void**)(&v_1)).ok()); + meta.get("key1", &type, &v_num, (const void**)(&v_1)); CHECK(type == Datatype::INT32); CHECK(v_num == (uint32_t)(value_1_vector.size())); CHECK(*(v_1) == 100); @@ -159,14 +151,14 @@ TEST_CASE( // Read key_2 metadata const double* v_2; - CHECK(meta.get("key2", &type, &v_num, (const void**)(&v_2)).ok()); + meta.get("key2", &type, &v_num, (const void**)(&v_2)); CHECK(type == Datatype::FLOAT64); CHECK(v_num == value_2_size); CHECK(*(v_2) == value_2); // Read key_3 metadata const char* v_3; - CHECK(meta.get("key3", &type, &v_num, (const void**)(&v_3)).ok()); + meta.get("key3", &type, &v_num, (const void**)(&v_3)); CHECK(type == Datatype::STRING_ASCII); CHECK(v_num == value_3_size); CHECK(std::string(v_3, value_3_size) == value_3); diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index a1b632addcc..a305a187d47 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -115,9 +115,9 @@ Status metadata_from_capnp( } if (entry_reader.getDel()) { - RETURN_NOT_OK(metadata->del(key.c_str())); + metadata->del(key.c_str()); } else { - RETURN_NOT_OK(metadata->put(key.c_str(), type, value_num, value)); + metadata->put(key.c_str(), type, value_num, value); } }