Skip to content

Commit

Permalink
Simplify handling of unsupported C API stride arguments (#4963)
Browse files Browse the repository at this point in the history 
The C API contains arguments for `stride` that are unsupported. This PR
simplifies the internal handling of them. The main tactic for
simplification is deal with these arguments in the C API implementation
function and remove the arguments from the functions that support them.

Add `ensure_subarray_is_valid` replaces the old `sanity_check` function
for subarray. Added validation to all relevant dimension label
functions, which heretofore weren't validating subarray arguments in any
way.

---
TYPE: NO_HISTORY
DESC: Simplify handling of unsupported C API `stride` arguments
  • Loading branch information
@eric-hughes-tiledb
eric-hughes-tiledb authored May 13, 2024
1 parent 8de7d1c commit 62098dc
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 154 deletions.
15 changes: 15 additions & 0 deletions tiledb/api/c_api_support/argument_validation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,21 @@ inline void ensure_output_pointer_is_valid(void* p) {
}
}

/**
* Ensure that the output pointer for a stride argument is null.
*
* The C API has arguments for the "stride" of a range, but does not support
* such arguments at the present time. This validation ensures that the argument
* is null.
*
* @param p The value of a `stride` argument to a C API function
*/
inline void ensure_unsupported_stride_is_null(const void* p) {
if (p != nullptr) {
throw CAPIException("Stride is currently unsupported");
}
}

} // namespace tiledb::api

#endif // TILEDB_CAPI_SUPPORT_ARGUMENT_VALIDATION_H
18 changes: 9 additions & 9 deletions tiledb/sm/c_api/api_argument_validator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,17 @@ inline int32_t sanity_check(tiledb_ctx_t* ctx, const tiledb_array_t* array) {
return TILEDB_OK;
}

inline int32_t sanity_check(
tiledb_ctx_t* ctx, const tiledb_subarray_t* subarray) {
if (subarray == nullptr || subarray->subarray_ == nullptr ||
subarray->subarray_->array() == nullptr) {
auto st = Status_Error("Invalid TileDB subarray object");
LOG_STATUS_NO_RETURN_VALUE(st);
save_error(ctx, st);
return TILEDB_ERR;
namespace tiledb::api {
/**
* Returns if a subarray handle (old style) is valid. Throws otherwise.
*/
inline void ensure_subarray_is_valid(const tiledb_subarray_t* p) {
if (p == nullptr || p->subarray_ == nullptr ||
p->subarray_->array() == nullptr) {
throw CAPIException("Invalid TileDB subarray object");
}
return TILEDB_OK;
}
} // namespace tiledb::api

inline int32_t sanity_check(
tiledb_ctx_t* ctx, const tiledb_array_schema_t* array_schema) {
Expand Down
115 changes: 51 additions & 64 deletions tiledb/sm/c_api/tiledb.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1072,12 +1072,11 @@ int32_t tiledb_query_set_subarray_t(
tiledb_query_t* query,
const tiledb_subarray_t* subarray) {
// Sanity check
if (sanity_check(ctx, query) == TILEDB_ERR ||
sanity_check(ctx, subarray) == TILEDB_ERR)
if (sanity_check(ctx, query) == TILEDB_ERR) {
return TILEDB_ERR;

}
ensure_subarray_is_valid(subarray);
query->query_->set_subarray(*subarray->subarray_);

return TILEDB_OK;
}

Expand Down Expand Up @@ -1762,9 +1761,8 @@ capi_return_t tiledb_subarray_alloc(
}

int32_t tiledb_subarray_set_config(
tiledb_ctx_t* ctx, tiledb_subarray_t* subarray, tiledb_config_t* config) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
tiledb_ctx_t*, tiledb_subarray_t* subarray, tiledb_config_t* config) {
ensure_subarray_is_valid(subarray);
api::ensure_config_is_valid(config);
subarray->subarray_->set_config(
tiledb::sm::QueryType::READ, config->config());
Expand All @@ -1784,196 +1782,185 @@ void tiledb_subarray_free(tiledb_subarray_t** subarray) {
}

int32_t tiledb_subarray_set_coalesce_ranges(
tiledb_ctx_t* ctx, tiledb_subarray_t* subarray, int coalesce_ranges) {
// Sanity check
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
tiledb_ctx_t*, tiledb_subarray_t* subarray, int coalesce_ranges) {
ensure_subarray_is_valid(subarray);
subarray->subarray_->set_coalesce_ranges(coalesce_ranges != 0);
return TILEDB_OK;
}

int32_t tiledb_subarray_set_subarray(
tiledb_ctx_t* ctx,
tiledb_subarray_t* subarray_obj,
const void* subarray_vals) {
if (sanity_check(ctx, subarray_obj) == TILEDB_ERR)
return TILEDB_ERR;

tiledb_ctx_t*, tiledb_subarray_t* subarray_obj, const void* subarray_vals) {
ensure_subarray_is_valid(subarray_obj);
subarray_obj->subarray_->set_subarray(subarray_vals);

return TILEDB_OK;
}

int32_t tiledb_subarray_add_range(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
tiledb_subarray_t* subarray,
uint32_t dim_idx,
const void* start,
const void* end,
const void* stride) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;

subarray->subarray_->add_range(dim_idx, start, end, stride);

ensure_subarray_is_valid(subarray);
ensure_unsupported_stride_is_null(stride);
subarray->subarray_->add_range(dim_idx, start, end);
return TILEDB_OK;
}

int32_t tiledb_subarray_add_point_ranges(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
tiledb_subarray_t* subarray,
uint32_t dim_idx,
const void* start,
uint64_t count) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->add_point_ranges(dim_idx, start, count);
return TILEDB_OK;
}

int32_t tiledb_subarray_add_range_by_name(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
tiledb_subarray_t* subarray,
const char* dim_name,
const void* start,
const void* end,
const void* stride) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
subarray->subarray_->add_range_by_name(dim_name, start, end, stride);
ensure_subarray_is_valid(subarray);
ensure_unsupported_stride_is_null(stride);
subarray->subarray_->add_range_by_name(dim_name, start, end);
return TILEDB_OK;
}

