-
Notifications
You must be signed in to change notification settings - Fork 185
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 API to dump groups that returns a string handle. #5367
Changes from all commits
ea4bdb3
8a518ae
84eb742
9c00aa8
d78c442
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 | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -621,21 +621,49 @@ TILEDB_EXPORT capi_return_t tiledb_group_get_query_type( | |||||||||||||||||||||||||||||||||||||||||
tiledb_query_type_t* query_type) TILEDB_NOEXCEPT; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||
* Dump a string representation of a group | ||||||||||||||||||||||||||||||||||||||||||
* Dump a string representation of a group. | ||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||
* Deprecated, use tiledb_group_dump_str_v2 instead. | ||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||
* @param ctx The TileDB context. | ||||||||||||||||||||||||||||||||||||||||||
* @param group The group. | ||||||||||||||||||||||||||||||||||||||||||
* @param dump_ascii The output string. The caller takes ownership | ||||||||||||||||||||||||||||||||||||||||||
* of the c-string. | ||||||||||||||||||||||||||||||||||||||||||
* @param recursive should we recurse into sub-groups | ||||||||||||||||||||||||||||||||||||||||||
* @param recursive True if the dump should recurse into subgroups. | ||||||||||||||||||||||||||||||||||||||||||
* @return `TILEDB_OK` for success and `TILEDB_ERR` for error. | ||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||
TILEDB_EXPORT capi_return_t tiledb_group_dump_str( | ||||||||||||||||||||||||||||||||||||||||||
TILEDB_DEPRECATED_EXPORT capi_return_t tiledb_group_dump_str( | ||||||||||||||||||||||||||||||||||||||||||
tiledb_ctx_t* ctx, | ||||||||||||||||||||||||||||||||||||||||||
tiledb_group_t* group, | ||||||||||||||||||||||||||||||||||||||||||
char** dump_ascii, | ||||||||||||||||||||||||||||||||||||||||||
const uint8_t recursive) TILEDB_NOEXCEPT; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||
* Dump a string representation of a group. | ||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||
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. Missing example. The example could highlight the user's responsibility to free 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. It's more clear to add an 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. Updated documentation to match other dump APIs. TileDB/tiledb/api/c_api/dimension/dimension_api_external.h Lines 298 to 317 in abca93c
|
||||||||||||||||||||||||||||||||||||||||||
* The output string handle must be freed by the user after use. | ||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||
* **Example:** | ||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||
* @code{.c} | ||||||||||||||||||||||||||||||||||||||||||
* tiledb_string_t* tdb_string; | ||||||||||||||||||||||||||||||||||||||||||
* tiledb_group_dump_str_v2(ctx, group, &tdb_string); | ||||||||||||||||||||||||||||||||||||||||||
* // Use the string | ||||||||||||||||||||||||||||||||||||||||||
* tiledb_string_free(&tdb_string); | ||||||||||||||||||||||||||||||||||||||||||
* @endcode | ||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||
* @param ctx The TileDB context. | ||||||||||||||||||||||||||||||||||||||||||
* @param group The group. | ||||||||||||||||||||||||||||||||||||||||||
* @param dump_ascii The output string handle. | ||||||||||||||||||||||||||||||||||||||||||
* @param recursive True if the dump should recurse into subgroups. | ||||||||||||||||||||||||||||||||||||||||||
* @return `TILEDB_OK` for success and `TILEDB_ERR` for error. | ||||||||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||||||||
TILEDB_EXPORT capi_return_t tiledb_group_dump_str_v2( | ||||||||||||||||||||||||||||||||||||||||||
tiledb_ctx_t* ctx, | ||||||||||||||||||||||||||||||||||||||||||
tiledb_group_t* group, | ||||||||||||||||||||||||||||||||||||||||||
tiledb_string_t** dump_ascii, | ||||||||||||||||||||||||||||||||||||||||||
const uint8_t recursive) TILEDB_NOEXCEPT; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||||||||
* Consolidates the group metadata into a single group metadata file. | ||||||||||||||||||||||||||||||||||||||||||
* | ||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -473,13 +473,11 @@ class Group { | |||||||||||||
std::string dump(const bool recursive) const { | ||||||||||||||
auto& ctx = ctx_.get(); | ||||||||||||||
tiledb_ctx_t* c_ctx = ctx.ptr().get(); | ||||||||||||||
char* str; | ||||||||||||||
tiledb_string_t* str; | ||||||||||||||
ctx.handle_error( | ||||||||||||||
tiledb_group_dump_str(c_ctx, group_.get(), &str, recursive)); | ||||||||||||||
tiledb_group_dump_str_v2(c_ctx, group_.get(), &str, recursive)); | ||||||||||||||
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. I really don't see that much into this code, but don't we need to free it here since we copy it into string? or does the convert_to_string logic frees the string? 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. The latter. We move the handle to a class which frees it when it gets destroyed. TileDB/tiledb/sm/cpp_api/capi_string.h Lines 102 to 107 in abca93c
|
||||||||||||||
|
||||||||||||||
std::string ret(str); | ||||||||||||||
free(str); | ||||||||||||||
return ret; | ||||||||||||||
return impl::convert_to_string(&str).value(); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
/** | ||||||||||||||
|
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 mention the caller has the responsibility to free the allocated
tiledb_string_t
.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.
Mentioned.