Skip to content

Commit

Permalink
Remove statuses from the Metadata code. (#4012)
Browse files Browse the repository at this point in the history 
* 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<Datatype>`.

Applies suggestion from #3972 (comment).

* Remove status.

* Fix error message spacing.

* Fix standalone test.

---------

Co-authored-by: KiterLuc <67824247+KiterLuc@users.noreply.github.com>
Co-authored-by: Luc Rancourt <lucrancourt@gmail.com>
  • Loading branch information
@teo-tsirpanis @KiterLuc
3 people committed Oct 4, 2023
1 parent 74986e1 commit 2c99613
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 250 deletions.
4 changes: 2 additions & 2 deletions test/src/unit-capi-array.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<tiledb_datatype_t>(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<tiledb_datatype_t>(type) == TILEDB_FLOAT32);
CHECK(v_num == 2);
CHECK(((const float*)v_r)[0] == 1.1f);
Expand Down
23 changes: 10 additions & 13 deletions tiledb/api/c_api/group/group_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<tiledb::sm::Datatype>(value_type), value_num, value));
group->group().put_metadata(
key, static_cast<tiledb::sm::Datatype>(value_type), value_num, value);

return TILEDB_OK;
}
Expand All @@ -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;
}
Expand All @@ -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<tiledb_datatype_t>(type);

Expand All @@ -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;
}
Expand All @@ -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<tiledb_datatype_t>(type);

Expand All @@ -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<tiledb::sm::Datatype> type = group->group().metadata_type(key);

*has_key = has_the_key ? 1 : 0;
if (has_the_key) {
*value_type = static_cast<tiledb_datatype_t>(type);
*has_key = type.has_value();
if (*has_key) {
*value_type = static_cast<tiledb_datatype_t>(type.value());
}

return TILEDB_OK;
Expand Down
114 changes: 42 additions & 72 deletions tiledb/sm/array/array.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -615,8 +615,7 @@ std::vector<shared_ptr<const Enumeration>> 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<std::string> names_to_load;
Expand Down Expand Up @@ -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,
Expand All @@ -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<Datatype> 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() {
Expand Down
18 changes: 7 additions & 11 deletions tiledb/sm/array/array.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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<Datatype> metadata_type(const char* key);

/** Retrieves the array metadata object. */
Status metadata(Metadata** metadata);
Expand Down

0 comments on commit 2c99613

Please sign in to comment.