int32_t tiledb_subarray_add_range_var(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
tiledb_subarray_t* subarray,
uint32_t dim_idx,
const void* start,
uint64_t start_size,
const void* end,
uint64_t end_size) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->add_range_var(dim_idx, start, start_size, end, end_size);
return TILEDB_OK;
}

int32_t tiledb_subarray_add_range_var_by_name(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
tiledb_subarray_t* subarray,
const char* dim_name,
const void* start,
uint64_t start_size,
const void* end,
uint64_t end_size) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->add_range_var_by_name(
dim_name, start, start_size, end, end_size);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range_num(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
uint32_t dim_idx,
uint64_t* range_num) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_range_num(dim_idx, range_num);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range_num_from_name(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
const char* dim_name,
uint64_t* range_num) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_range_num_from_name(dim_name, range_num);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
uint32_t dim_idx,
uint64_t range_idx,
const void** start,
const void** end,
const void** stride) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
subarray->subarray_->get_range(dim_idx, range_idx, start, end, stride);
ensure_subarray_is_valid(subarray);
ensure_output_pointer_is_valid(start);
ensure_output_pointer_is_valid(end);
if (stride != nullptr) {
*stride = nullptr;
}
subarray->subarray_->get_range(dim_idx, range_idx, start, end);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range_var_size(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
uint32_t dim_idx,
uint64_t range_idx,
uint64_t* start_size,
uint64_t* end_size) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_range_var_size(
dim_idx, range_idx, start_size, end_size);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range_from_name(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
const char* dim_name,
uint64_t range_idx,
const void** start,
const void** end,
const void** stride) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
subarray->subarray_->get_range_from_name(
dim_name, range_idx, start, end, stride);
ensure_subarray_is_valid(subarray);
ensure_output_pointer_is_valid(start);
ensure_output_pointer_is_valid(end);
if (stride != nullptr) {
*stride = nullptr;
}
subarray->subarray_->get_range_from_name(dim_name, range_idx, start, end);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range_var_size_from_name(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
const char* dim_name,
uint64_t range_idx,
uint64_t* start_size,
uint64_t* end_size) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_range_var_size_from_name(
dim_name, range_idx, start_size, end_size);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range_var(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
uint32_t dim_idx,
uint64_t range_idx,
void* start,
void* end) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_range_var(dim_idx, range_idx, start, end);
return TILEDB_OK;
}

