-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tiledb_group_get_member_by(index|name)_v2
.
#4019
Changes from 21 commits
4b6d799
6fc5d17
c0f62f6
8dde4aa
b5a07a2
0ce0af4
6959e7b
24847c1
cc325c9
68b7063
8c2fd53
3f73c58
5f48204
4660ebd
948ca02
3920281
35038c6
1b8f8eb
f538bda
d266ee7
308f7f9
7d3450f
23ecd15
f4be46e
b11b572
7b70576
f756520
002d98a
8c017f2
388a38a
d9c858f
b348631
c3eceb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,7 +86,7 @@ struct GroupFx { | |
std::string get_golden_ls(const std::string& path); | ||
static std::string random_name(const std::string& prefix); | ||
std::vector<std::pair<tiledb::sm::URI, tiledb_object_t>> read_group( | ||
tiledb_group_t* group) const; | ||
tiledb_group_t* group, bool use_get_member_by_index_v2 = true) const; | ||
void set_group_timestamp( | ||
tiledb_group_t* group, const uint64_t& timestamp) const; | ||
void write_group_metadata(const char* group_uri) const; | ||
|
@@ -230,20 +230,46 @@ void GroupFx::vacuum(const char* group_uri) const { | |
} | ||
|
||
std::vector<std::pair<tiledb::sm::URI, tiledb_object_t>> GroupFx::read_group( | ||
tiledb_group_t* group) const { | ||
tiledb_group_t* group, bool use_get_member_by_index_v2) const { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would better have been done as a template argument. It's adequate as-is, though. Using a template argument would make removal easier, because there's so little shared code that the two versions could have been defined with specializations instead of |
||
std::vector<std::pair<tiledb::sm::URI, tiledb_object_t>> ret; | ||
uint64_t count = 0; | ||
char* uri; | ||
tiledb_object_t type; | ||
char* name; | ||
int rc = tiledb_group_get_member_count(ctx_, group, &count); | ||
REQUIRE(rc == TILEDB_OK); | ||
for (uint64_t i = 0; i < count; i++) { | ||
rc = tiledb_group_get_member_by_index(ctx_, group, i, &uri, &type, &name); | ||
REQUIRE(rc == TILEDB_OK); | ||
ret.emplace_back(uri, type); | ||
std::free(uri); | ||
std::free(name); | ||
eric-hughes-tiledb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (use_get_member_by_index_v2) { | ||
for (uint64_t i = 0; i < count; i++) { | ||
std::string uri; | ||
tiledb_object_t type; | ||
tiledb_string_t *uri_handle, *name_handle; | ||
rc = tiledb_group_get_member_by_index_v2( | ||
ctx_, group, i, &uri_handle, &type, &name_handle); | ||
REQUIRE(rc == TILEDB_OK); | ||
const char* uri_str; | ||
size_t uri_size; | ||
rc = tiledb_string_view(uri_handle, &uri_str, &uri_size); | ||
CHECK(rc == TILEDB_OK); | ||
uri = std::string(uri_str, uri_size); | ||
rc = tiledb_string_free(&uri_handle); | ||
CHECK(rc == TILEDB_OK); | ||
if (name_handle) { | ||
rc = tiledb_string_free(&name_handle); | ||
CHECK(rc == TILEDB_OK); | ||
} | ||
ret.emplace_back(uri, type); | ||
} | ||
} else { | ||
// When tiledb_group_get_member_by_index gets removed, the else part can | ||
// simply be removed. | ||
for (uint64_t i = 0; i < count; i++) { | ||
std::string uri; | ||
tiledb_object_t type; | ||
char *uri_ptr, *name_ptr; | ||
// NOTE: The following function leaks memory. | ||
davisp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
rc = tiledb_group_get_member_by_index( | ||
ctx_, group, i, &uri_ptr, &type, &name_ptr); | ||
REQUIRE(rc == TILEDB_OK); | ||
uri = uri_ptr == nullptr ? "" : std::string(uri_ptr); | ||
ret.emplace_back(uri, type); | ||
} | ||
} | ||
return ret; | ||
} | ||
|
@@ -549,6 +575,13 @@ TEST_CASE_METHOD( | |
|
||
TEST_CASE_METHOD( | ||
GroupFx, "C API: Group, write/read", "[capi][group][metadata][read]") { | ||
bool use_get_member_by_index_v2 = true; | ||
SECTION("Using tiledb_group_get_member_by_index_v2") { | ||
use_get_member_by_index_v2 = true; | ||
} | ||
SECTION("Using tiledb_group_get_member_by_index") { | ||
use_get_member_by_index_v2 = false; | ||
} | ||
// Create and open group in write mode | ||
// TODO: refactor for each supported FS. | ||
std::string temp_dir = fs_vec_[0]->temp_dir(); | ||
|
@@ -618,12 +651,12 @@ TEST_CASE_METHOD( | |
REQUIRE(rc == TILEDB_OK); | ||
|
||
std::vector<std::pair<tiledb::sm::URI, tiledb_object_t>> group1_received = | ||
read_group(group1); | ||
read_group(group1, use_get_member_by_index_v2); | ||
REQUIRE_THAT( | ||
group1_received, Catch::Matchers::UnorderedEquals(group1_expected)); | ||
|
||
std::vector<std::pair<tiledb::sm::URI, tiledb_object_t>> group2_received = | ||
read_group(group2); | ||
read_group(group2, use_get_member_by_index_v2); | ||
REQUIRE_THAT( | ||
group2_received, Catch::Matchers::UnorderedEquals(group2_expected)); | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -41,6 +41,7 @@ | |||||||||||||||||||||||||||||||||||||||||||
#include "../buffer/buffer_api_internal.h" | ||||||||||||||||||||||||||||||||||||||||||||
#include "../config/config_api_internal.h" | ||||||||||||||||||||||||||||||||||||||||||||
#include "../context/context_api_internal.h" | ||||||||||||||||||||||||||||||||||||||||||||
#include "../string/string_api_internal.h" | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
#include "group_api_internal.h" | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -306,6 +307,10 @@ capi_return_t tiledb_group_get_member_by_index( | |||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(type); | ||||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(name); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
LOG_WARN( | ||||||||||||||||||||||||||||||||||||||||||||
"tiledb_group_get_member_by_index is deprecated. Please use " | ||||||||||||||||||||||||||||||||||||||||||||
"tiledb_group_get_member_by_index_v2 instead."); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
char* tmp_uri = nullptr; | ||||||||||||||||||||||||||||||||||||||||||||
char* tmp_name = nullptr; | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -343,6 +348,34 @@ capi_return_t tiledb_group_get_member_by_index( | |||||||||||||||||||||||||||||||||||||||||||
return TILEDB_OK; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_member_by_index_v2( | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_handle_t* group, | ||||||||||||||||||||||||||||||||||||||||||||
uint64_t index, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_t** uri, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_object_t* type, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_t** name) { | ||||||||||||||||||||||||||||||||||||||||||||
ensure_group_is_valid(group); | ||||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(uri); | ||||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(type); | ||||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(name); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
auto&& [uri_str, object_type, name_str] = | ||||||||||||||||||||||||||||||||||||||||||||
group->group().member_by_index(index); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
*uri = tiledb_string_handle_t::make_handle(uri_str); | ||||||||||||||||||||||||||||||||||||||||||||
*type = static_cast<tiledb_object_t>(object_type); | ||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||
*name = name_str.has_value() ? | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_handle_t::make_handle(*name_str) : | ||||||||||||||||||||||||||||||||||||||||||||
nullptr; | ||||||||||||||||||||||||||||||||||||||||||||
} catch (...) { | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_handle_t::break_handle(*uri); | ||||||||||||||||||||||||||||||||||||||||||||
throw; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return TILEDB_OK; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_member_by_name( | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_handle_t* group, | ||||||||||||||||||||||||||||||||||||||||||||
const char* name, | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -353,6 +386,10 @@ capi_return_t tiledb_group_get_member_by_name( | |||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(uri); | ||||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(type); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
LOG_WARN( | ||||||||||||||||||||||||||||||||||||||||||||
"tiledb_group_get_member_by_name is deprecated. Please use " | ||||||||||||||||||||||||||||||||||||||||||||
"tiledb_group_get_member_by_name_v2 instead."); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
auto&& [uri_str, object_type, name_str, ignored_relative] = | ||||||||||||||||||||||||||||||||||||||||||||
group->group().member_by_name(name); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
|
@@ -366,6 +403,26 @@ capi_return_t tiledb_group_get_member_by_name( | |||||||||||||||||||||||||||||||||||||||||||
return TILEDB_OK; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_member_by_name_v2( | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_handle_t* group, | ||||||||||||||||||||||||||||||||||||||||||||
const char* name, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_t** uri, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_object_t* type) { | ||||||||||||||||||||||||||||||||||||||||||||
ensure_group_is_valid(group); | ||||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(uri); | ||||||||||||||||||||||||||||||||||||||||||||
ensure_output_pointer_is_valid(type); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
std::string uri_str; | ||||||||||||||||||||||||||||||||||||||||||||
sm::ObjectType object_type; | ||||||||||||||||||||||||||||||||||||||||||||
std::tie(uri_str, object_type, std::ignore, std::ignore) = | ||||||||||||||||||||||||||||||||||||||||||||
group->group().member_by_name(name); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
*uri = tiledb_string_handle_t::make_handle(std::move(uri_str)); | ||||||||||||||||||||||||||||||||||||||||||||
*type = static_cast<tiledb_object_t>(object_type); | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return TILEDB_OK; | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_is_relative_uri_by_name( | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_handle_t* group, const char* name, uint8_t* is_relative) { | ||||||||||||||||||||||||||||||||||||||||||||
ensure_group_is_valid(group); | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -695,6 +752,17 @@ capi_return_t tiledb_group_get_member_by_index( | |||||||||||||||||||||||||||||||||||||||||||
ctx, group, index, uri, type, name); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_member_by_index_v2( | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_ctx_t* ctx, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_t* group, | ||||||||||||||||||||||||||||||||||||||||||||
uint64_t index, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_t** uri, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_object_t* type, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_t** name) TILEDB_NOEXCEPT { | ||||||||||||||||||||||||||||||||||||||||||||
return api_entry_context<tiledb::api::tiledb_group_get_member_by_index_v2>( | ||||||||||||||||||||||||||||||||||||||||||||
ctx, group, index, uri, type, name); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It looks like this needs to be rebased and converted to the new CAPI_INTERFACE approach. |
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_member_by_name( | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_ctx_t* ctx, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_t* group, | ||||||||||||||||||||||||||||||||||||||||||||
|
@@ -705,6 +773,16 @@ capi_return_t tiledb_group_get_member_by_name( | |||||||||||||||||||||||||||||||||||||||||||
ctx, group, name, uri, type); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_member_by_name_v2( | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here too on the CAPI_INTERFACE macro. |
||||||||||||||||||||||||||||||||||||||||||||
tiledb_ctx_t* ctx, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_t* group, | ||||||||||||||||||||||||||||||||||||||||||||
const char* name, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_string_t** uri, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_object_t* type) TILEDB_NOEXCEPT { | ||||||||||||||||||||||||||||||||||||||||||||
return api_entry_context<tiledb::api::tiledb_group_get_member_by_name_v2>( | ||||||||||||||||||||||||||||||||||||||||||||
ctx, group, name, uri, type); | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
capi_return_t tiledb_group_get_is_relative_uri_by_name( | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_ctx_t* ctx, | ||||||||||||||||||||||||||||||||||||||||||||
tiledb_group_t* group, | ||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to update the version numbers here.