Skip to content

Commit

Permalink
Add API to dump groups that returns a string handle. (#5367)
Browse files Browse the repository at this point in the history
[SC-59017](https://app.shortcut.com/tiledb-inc/story/59017/add-tiledb-group-dump-str-v2-function-that-returns-a-string-handle)

This PR adds a new C API `tiledb_group_dump_str_v2` that dumps a group
into a `tiledb_string_t*`. This allows the string to be safely freed
under certain circumstances (such as when debug and release CRT are
mixed on Windows).

The existing `tiledb_group_dump_str` API was deprecated. Migrating to
the new API is straightforward and requires no changes to the public
surface of the higher-level APIs. The C++ API and the C groups example
are migrated in this PR.

---
TYPE: C_API
DESC: Added `tiledb_group_dump_str_v2` API that returns a
`tiledb_string_t*`. The existing `tiledb_group_dump_str` API is
deprecated.
  • Loading branch information
teo-tsirpanis authored Nov 14, 2024
1 parent 808640a commit 871d2ed
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 20 deletions.
4 changes: 4 additions & 0 deletions doc/policy/api_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ Function removal shall be announced one release cycle before removal, following
- `tiledb_group_get_member_by_index`
- `tiledb_group_get_member_by_name`

### 2.27.0..2.28.0

- `tiledb_group_dump_str`

## Deprecation History Generation

<details>
Expand Down
11 changes: 7 additions & 4 deletions examples/c_api/groups.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,15 @@ void print_group() {
tiledb_group_alloc(ctx, "my_group", &my_group);
tiledb_group_open(ctx, my_group, TILEDB_READ);

char* str;
tiledb_group_dump_str(ctx, my_group, &str, 1);
tiledb_string_t* str;
const char* str_ptr;
size_t str_len;
tiledb_group_dump_str_v2(ctx, my_group, &str, 1);

printf("%s\n", str);
free(str);
tiledb_string_view(str, &str_ptr, &str_len);
printf("%s\n", str_ptr);

tiledb_string_free(&str);
tiledb_group_close(ctx, my_group);
tiledb_group_free(&my_group);
}
Expand Down
38 changes: 30 additions & 8 deletions test/src/unit-capi-group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1563,14 +1563,36 @@ TEST_CASE_METHOD(
rc = tiledb_group_open(ctx_, group1, TILEDB_READ);
REQUIRE(rc == TILEDB_OK);

char* group_dump;
rc = tiledb_group_dump_str(ctx_, group1, &group_dump, 1);
REQUIRE(rc == TILEDB_OK);
REQUIRE(group_dump != nullptr);
REQUIRE(
std::string(group_dump) ==
"group1 GROUP\n|-- group2 GROUP (does not exist)\n");
free(group_dump);
std::string group_dump;
// Test both the new and the deprecated API. Remove the else part when the
// deprecated API gets removed.
bool use_dump_v2 = GENERATE(true, false);
if (use_dump_v2) {
tiledb_string_t* group_dump_handle;
rc = tiledb_group_dump_str_v2(ctx_, group1, &group_dump_handle, 1);
REQUIRE(rc == TILEDB_OK);
REQUIRE(group_dump_handle != nullptr);
const char* group_dump_ptr;
size_t group_dump_size;
rc = tiledb_string_view(
group_dump_handle, &group_dump_ptr, &group_dump_size);
REQUIRE(rc == TILEDB_OK);
REQUIRE(group_dump_ptr != nullptr);
group_dump = std::string(group_dump_ptr, group_dump_size);
rc = tiledb_string_free(&group_dump_handle);
REQUIRE(rc == TILEDB_OK);
} else {
char* group_dump_ptr;
rc = tiledb_group_dump_str(ctx_, group1, &group_dump_ptr, 1);
REQUIRE(rc == TILEDB_OK);
REQUIRE(group_dump_ptr != nullptr);
group_dump = std::string(group_dump_ptr);
// Usually it's not safe to call free() on this pointer, but since we are
// building everything at the same time, tiledb's and tiledb_unit's
// allocators are the same.
free(group_dump_ptr);
}
REQUIRE(group_dump == "group1 GROUP\n|-- group2 GROUP (does not exist)\n");

rc = tiledb_group_close(ctx_, group1);
REQUIRE(rc == TILEDB_OK);
Expand Down
23 changes: 23 additions & 0 deletions tiledb/api/c_api/group/group_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,19 @@ capi_return_t tiledb_group_dump_str(
return TILEDB_OK;
}

capi_return_t tiledb_group_dump_str_v2(
tiledb_group_handle_t* group,
tiledb_string_handle_t** dump_ascii,
const uint8_t recursive) {
ensure_group_is_valid(group);
ensure_output_pointer_is_valid(dump_ascii);

*dump_ascii =
tiledb_string_handle_t::make_handle(group->group().dump(2, 0, recursive));

return TILEDB_OK;
}

capi_return_t tiledb_serialize_group(
tiledb_ctx_handle_t* ctx,
const tiledb_group_handle_t* group,
Expand Down Expand Up @@ -787,6 +800,16 @@ CAPI_INTERFACE(
ctx, group, dump_ascii, recursive);
}

CAPI_INTERFACE(
group_dump_str_v2,
tiledb_ctx_t* ctx,
tiledb_group_t* group,
tiledb_string_handle_t** dump_ascii,
const uint8_t recursive) {
return api_entry_context<tiledb::api::tiledb_group_dump_str_v2>(
ctx, group, dump_ascii, recursive);
}

CAPI_INTERFACE(
serialize_group,
tiledb_ctx_t* ctx,
Expand Down
34 changes: 31 additions & 3 deletions tiledb/api/c_api/group/group_api_external.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* 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.
*
Expand Down
8 changes: 3 additions & 5 deletions tiledb/sm/cpp_api/group.h
Original file line number Diff line number Diff line change
Expand Up @@ -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));

std::string ret(str);
free(str);
return ret;
return impl::convert_to_string(&str).value();
}

/**
Expand Down

0 comments on commit 871d2ed

Please sign in to comment.