int32_t tiledb_subarray_get_range_var_from_name(
tiledb_ctx_t* ctx,
tiledb_ctx_t*,
const tiledb_subarray_t* subarray,
const char* dim_name,
uint64_t range_idx,
void* start,
void* end) {
if (sanity_check(ctx, subarray) == TILEDB_ERR)
return TILEDB_ERR;
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_range_var_from_name(dim_name, range_idx, start, end);
return TILEDB_OK;
}
Expand Down
18 changes: 16 additions & 2 deletions tiledb/sm/c_api/tiledb_dimension_label.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ capi_return_t tiledb_subarray_add_label_range(
const void* start,
const void* end,
const void* stride) {
subarray->subarray_->add_label_range(label_name, start, end, stride);
ensure_subarray_is_valid(subarray);
ensure_unsupported_stride_is_null(stride);
subarray->subarray_->add_label_range(label_name, start, end);
return TILEDB_OK;
}

Expand All @@ -141,13 +143,15 @@ capi_return_t tiledb_subarray_add_label_range_var(
uint64_t start_size,
const void* end,
uint64_t end_size) {
ensure_subarray_is_valid(subarray);
subarray->subarray_->add_label_range_var(
label_name, start, start_size, end, end_size);
return TILEDB_OK;
}

capi_return_t tiledb_subarray_get_label_name(
tiledb_subarray_t* subarray, uint32_t dim_idx, const char** label_name) {
ensure_subarray_is_valid(subarray);
const auto& name = subarray->subarray_->get_label_name(dim_idx);
*label_name = name.c_str();
return TILEDB_OK;
Expand All @@ -160,14 +164,21 @@ capi_return_t tiledb_subarray_get_label_range(
const void** start,
const void** end,
const void** stride) {
subarray->subarray_->get_label_range(dim_name, range_idx, start, end, stride);
ensure_subarray_is_valid(subarray);
ensure_output_pointer_is_valid(start);
ensure_output_pointer_is_valid(end);
if (stride != nullptr) {
*stride = nullptr;
}
subarray->subarray_->get_label_range(dim_name, range_idx, start, end);
return TILEDB_OK;
}

capi_return_t tiledb_subarray_get_label_range_num(
const tiledb_subarray_t* subarray,
const char* dim_name,
uint64_t* range_num) {
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_label_range_num(dim_name, range_num);
return TILEDB_OK;
}
Expand All @@ -178,6 +189,7 @@ capi_return_t tiledb_subarray_get_label_range_var(
uint64_t range_idx,
void* start,
void* end) {
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_label_range_var(dim_name, range_idx, start, end);
return TILEDB_OK;
}
Expand All @@ -188,6 +200,7 @@ capi_return_t tiledb_subarray_get_label_range_var_size(
uint64_t range_idx,
uint64_t* start_size,
uint64_t* end_size) {
ensure_subarray_is_valid(subarray);
subarray->subarray_->get_label_range_var_size(
dim_name, range_idx, start_size, end_size);
return TILEDB_OK;
Expand All @@ -197,6 +210,7 @@ capi_return_t tiledb_subarray_has_label_ranges(
const tiledb_subarray_t* subarray,
const uint32_t dim_idx,
int32_t* has_label_ranges) {
ensure_subarray_is_valid(subarray);
bool has_ranges = subarray->subarray_->has_label_ranges(dim_idx);
*has_label_ranges = has_ranges ? 1 : 0;
return TILEDB_OK;
Expand Down
Loading

0 comments on commit 62098dc

Please sign in to comment.