From b484eb6572e889128146f162aac84decf30666dc Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 23 Oct 2024 16:30:52 -0400 Subject: [PATCH 01/23] Add tiledb_array_schema_get_enumeration API --- test/src/unit-cppapi-enumerations.cc | 73 +++++++++++++++++++ .../c_api/array_schema/array_schema_api.cc | 30 ++++++++ .../array_schema_api_experimental.h | 30 ++++++++ .../array_schema/array_schema_api_internal.h | 5 ++ .../test/unit_capi_array_schema.cc | 62 ++++++++++++++++ tiledb/sm/cpp_api/array.h | 12 ++- tiledb/sm/cpp_api/array_schema_experimental.h | 25 +++++++ 7 files changed, 234 insertions(+), 3 deletions(-) diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index 63e3d7d7e3e..def8446600f 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -44,6 +44,18 @@ using namespace tiledb; +static bool is_equivalent_enumeration( + const Enumeration& left, const Enumeration& right) { + return left.name() == right.name() && left.type() == right.type() && + left.cell_val_num() == right.cell_val_num() && + left.ordered() == right.ordered() && + std::equal( + left.ptr()->data().begin(), + left.ptr()->data().end(), + right.ptr()->data().begin(), + right.ptr()->data().end()); +} + struct CPPEnumerationFx { CPPEnumerationFx(); ~CPPEnumerationFx() = default; @@ -313,6 +325,67 @@ TEST_CASE_METHOD( REQUIRE(enmr_name2.has_value() == false); } +TEST_CASE_METHOD( + CPPEnumerationFx, + "CPP: Enumerations From Disk - ArraySchema::get_enumeration", + "[enumeration][array-schema-get-enumeration]") { + create_array(); + + SECTION("default schema load does not populate enumeration") { + auto schema = Array::load_schema(ctx_, uri_); + auto enmr = ArraySchemaExperimental::get_enumeration( + ctx_, schema, "an_enumeration"); + CHECK(!enmr.has_value()); + } + + SECTION("array get_enumeration loads enumeration and schema shares") { + auto array = tiledb::Array(ctx_, uri_, TILEDB_READ); + auto from_array = + ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration"); + + auto schema = array.schema(); + auto from_schema = ArraySchemaExperimental::get_enumeration( + ctx_, schema, "an_enumeration"); + CHECK(is_equivalent_enumeration(from_array, *from_schema)); + } + + SECTION("array get_enumeration loads into user schema handle") { + auto array = tiledb::Array(ctx_, uri_, TILEDB_READ); + auto schema = array.schema(); + + // the enumeration is not populated yet + REQUIRE(!ArraySchemaExperimental::get_enumeration( + ctx_, schema, "an_enumeration") + .has_value()); + + // array method actually loads it + auto from_array = + ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration"); + + // and once it is loaded, it is set on the schema + auto from_schema = ArraySchemaExperimental::get_enumeration( + ctx_, schema, "an_enumeration"); + REQUIRE(from_schema.has_value()); + + CHECK(is_equivalent_enumeration(from_array, *from_schema)); + } + + SECTION("schema load with rest config does populate enumeration") { + Config config; + config["rest.load_enumerations_on_array_open"] = "true"; + + auto schema = Array::load_schema(ctx_, uri_, config); + auto enmr = ArraySchemaExperimental::get_enumeration( + ctx_, schema, "an_enumeration"); + REQUIRE(enmr.has_value()); + REQUIRE(enmr->ptr() != nullptr); + REQUIRE(enmr->name() == "an_enumeration"); + REQUIRE(enmr->type() == TILEDB_STRING_ASCII); + REQUIRE(enmr->cell_val_num() == TILEDB_VAR_NUM); + REQUIRE(enmr->ordered() == false); + } +} + TEST_CASE_METHOD( CPPEnumerationFx, "CPP: Array::load_all_enumerations", diff --git a/tiledb/api/c_api/array_schema/array_schema_api.cc b/tiledb/api/c_api/array_schema/array_schema_api.cc index 36887033bf5..91df3da4d1e 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api.cc +++ b/tiledb/api/c_api/array_schema/array_schema_api.cc @@ -181,6 +181,26 @@ capi_return_t tiledb_array_schema_timestamp_range( return TILEDB_OK; } +capi_return_t tiledb_array_schema_get_enumeration( + tiledb_array_schema_t* array_schema, + const char* enumeration_name, + tiledb_enumeration_t** enumeration) { + ensure_array_schema_is_valid(array_schema); + ensure_output_pointer_is_valid(enumeration); + + if (enumeration_name == nullptr) { + throw CAPIException("'enumeration_name' must not be null"); + } + + auto ptr = array_schema->get_enumeration(enumeration_name); + if (ptr) { + *enumeration = tiledb_enumeration_handle_t::make_handle(ptr); + } else { + *enumeration = nullptr; + } + return TILEDB_OK; +} + capi_return_t tiledb_array_schema_add_enumeration( tiledb_array_schema_t* array_schema, tiledb_enumeration_t* enumeration) { ensure_array_schema_is_valid(array_schema); @@ -540,6 +560,16 @@ CAPI_INTERFACE( ctx, array_schema, lo, hi); } +CAPI_INTERFACE( + array_schema_get_enumeration, + tiledb_ctx_t* ctx, + tiledb_array_schema_t* array_schema, + const char* enumeration_name, + tiledb_enumeration_t** enumeration) { + return api_entry_context( + ctx, array_schema, enumeration_name, enumeration); +} + CAPI_INTERFACE( array_schema_add_enumeration, tiledb_ctx_t* ctx, diff --git a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h index ab050882366..cc8500fd3a9 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h @@ -67,6 +67,36 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( uint64_t* lo, uint64_t* hi) TILEDB_NOEXCEPT; +/** + * Retrieves an already-loaded enumeration given its name. + * + * If the enumeration has already been loaded, + * such as via `tiledb_array_get_enumeration`, + * then this returns a pointer to the enumeration. + * Otherwise this returns `TILEDB_OK` and sets `enumeration` to NULL. + * + * **Example:** + * + * The following retrieves the enumeration named "states" in the schema. + * + * @code{.c} + * tiledb_enumeration_t* enmr; + * tiledb_array_schema_get_enumeration(ctx, array_schema, "states", &enmr); + * tiledb_attribute_free(&enmr); + * @endcode + * + * @param[in] ctx The TileDB context. + * @param[in] array_schema The array schema. + * @param[in] name The name of the enumeration to retrieve. + * @param[out] enmr The enumeration object to retrieve. + * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. + */ +TILEDB_EXPORT capi_return_t tiledb_array_schema_get_enumeration( + tiledb_ctx_t* ctx, + tiledb_array_schema_t* array_schema, + const char* name, + tiledb_enumeration_t** enumeration) TILEDB_NOEXCEPT; + /** * Adds an enumeration to an array schema. * diff --git a/tiledb/api/c_api/array_schema/array_schema_api_internal.h b/tiledb/api/c_api/array_schema/array_schema_api_internal.h index c8daaa13106..41f61f129f9 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_internal.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_internal.h @@ -90,6 +90,11 @@ struct tiledb_array_schema_handle_t dim_id, name, label_order, label_type, check_name); } + shared_ptr get_enumeration( + const char* name) const { + return array_schema_->get_enumeration(name); + } + void add_enumeration(shared_ptr enmr) { return array_schema_->add_enumeration(enmr); } diff --git a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc index 8e86cc4c5e6..b26f6387c0a 100644 --- a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc +++ b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc @@ -361,6 +361,68 @@ TEST_CASE( CHECK(enumeration == nullptr); } +TEST_CASE( + "C API: tiledb_array_schema_get_enumeration argument validation", + "[capi][array_schema]") { + capi_return_t rc; + ordinary_array_schema x{}; + tiledb_enumeration_t* enumeration; + SECTION("null context") { + rc = tiledb_array_schema_get_enumeration( + nullptr, x.schema, "primes", &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_INVALID_CONTEXT); + } + SECTION("null schema") { + rc = tiledb_array_schema_get_enumeration( + x.ctx(), nullptr, "primes", &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } + SECTION("null name") { + rc = tiledb_array_schema_get_enumeration( + x.ctx(), x.schema, nullptr, &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } + SECTION("null enumeration") { + rc = tiledb_array_schema_get_enumeration( + x.ctx(), x.schema, "primes", nullptr); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } + SECTION("success") { + int32_t values[5] = {2, 3, 5, 7, 11}; + rc = tiledb_enumeration_alloc( + x.ctx(), + "primes", + TILEDB_UINT32, + 1, + 0, + values, + sizeof(uint32_t) * 5, + nullptr, + 0, + &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + tiledb_array_schema_add_enumeration(x.ctx(), x.schema, enumeration); + REQUIRE_NOTHROW(tiledb_enumeration_free(&enumeration)); + CHECK(enumeration == nullptr); + + rc = tiledb_array_schema_get_enumeration( + x.ctx(), x.schema, "primes", &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + REQUIRE(enumeration != nullptr); + + tiledb_string_t* tiledb_name(nullptr); + rc = tiledb_enumeration_get_name(x.ctx(), enumeration, &tiledb_name); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + REQUIRE(tiledb_name != nullptr); + + const char* name; + size_t length; + rc = tiledb_string_view(tiledb_name, &name, &length); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + CHECK(std::string(name, length) == "primes"); + } +} + TEST_CASE( "C API: tiledb_array_schema_set_coords_filter_list argument validation", "[capi][array_schema]") { diff --git a/tiledb/sm/cpp_api/array.h b/tiledb/sm/cpp_api/array.h index 6ef99ad961c..06caa4e45d7 100644 --- a/tiledb/sm/cpp_api/array.h +++ b/tiledb/sm/cpp_api/array.h @@ -699,6 +699,8 @@ class Array { /** * Loads the array schema from an array. + * Options to load additional features are read from the optionally-provided + * `config`. See `tiledb_array_schema_load_with_config`. * * **Example:** * @code{.cpp} @@ -708,12 +710,16 @@ class Array { * * @param ctx The TileDB context. * @param uri The array URI. + * @param config The optional request for additional features. * @return The loaded ArraySchema object. */ - static ArraySchema load_schema(const Context& ctx, const std::string& uri) { + static ArraySchema load_schema( + const Context& ctx, + const std::string& uri, + const Config& config = Config()) { tiledb_array_schema_t* schema; - ctx.handle_error( - tiledb_array_schema_load(ctx.ptr().get(), uri.c_str(), &schema)); + ctx.handle_error(tiledb_array_schema_load_with_config( + ctx.ptr().get(), config.ptr().get(), uri.c_str(), &schema)); return ArraySchema(ctx, schema); } diff --git a/tiledb/sm/cpp_api/array_schema_experimental.h b/tiledb/sm/cpp_api/array_schema_experimental.h index 8da27be3c94..0519dd12c6b 100644 --- a/tiledb/sm/cpp_api/array_schema_experimental.h +++ b/tiledb/sm/cpp_api/array_schema_experimental.h @@ -189,6 +189,31 @@ class ArraySchemaExperimental { ctx.ptr().get(), array_schema.ptr().get(), current_domain.ptr().get())); } + /** + * Retrieve an enumeration from the array schema, if has already been loaded + * (such as via `ArrayExperimental::get_enumeration`). + * + * @param ctx TileDB context. + * @param array_schema Array schema. + * @param enumeration_name The name of the enumeration to retrieve. + */ + static std::optional get_enumeration( + const Context& ctx, + const ArraySchema& array_schema, + const char* enumeration_name) { + tiledb_enumeration_t* enumeration; + ctx.handle_error(tiledb_array_schema_get_enumeration( + ctx.ptr().get(), + array_schema.ptr().get(), + enumeration_name, + &enumeration)); + if (enumeration) { + return Enumeration(ctx, enumeration); + } else { + return std::nullopt; + } + } + /** * Add an enumeration to the array schema. * From c1efcf0a73ca031b33f073e49c758690048acff7 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 24 Oct 2024 12:04:38 -0400 Subject: [PATCH 02/23] Fix comment typo --- tiledb/sm/cpp_api/array_schema_experimental.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tiledb/sm/cpp_api/array_schema_experimental.h b/tiledb/sm/cpp_api/array_schema_experimental.h index 0519dd12c6b..37bf5c69310 100644 --- a/tiledb/sm/cpp_api/array_schema_experimental.h +++ b/tiledb/sm/cpp_api/array_schema_experimental.h @@ -190,8 +190,8 @@ class ArraySchemaExperimental { } /** - * Retrieve an enumeration from the array schema, if has already been loaded - * (such as via `ArrayExperimental::get_enumeration`). + * Retrieve an enumeration from the array schema, if it has already been + * loaded (such as via `ArrayExperimental::get_enumeration`). * * @param ctx TileDB context. * @param array_schema Array schema. From 0a775517048b4b1227c9506477e967cbea527a79 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 24 Oct 2024 12:35:00 -0400 Subject: [PATCH 03/23] Move is_equivalent_enumeration to array_schema_helpers.cc --- test/src/unit-cppapi-enumerations.cc | 17 ++------ test/support/CMakeLists.txt | 1 + test/support/src/array_schema_helpers.cc | 55 ++++++++++++++++++++++++ test/support/src/array_schema_helpers.h | 51 ++++++++++++++++++++++ 4 files changed, 110 insertions(+), 14 deletions(-) create mode 100644 test/support/src/array_schema_helpers.cc create mode 100644 test/support/src/array_schema_helpers.h diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index def8446600f..cae714b104c 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -33,6 +33,7 @@ #include #include +#include "test/support/src/array_schema_helpers.h" #include "test/support/src/vfs_helpers.h" #include "tiledb/api/c_api/array/array_api_internal.h" #include "tiledb/api/c_api/array_schema/array_schema_api_internal.h" @@ -44,18 +45,6 @@ using namespace tiledb; -static bool is_equivalent_enumeration( - const Enumeration& left, const Enumeration& right) { - return left.name() == right.name() && left.type() == right.type() && - left.cell_val_num() == right.cell_val_num() && - left.ordered() == right.ordered() && - std::equal( - left.ptr()->data().begin(), - left.ptr()->data().end(), - right.ptr()->data().begin(), - right.ptr()->data().end()); -} - struct CPPEnumerationFx { CPPEnumerationFx(); ~CPPEnumerationFx() = default; @@ -346,7 +335,7 @@ TEST_CASE_METHOD( auto schema = array.schema(); auto from_schema = ArraySchemaExperimental::get_enumeration( ctx_, schema, "an_enumeration"); - CHECK(is_equivalent_enumeration(from_array, *from_schema)); + CHECK(test::is_equivalent_enumeration(from_array, *from_schema)); } SECTION("array get_enumeration loads into user schema handle") { @@ -367,7 +356,7 @@ TEST_CASE_METHOD( ctx_, schema, "an_enumeration"); REQUIRE(from_schema.has_value()); - CHECK(is_equivalent_enumeration(from_array, *from_schema)); + CHECK(test::is_equivalent_enumeration(from_array, *from_schema)); } SECTION("schema load with rest config does populate enumeration") { diff --git a/test/support/CMakeLists.txt b/test/support/CMakeLists.txt index 8ffc4028043..f6d11f1ac4c 100644 --- a/test/support/CMakeLists.txt +++ b/test/support/CMakeLists.txt @@ -36,6 +36,7 @@ list(APPEND TILEDB_CORE_INCLUDE_DIR "${CMAKE_SOURCE_DIR}/tiledb/sm/c_api") # Gather the test source files set(TILEDB_TEST_SUPPORT_SOURCES + src/array_schema_helpers.cc src/ast_helpers.h src/ast_helpers.cc src/helpers.h diff --git a/test/support/src/array_schema_helpers.cc b/test/support/src/array_schema_helpers.cc new file mode 100644 index 00000000000..dc12d1c399a --- /dev/null +++ b/test/support/src/array_schema_helpers.cc @@ -0,0 +1,55 @@ +/** + * @file array_schema_helpers.cc + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2017-2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file defines some array schema test suite helper functions. + */ + +#include "test/support/src/array_schema_helpers.h" +#include "tiledb/api/c_api/enumeration/enumeration_api_internal.h" +#include "tiledb/sm/array_schema/enumeration.h" +#include "tiledb/sm/cpp_api/tiledb" + +using namespace tiledb; + +namespace tiledb::test { + +bool is_equivalent_enumeration( + const Enumeration& left, const Enumeration& right) { + return left.name() == right.name() && left.type() == right.type() && + left.cell_val_num() == right.cell_val_num() && + left.ordered() == right.ordered() && + std::equal( + left.ptr()->data().begin(), + left.ptr()->data().end(), + right.ptr()->data().begin(), + right.ptr()->data().end()); +} + +} // namespace tiledb::test + diff --git a/test/support/src/array_schema_helpers.h b/test/support/src/array_schema_helpers.h new file mode 100644 index 00000000000..66bf72a7af6 --- /dev/null +++ b/test/support/src/array_schema_helpers.h @@ -0,0 +1,51 @@ +/** + * @file array_schema_helpers.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2017-2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file declares some array schema test suite helper functions. + */ + +#ifndef TILEDB_TEST_ARRAY_SCHEMA_HELPERS_H +#define TILEDB_TEST_ARRAY_SCHEMA_HELPERS_H + +#include "tiledb/sm/cpp_api/tiledb" +#include "tiledb/sm/cpp_api/tiledb_experimental" + +namespace tiledb::test { + +/** + * @return if two enumerations `left` and `right` are equivalent, + * i.e. have the same name, datatype, variants, etc + */ +bool is_equivalent_enumeration( + const tiledb::Enumeration& left, const tiledb::Enumeration& right); + +} // namespace tiledb::test + +#endif + From dc2b5ae86c49ca8f777984873ed47f1c0e6eaf5c Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 24 Oct 2024 12:39:32 -0400 Subject: [PATCH 04/23] Fix missing return code --- tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc index b26f6387c0a..e4673aaedee 100644 --- a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc +++ b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc @@ -401,7 +401,8 @@ TEST_CASE( 0, &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_OK); - tiledb_array_schema_add_enumeration(x.ctx(), x.schema, enumeration); + rc = tiledb_array_schema_add_enumeration(x.ctx(), x.schema, enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_OK); REQUIRE_NOTHROW(tiledb_enumeration_free(&enumeration)); CHECK(enumeration == nullptr); From 5d5c3ba27a5b6ffc91c6f7e980b155d7821a1d0d Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 24 Oct 2024 12:43:02 -0400 Subject: [PATCH 05/23] Add missing rest tag --- test/src/unit-cppapi-enumerations.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index cae714b104c..8b3a18c1d5c 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -317,7 +317,7 @@ TEST_CASE_METHOD( TEST_CASE_METHOD( CPPEnumerationFx, "CPP: Enumerations From Disk - ArraySchema::get_enumeration", - "[enumeration][array-schema-get-enumeration]") { + "[enumeration][array-schema-get-enumeration][rest]") { create_array(); SECTION("default schema load does not populate enumeration") { From d18cb945459376e2a877bfa7b243afb129110ccd Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 24 Oct 2024 12:43:41 -0400 Subject: [PATCH 06/23] Fix formatting --- test/support/src/array_schema_helpers.cc | 1 - test/support/src/array_schema_helpers.h | 1 - 2 files changed, 2 deletions(-) diff --git a/test/support/src/array_schema_helpers.cc b/test/support/src/array_schema_helpers.cc index dc12d1c399a..f9aa9855bc0 100644 --- a/test/support/src/array_schema_helpers.cc +++ b/test/support/src/array_schema_helpers.cc @@ -52,4 +52,3 @@ bool is_equivalent_enumeration( } } // namespace tiledb::test - diff --git a/test/support/src/array_schema_helpers.h b/test/support/src/array_schema_helpers.h index 66bf72a7af6..578c5aefe9b 100644 --- a/test/support/src/array_schema_helpers.h +++ b/test/support/src/array_schema_helpers.h @@ -48,4 +48,3 @@ bool is_equivalent_enumeration( } // namespace tiledb::test #endif - From 0bf188d276d2da0ccd7624309639f491f5586cc9 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 24 Oct 2024 21:32:11 -0400 Subject: [PATCH 07/23] tiledb_array_schema_get_enumeration => tiledb_array_schema_get_enumeration_if_needed --- test/src/unit-cppapi-enumerations.cc | 10 +++++----- tiledb/api/c_api/array_schema/array_schema_api.cc | 7 ++++--- .../c_api/array_schema/array_schema_api_experimental.h | 8 +++++--- .../c_api/array_schema/test/unit_capi_array_schema.cc | 10 +++++----- tiledb/sm/cpp_api/array_schema_experimental.h | 4 ++-- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index 8b3a18c1d5c..bfdb1c9b1d6 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -322,7 +322,7 @@ TEST_CASE_METHOD( SECTION("default schema load does not populate enumeration") { auto schema = Array::load_schema(ctx_, uri_); - auto enmr = ArraySchemaExperimental::get_enumeration( + auto enmr = ArraySchemaExperimental::get_enumeration_if_loaded( ctx_, schema, "an_enumeration"); CHECK(!enmr.has_value()); } @@ -333,7 +333,7 @@ TEST_CASE_METHOD( ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration"); auto schema = array.schema(); - auto from_schema = ArraySchemaExperimental::get_enumeration( + auto from_schema = ArraySchemaExperimental::get_enumeration_if_loaded( ctx_, schema, "an_enumeration"); CHECK(test::is_equivalent_enumeration(from_array, *from_schema)); } @@ -343,7 +343,7 @@ TEST_CASE_METHOD( auto schema = array.schema(); // the enumeration is not populated yet - REQUIRE(!ArraySchemaExperimental::get_enumeration( + REQUIRE(!ArraySchemaExperimental::get_enumeration_if_loaded( ctx_, schema, "an_enumeration") .has_value()); @@ -352,7 +352,7 @@ TEST_CASE_METHOD( ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration"); // and once it is loaded, it is set on the schema - auto from_schema = ArraySchemaExperimental::get_enumeration( + auto from_schema = ArraySchemaExperimental::get_enumeration_if_loaded( ctx_, schema, "an_enumeration"); REQUIRE(from_schema.has_value()); @@ -364,7 +364,7 @@ TEST_CASE_METHOD( config["rest.load_enumerations_on_array_open"] = "true"; auto schema = Array::load_schema(ctx_, uri_, config); - auto enmr = ArraySchemaExperimental::get_enumeration( + auto enmr = ArraySchemaExperimental::get_enumeration_if_loaded( ctx_, schema, "an_enumeration"); REQUIRE(enmr.has_value()); REQUIRE(enmr->ptr() != nullptr); diff --git a/tiledb/api/c_api/array_schema/array_schema_api.cc b/tiledb/api/c_api/array_schema/array_schema_api.cc index 91df3da4d1e..24d441e1b86 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api.cc +++ b/tiledb/api/c_api/array_schema/array_schema_api.cc @@ -181,7 +181,7 @@ capi_return_t tiledb_array_schema_timestamp_range( return TILEDB_OK; } -capi_return_t tiledb_array_schema_get_enumeration( +capi_return_t tiledb_array_schema_get_enumeration_if_loaded( tiledb_array_schema_t* array_schema, const char* enumeration_name, tiledb_enumeration_t** enumeration) { @@ -561,12 +561,13 @@ CAPI_INTERFACE( } CAPI_INTERFACE( - array_schema_get_enumeration, + array_schema_get_enumeration_if_loaded, tiledb_ctx_t* ctx, tiledb_array_schema_t* array_schema, const char* enumeration_name, tiledb_enumeration_t** enumeration) { - return api_entry_context( + return api_entry_context< + tiledb::api::tiledb_array_schema_get_enumeration_if_loaded>( ctx, array_schema, enumeration_name, enumeration); } diff --git a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h index cc8500fd3a9..2e04521bc78 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h @@ -68,7 +68,8 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( uint64_t* hi) TILEDB_NOEXCEPT; /** - * Retrieves an already-loaded enumeration given its name. + * Retrieves an already-loaded enumeration given its name, + * if it has already been loaded into memory. * * If the enumeration has already been loaded, * such as via `tiledb_array_get_enumeration`, @@ -81,7 +82,8 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( * * @code{.c} * tiledb_enumeration_t* enmr; - * tiledb_array_schema_get_enumeration(ctx, array_schema, "states", &enmr); + * tiledb_array_schema_get_enumeration_if_loaded(ctx, + * array_schema, "states", &enmr); * tiledb_attribute_free(&enmr); * @endcode * @@ -91,7 +93,7 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( * @param[out] enmr The enumeration object to retrieve. * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. */ -TILEDB_EXPORT capi_return_t tiledb_array_schema_get_enumeration( +TILEDB_EXPORT capi_return_t tiledb_array_schema_get_enumeration_if_loaded( tiledb_ctx_t* ctx, tiledb_array_schema_t* array_schema, const char* name, diff --git a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc index e4673aaedee..41c05a836d2 100644 --- a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc +++ b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc @@ -368,22 +368,22 @@ TEST_CASE( ordinary_array_schema x{}; tiledb_enumeration_t* enumeration; SECTION("null context") { - rc = tiledb_array_schema_get_enumeration( + rc = tiledb_array_schema_get_enumeration_if_loaded( nullptr, x.schema, "primes", &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_INVALID_CONTEXT); } SECTION("null schema") { - rc = tiledb_array_schema_get_enumeration( + rc = tiledb_array_schema_get_enumeration_if_loaded( x.ctx(), nullptr, "primes", &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_ERR); } SECTION("null name") { - rc = tiledb_array_schema_get_enumeration( + rc = tiledb_array_schema_get_enumeration_if_loaded( x.ctx(), x.schema, nullptr, &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_ERR); } SECTION("null enumeration") { - rc = tiledb_array_schema_get_enumeration( + rc = tiledb_array_schema_get_enumeration_if_loaded( x.ctx(), x.schema, "primes", nullptr); REQUIRE(tiledb_status(rc) == TILEDB_ERR); } @@ -406,7 +406,7 @@ TEST_CASE( REQUIRE_NOTHROW(tiledb_enumeration_free(&enumeration)); CHECK(enumeration == nullptr); - rc = tiledb_array_schema_get_enumeration( + rc = tiledb_array_schema_get_enumeration_if_loaded( x.ctx(), x.schema, "primes", &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_OK); REQUIRE(enumeration != nullptr); diff --git a/tiledb/sm/cpp_api/array_schema_experimental.h b/tiledb/sm/cpp_api/array_schema_experimental.h index 37bf5c69310..152a72a01f6 100644 --- a/tiledb/sm/cpp_api/array_schema_experimental.h +++ b/tiledb/sm/cpp_api/array_schema_experimental.h @@ -197,12 +197,12 @@ class ArraySchemaExperimental { * @param array_schema Array schema. * @param enumeration_name The name of the enumeration to retrieve. */ - static std::optional get_enumeration( + static std::optional get_enumeration_if_loaded( const Context& ctx, const ArraySchema& array_schema, const char* enumeration_name) { tiledb_enumeration_t* enumeration; - ctx.handle_error(tiledb_array_schema_get_enumeration( + ctx.handle_error(tiledb_array_schema_get_enumeration_if_loaded( ctx.ptr().get(), array_schema.ptr().get(), enumeration_name, From 754f239e45aded90878acbcb3434a908d67e2eed Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 24 Oct 2024 21:39:44 -0400 Subject: [PATCH 08/23] Undo default argument, add ArraySchema::load_schema_with_config --- test/src/unit-cppapi-enumerations.cc | 2 +- tiledb/sm/cpp_api/array.h | 32 ++++++++++++++++++++++------ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index bfdb1c9b1d6..814699429a7 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -363,7 +363,7 @@ TEST_CASE_METHOD( Config config; config["rest.load_enumerations_on_array_open"] = "true"; - auto schema = Array::load_schema(ctx_, uri_, config); + auto schema = Array::load_schema_with_config(ctx_, config, uri_); auto enmr = ArraySchemaExperimental::get_enumeration_if_loaded( ctx_, schema, "an_enumeration"); REQUIRE(enmr.has_value()); diff --git a/tiledb/sm/cpp_api/array.h b/tiledb/sm/cpp_api/array.h index 06caa4e45d7..1b21340d9a5 100644 --- a/tiledb/sm/cpp_api/array.h +++ b/tiledb/sm/cpp_api/array.h @@ -697,6 +697,26 @@ class Array { create(schema.context(), uri, schema); } + /** + * Loads the array schema from an array. + * + * **Example:** + * @code{.cpp} + * auto schema = tiledb::Array::load_schema(ctx, + * "s3://bucket-name/array-name"); + * @endcode + * + * @param ctx The TileDB context. + * @param uri The array URI. + * @return The loaded ArraySchema object. + */ + static ArraySchema load_schema(const Context& ctx, const std::string& uri) { + tiledb_array_schema_t* schema; + ctx.handle_error( + tiledb_array_schema_load(ctx.ptr().get(), uri.c_str(), &schema)); + return ArraySchema(ctx, schema); + } + /** * Loads the array schema from an array. * Options to load additional features are read from the optionally-provided @@ -704,19 +724,19 @@ class Array { * * **Example:** * @code{.cpp} - * auto schema = tiledb::Array::load_schema(ctx, + * tiledb::Config config; + * config["rest.load_enumerations_on_array_open"] = "true"; + * auto schema = tiledb::Array::load_schema_with_config(ctx, config, * "s3://bucket-name/array-name"); * @endcode * * @param ctx The TileDB context. + * @param config The request for additional features. * @param uri The array URI. - * @param config The optional request for additional features. * @return The loaded ArraySchema object. */ - static ArraySchema load_schema( - const Context& ctx, - const std::string& uri, - const Config& config = Config()) { + static ArraySchema load_schema_with_config( + const Context& ctx, const Config& config, const std::string& uri) { tiledb_array_schema_t* schema; ctx.handle_error(tiledb_array_schema_load_with_config( ctx.ptr().get(), config.ptr().get(), uri.c_str(), &schema)); From 2c2de31647cee927c3917af3100d3ae51019c6a1 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Mon, 28 Oct 2024 22:47:12 -0400 Subject: [PATCH 09/23] Split ArraySchema::get_enumeration to look up from enumeration name or attribute name, and also use context to do the I/O --- test/src/unit-cppapi-enumerations.cc | 110 +++++++++++------- tiledb/api/c_api/array_schema/CMakeLists.txt | 1 + .../c_api/array_schema/array_schema_api.cc | 58 +++++++-- .../array_schema_api_experimental.h | 43 +++++-- .../array_schema/array_schema_api_internal.h | 5 + .../test/unit_capi_array_schema.cc | 92 ++++++++++++++- tiledb/sm/array/array_directory.h | 24 ++-- tiledb/sm/array_schema/array_schema.cc | 32 +++++ tiledb/sm/array_schema/array_schema.h | 9 ++ tiledb/sm/cpp_api/array_schema_experimental.h | 43 +++++-- 10 files changed, 330 insertions(+), 87 deletions(-) diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index 814699429a7..26df4bd2546 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -316,65 +316,97 @@ TEST_CASE_METHOD( TEST_CASE_METHOD( CPPEnumerationFx, - "CPP: Enumerations From Disk - ArraySchema::get_enumeration", - "[enumeration][array-schema-get-enumeration][rest]") { + "CPP: Enumerations From Disk - ArraySchema::get_enumeration_from_name", + "[enumeration][array-schema-get-enumeration-from-name][rest]") { create_array(); - SECTION("default schema load does not populate enumeration") { + const std::string enmr_name = "an_enumeration"; + + std::optional expect_enumeration; + { + auto array = tiledb::Array(ctx_, uri_, TILEDB_READ); + expect_enumeration = + ArrayExperimental::get_enumeration(ctx_, array, enmr_name); + } + + SECTION("default schema load retrieves enumeration on request only") { auto schema = Array::load_schema(ctx_, uri_); - auto enmr = ArraySchemaExperimental::get_enumeration_if_loaded( - ctx_, schema, "an_enumeration"); - CHECK(!enmr.has_value()); + + CHECK(!schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + + auto actual_enumeration = + ArraySchemaExperimental::get_enumeration_from_name( + ctx_, schema, enmr_name); + CHECK(schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + CHECK(test::is_equivalent_enumeration( + *expect_enumeration, actual_enumeration)); } - SECTION("array get_enumeration loads enumeration and schema shares") { - auto array = tiledb::Array(ctx_, uri_, TILEDB_READ); - auto from_array = - ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration"); + SECTION("schema load with rest config retrieves enumeration eagerly") { + Config config; + config["rest.load_enumerations_on_array_open"] = "true"; - auto schema = array.schema(); - auto from_schema = ArraySchemaExperimental::get_enumeration_if_loaded( - ctx_, schema, "an_enumeration"); - CHECK(test::is_equivalent_enumeration(from_array, *from_schema)); + auto schema = Array::load_schema_with_config(ctx_, config, uri_); + CHECK(schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + + // requesting it should do no I/O (we did it already), + // unclear how to check that + auto actual_enumeration = + ArraySchemaExperimental::get_enumeration_from_name( + ctx_, schema, enmr_name); + CHECK(schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + CHECK(test::is_equivalent_enumeration( + *expect_enumeration, actual_enumeration)); } +} - SECTION("array get_enumeration loads into user schema handle") { - auto array = tiledb::Array(ctx_, uri_, TILEDB_READ); - auto schema = array.schema(); +TEST_CASE_METHOD( + CPPEnumerationFx, + "CPP: Enumerations From Disk - " + "ArraySchema::get_enumeration_from_attribute_name", + "[enumeration][array-schema-get-enumeration-from-attribute-name][rest]") { + create_array(); - // the enumeration is not populated yet - REQUIRE(!ArraySchemaExperimental::get_enumeration_if_loaded( - ctx_, schema, "an_enumeration") - .has_value()); + const std::string enmr_name = "an_enumeration"; + const std::string attr_name = "attr1"; - // array method actually loads it - auto from_array = - ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration"); + std::optional expect_enumeration; + { + auto array = tiledb::Array(ctx_, uri_, TILEDB_READ); + expect_enumeration = + ArrayExperimental::get_enumeration(ctx_, array, enmr_name); + } - // and once it is loaded, it is set on the schema - auto from_schema = ArraySchemaExperimental::get_enumeration_if_loaded( - ctx_, schema, "an_enumeration"); - REQUIRE(from_schema.has_value()); + SECTION("default schema load retrieves enumeration on request only") { + auto schema = Array::load_schema(ctx_, uri_); - CHECK(test::is_equivalent_enumeration(from_array, *from_schema)); + CHECK(!schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + + auto actual_enumeration = + ArraySchemaExperimental::get_enumeration_from_attribute_name( + ctx_, schema, attr_name); + CHECK(schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + CHECK(test::is_equivalent_enumeration( + *expect_enumeration, actual_enumeration)); } - SECTION("schema load with rest config does populate enumeration") { + SECTION("schema load with rest config retrieves enumeration eagerly") { Config config; config["rest.load_enumerations_on_array_open"] = "true"; auto schema = Array::load_schema_with_config(ctx_, config, uri_); - auto enmr = ArraySchemaExperimental::get_enumeration_if_loaded( - ctx_, schema, "an_enumeration"); - REQUIRE(enmr.has_value()); - REQUIRE(enmr->ptr() != nullptr); - REQUIRE(enmr->name() == "an_enumeration"); - REQUIRE(enmr->type() == TILEDB_STRING_ASCII); - REQUIRE(enmr->cell_val_num() == TILEDB_VAR_NUM); - REQUIRE(enmr->ordered() == false); + CHECK(schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + + // requesting it should do no I/O (we did it already), + // unclear how to check that + auto actual_enumeration = + ArraySchemaExperimental::get_enumeration_from_attribute_name( + ctx_, schema, attr_name); + CHECK(schema.ptr()->array_schema()->is_enumeration_loaded(enmr_name)); + CHECK(test::is_equivalent_enumeration( + *expect_enumeration, actual_enumeration)); } } - TEST_CASE_METHOD( CPPEnumerationFx, "CPP: Array::load_all_enumerations", diff --git a/tiledb/api/c_api/array_schema/CMakeLists.txt b/tiledb/api/c_api/array_schema/CMakeLists.txt index 4beb4b49d9f..39c6d30f4de 100644 --- a/tiledb/api/c_api/array_schema/CMakeLists.txt +++ b/tiledb/api/c_api/array_schema/CMakeLists.txt @@ -36,6 +36,7 @@ commence(object_library capi_array_schema_stub) this_target_sources(${SOURCES}) this_target_link_libraries(export) this_target_object_libraries(array_schema) + this_target_object_libraries(array) this_target_object_libraries(capi_context_stub) conclude(object_library) diff --git a/tiledb/api/c_api/array_schema/array_schema_api.cc b/tiledb/api/c_api/array_schema/array_schema_api.cc index 24d441e1b86..1a000aa6790 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api.cc +++ b/tiledb/api/c_api/array_schema/array_schema_api.cc @@ -34,6 +34,7 @@ #include "array_schema_api_experimental.h" #include "array_schema_api_internal.h" +#include "tiledb/api/c_api/attribute/attribute_api_external_experimental.h" #include "tiledb/api/c_api/attribute/attribute_api_internal.h" #include "tiledb/api/c_api/context/context_api_internal.h" #include "tiledb/api/c_api/current_domain/current_domain_api_external_experimental.h" @@ -181,7 +182,8 @@ capi_return_t tiledb_array_schema_timestamp_range( return TILEDB_OK; } -capi_return_t tiledb_array_schema_get_enumeration_if_loaded( +capi_return_t tiledb_array_schema_get_enumeration_from_name( + tiledb_ctx_t* ctx, tiledb_array_schema_t* array_schema, const char* enumeration_name, tiledb_enumeration_t** enumeration) { @@ -192,15 +194,42 @@ capi_return_t tiledb_array_schema_get_enumeration_if_loaded( throw CAPIException("'enumeration_name' must not be null"); } + array_schema->load_enumeration(ctx, enumeration_name); + auto ptr = array_schema->get_enumeration(enumeration_name); - if (ptr) { - *enumeration = tiledb_enumeration_handle_t::make_handle(ptr); - } else { - *enumeration = nullptr; - } + *enumeration = tiledb_enumeration_handle_t::make_handle(ptr); + return TILEDB_OK; } +capi_return_t tiledb_array_schema_get_enumeration_from_attribute_name( + tiledb_ctx_t* ctx, + tiledb_array_schema_t* array_schema, + const char* attribute_name, + tiledb_enumeration_t** enumeration) { + ensure_array_schema_is_valid(array_schema); + ensure_output_pointer_is_valid(enumeration); + + tiledb_attribute_t* attribute; + capi_return_t getattr = tiledb_array_schema_get_attribute_from_name( + ctx, array_schema, attribute_name, &attribute); + if (tiledb_status(getattr) != TILEDB_OK) { + return getattr; + } + + tiledb_string_t* enumeration_name_inner; + capi_return_t getenmr = tiledb_attribute_get_enumeration_name( + ctx, attribute, &enumeration_name_inner); + if (tiledb_status(getenmr) != TILEDB_OK) { + return getenmr; + } + + std::string enumeration_name(enumeration_name_inner->view()); + return api_entry_with_context< + tiledb::api::tiledb_array_schema_get_enumeration_from_name>( + ctx, array_schema, enumeration_name.c_str(), enumeration); +} + capi_return_t tiledb_array_schema_add_enumeration( tiledb_array_schema_t* array_schema, tiledb_enumeration_t* enumeration) { ensure_array_schema_is_valid(array_schema); @@ -561,16 +590,27 @@ CAPI_INTERFACE( } CAPI_INTERFACE( - array_schema_get_enumeration_if_loaded, + array_schema_get_enumeration_from_name, tiledb_ctx_t* ctx, tiledb_array_schema_t* array_schema, const char* enumeration_name, tiledb_enumeration_t** enumeration) { - return api_entry_context< - tiledb::api::tiledb_array_schema_get_enumeration_if_loaded>( + return api_entry_with_context< + tiledb::api::tiledb_array_schema_get_enumeration_from_name>( ctx, array_schema, enumeration_name, enumeration); } +CAPI_INTERFACE( + array_schema_get_enumeration_from_attribute_name, + tiledb_ctx_t* ctx, + tiledb_array_schema_t* array_schema, + const char* attribute_name, + tiledb_enumeration_t** enumeration) { + return api_entry_with_context< + tiledb::api::tiledb_array_schema_get_enumeration_from_attribute_name>( + ctx, array_schema, attribute_name, enumeration); +} + CAPI_INTERFACE( array_schema_add_enumeration, tiledb_ctx_t* ctx, diff --git a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h index 2e04521bc78..2f95219a95c 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h @@ -68,13 +68,7 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( uint64_t* hi) TILEDB_NOEXCEPT; /** - * Retrieves an already-loaded enumeration given its name, - * if it has already been loaded into memory. - * - * If the enumeration has already been loaded, - * such as via `tiledb_array_get_enumeration`, - * then this returns a pointer to the enumeration. - * Otherwise this returns `TILEDB_OK` and sets `enumeration` to NULL. + * Retrieves an enumeration from an array schema using the enumeration name, * * **Example:** * @@ -82,7 +76,7 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( * * @code{.c} * tiledb_enumeration_t* enmr; - * tiledb_array_schema_get_enumeration_if_loaded(ctx, + * tiledb_array_schema_get_enumeration_from_name(ctx, * array_schema, "states", &enmr); * tiledb_attribute_free(&enmr); * @endcode @@ -93,10 +87,39 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( * @param[out] enmr The enumeration object to retrieve. * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. */ -TILEDB_EXPORT capi_return_t tiledb_array_schema_get_enumeration_if_loaded( +TILEDB_EXPORT capi_return_t tiledb_array_schema_get_enumeration_from_name( + tiledb_ctx_t* ctx, + tiledb_array_schema_t* array_schema, + const char* enumeration_name, + tiledb_enumeration_t** enumeration) TILEDB_NOEXCEPT; + +/** + * Retrieves an enumeration from an array schema from the attribute with the + * given name. + * + * **Example:** + * + * The following retrieves the enumeration for the attribute named "states" in + * the schema. + * + * @code{.c} + * tiledb_enumeration_t* enmr; + * tiledb_array_schema_get_enumeration_from_attribute_name(ctx, + * array_schema, "states", &enmr); + * tiledb_attribute_free(&enmr); + * @endcode + * + * @param[in] ctx The TileDB context. + * @param[in] array_schema The array schema. + * @param[in] name The name of the attribute whose to retrieve. + * @param[out] enmr The enumeration object to retrieve. + * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. + */ +TILEDB_EXPORT capi_return_t +tiledb_array_schema_get_enumeration_from_attribute_name( tiledb_ctx_t* ctx, tiledb_array_schema_t* array_schema, - const char* name, + const char* attribute_name, tiledb_enumeration_t** enumeration) TILEDB_NOEXCEPT; /** diff --git a/tiledb/api/c_api/array_schema/array_schema_api_internal.h b/tiledb/api/c_api/array_schema/array_schema_api_internal.h index 41f61f129f9..a2cbd5b0898 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_internal.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_internal.h @@ -35,6 +35,7 @@ #include "array_schema_api_external.h" +#include "tiledb/api/c_api/context/context_api_internal.h" #include "tiledb/api/c_api_support/handle/handle.h" #include "tiledb/common/common.h" #include "tiledb/sm/array_schema/array_schema.h" @@ -90,6 +91,10 @@ struct tiledb_array_schema_handle_t dim_id, name, label_order, label_type, check_name); } + void load_enumeration(tiledb_ctx_t* ctx, const char* name) { + return array_schema_->load_enumeration(ctx->context(), name); + } + shared_ptr get_enumeration( const char* name) const { return array_schema_->get_enumeration(name); diff --git a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc index 41c05a836d2..02c789f42e3 100644 --- a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc +++ b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc @@ -32,6 +32,7 @@ #include #include "../../../c_api_test_support/testsupport_capi_array_schema.h" #include "../../../c_api_test_support/testsupport_capi_context.h" +#include "../../attribute/attribute_api_external_experimental.h" #include "../array_schema_api_experimental.h" #include "../array_schema_api_external.h" #include "../array_schema_api_internal.h" @@ -362,28 +363,28 @@ TEST_CASE( } TEST_CASE( - "C API: tiledb_array_schema_get_enumeration argument validation", + "C API: tiledb_array_schema_get_enumeration_from_name argument validation", "[capi][array_schema]") { capi_return_t rc; ordinary_array_schema x{}; tiledb_enumeration_t* enumeration; SECTION("null context") { - rc = tiledb_array_schema_get_enumeration_if_loaded( + rc = tiledb_array_schema_get_enumeration_from_name( nullptr, x.schema, "primes", &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_INVALID_CONTEXT); } SECTION("null schema") { - rc = tiledb_array_schema_get_enumeration_if_loaded( + rc = tiledb_array_schema_get_enumeration_from_name( x.ctx(), nullptr, "primes", &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_ERR); } SECTION("null name") { - rc = tiledb_array_schema_get_enumeration_if_loaded( + rc = tiledb_array_schema_get_enumeration_from_name( x.ctx(), x.schema, nullptr, &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_ERR); } SECTION("null enumeration") { - rc = tiledb_array_schema_get_enumeration_if_loaded( + rc = tiledb_array_schema_get_enumeration_from_name( x.ctx(), x.schema, "primes", nullptr); REQUIRE(tiledb_status(rc) == TILEDB_ERR); } @@ -406,7 +407,7 @@ TEST_CASE( REQUIRE_NOTHROW(tiledb_enumeration_free(&enumeration)); CHECK(enumeration == nullptr); - rc = tiledb_array_schema_get_enumeration_if_loaded( + rc = tiledb_array_schema_get_enumeration_from_name( x.ctx(), x.schema, "primes", &enumeration); REQUIRE(tiledb_status(rc) == TILEDB_OK); REQUIRE(enumeration != nullptr); @@ -424,6 +425,85 @@ TEST_CASE( } } +TEST_CASE( + "C API: tiledb_array_schema_get_enumeration_from_attribute_name argument " + "validation", + "[capi][array_schema]") { + capi_return_t rc; + ordinary_array_schema_with_attr x{}; + tiledb_enumeration_t* enumeration; + SECTION("null context") { + rc = tiledb_array_schema_get_enumeration_from_attribute_name( + nullptr, x.schema, "a", &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_INVALID_CONTEXT); + } + SECTION("null schema") { + rc = tiledb_array_schema_get_enumeration_from_attribute_name( + x.ctx(), nullptr, "a", &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } + SECTION("null name") { + rc = tiledb_array_schema_get_enumeration_from_attribute_name( + x.ctx(), x.schema, nullptr, &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } + SECTION("null enumeration") { + rc = tiledb_array_schema_get_enumeration_from_attribute_name( + x.ctx(), x.schema, "a", nullptr); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } + SECTION("invalid attribute") { + rc = tiledb_array_schema_get_enumeration_from_attribute_name( + x.ctx(), x.schema, "foobar", nullptr); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } + SECTION("success") { + // create and add enumeration to schema + int32_t values[5] = {2, 3, 5, 7, 11}; + rc = tiledb_enumeration_alloc( + x.ctx(), + "primes", + TILEDB_UINT32, + 1, + 0, + values, + sizeof(uint32_t) * 5, + nullptr, + 0, + &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + rc = tiledb_array_schema_add_enumeration(x.ctx(), x.schema, enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + REQUIRE_NOTHROW(tiledb_enumeration_free(&enumeration)); + CHECK(enumeration == nullptr); + + // add enumeration to the attribute + tiledb_attribute_t* attribute; + rc = tiledb_array_schema_get_attribute_from_name( + x.ctx(), x.schema, "a", &attribute); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + rc = tiledb_attribute_set_enumeration_name(x.ctx(), attribute, "primes"); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + + // then retrieve the enumeration using attribute name + rc = tiledb_array_schema_get_enumeration_from_attribute_name( + x.ctx(), x.schema, "a", &enumeration); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + REQUIRE(enumeration != nullptr); + + tiledb_string_t* tiledb_name(nullptr); + rc = tiledb_enumeration_get_name(x.ctx(), enumeration, &tiledb_name); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + REQUIRE(tiledb_name != nullptr); + + const char* name; + size_t length; + rc = tiledb_string_view(tiledb_name, &name, &length); + REQUIRE(tiledb_status(rc) == TILEDB_OK); + CHECK(std::string(name, length) == "primes"); + } +} + TEST_CASE( "C API: tiledb_array_schema_set_coords_filter_list argument validation", "[capi][array_schema]") { diff --git a/tiledb/sm/array/array_directory.h b/tiledb/sm/array/array_directory.h index 3016109a8b8..03c13646ec5 100644 --- a/tiledb/sm/array/array_directory.h +++ b/tiledb/sm/array/array_directory.h @@ -399,6 +399,18 @@ class ArrayDirectory { const EncryptionKey& encryption_key, shared_ptr memory_tracker) const; + /** + * Load an enumeration from the given path. + * + * @param enumeration_path The enumeration path to load. + * @param encryption_key The encryption key to use. + * @return shared_ptr The loaded enumeration. + */ + shared_ptr load_enumeration( + const std::string& enumeration_path, + const EncryptionKey& encryption_key, + shared_ptr memory_tracker) const; + /** Returns the array URI. */ const URI& uri() const; @@ -819,18 +831,6 @@ class ArrayDirectory { * @return True if supported, false otherwise */ bool consolidation_with_timestamps_supported(const URI& uri) const; - - /** - * Load an enumeration from the given path. - * - * @param enumeration_path The enumeration path to load. - * @param encryption_key The encryption key to use. - * @return shared_ptr The loaded enumeration. - */ - shared_ptr load_enumeration( - const std::string& enumeration_path, - const EncryptionKey& encryption_key, - shared_ptr memory_tracker) const; }; } // namespace tiledb::sm diff --git a/tiledb/sm/array_schema/array_schema.cc b/tiledb/sm/array_schema/array_schema.cc index 8413112b055..474e29dd8f6 100644 --- a/tiledb/sm/array_schema/array_schema.cc +++ b/tiledb/sm/array_schema/array_schema.cc @@ -36,6 +36,7 @@ #include "tiledb/common/heap_memory.h" #include "tiledb/common/logger.h" #include "tiledb/common/memory_tracker.h" +#include "tiledb/sm/array/array_directory.h" #include "tiledb/sm/array_schema/attribute.h" #include "tiledb/sm/array_schema/current_domain.h" #include "tiledb/sm/array_schema/dimension.h" @@ -43,10 +44,12 @@ #include "tiledb/sm/array_schema/domain.h" #include "tiledb/sm/array_schema/enumeration.h" #include "tiledb/sm/buffer/buffer.h" +#include "tiledb/sm/crypto/encryption_key.h" #include "tiledb/sm/enums/array_type.h" #include "tiledb/sm/enums/compressor.h" #include "tiledb/sm/enums/data_order.h" #include "tiledb/sm/enums/datatype.h" +#include "tiledb/sm/enums/encryption_type.h" #include "tiledb/sm/enums/filter_type.h" #include "tiledb/sm/enums/layout.h" #include "tiledb/sm/filter/compression_filter.h" @@ -54,6 +57,7 @@ #include "tiledb/sm/fragment/fragment_identifier.h" #include "tiledb/sm/misc/hilbert.h" #include "tiledb/sm/misc/tdb_time.h" +#include "tiledb/sm/storage_manager/context.h" #include "tiledb/storage_format/uri/generate_uri.h" #include "tiledb/type/apply_with_type.h" @@ -1128,6 +1132,34 @@ const std::string& ArraySchema::get_enumeration_path_name( return iter->second; } +void ArraySchema::load_enumeration(Context& ctx, const std::string& enmr_name) { + if (is_enumeration_loaded(enmr_name)) { + return; + } + + auto& path = get_enumeration_path_name(enmr_name); + + // Create key + tiledb::sm::EncryptionKey key; + throw_if_not_ok( + key.set_key(tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0)); + + // Load URIs from the array directory + optional array_dir; + array_dir.emplace( + ctx.resources(), + array_uri(), + 0, + UINT64_MAX, + tiledb::sm::ArrayDirectoryMode::SCHEMA_ONLY); + + auto tracker = ctx.resources().ephemeral_memory_tracker(); + auto enumeration = array_dir->load_enumeration(path, key, tracker); + + enumeration_map_[enumeration->name()] = enumeration; + enumeration_path_map_[enumeration->name()] = enumeration->path_name(); +} + void ArraySchema::drop_enumeration(const std::string& enmr_name) { std::lock_guard lock(mtx_); diff --git a/tiledb/sm/array_schema/array_schema.h b/tiledb/sm/array_schema/array_schema.h index 8ca8daae80d..a757cd46c66 100644 --- a/tiledb/sm/array_schema/array_schema.h +++ b/tiledb/sm/array_schema/array_schema.h @@ -51,6 +51,7 @@ namespace tiledb::sm { class Attribute; class Buffer; class ConstBuffer; +class Context; class Dimension; class DimensionLabel; class Domain; @@ -454,6 +455,14 @@ class ArraySchema { shared_ptr get_enumeration( const std::string& enmr_name) const; + /** + * Load an enumeration by name for subsequent retrieval. + * Throws if the enumeration is unknown. + * + * @param enmr_name The name of the Enumeration. + */ + void load_enumeration(Context& context, const std::string& enmr_name); + /** * Get an Enumeration's object name. Throws if the enumeration is unknown. * diff --git a/tiledb/sm/cpp_api/array_schema_experimental.h b/tiledb/sm/cpp_api/array_schema_experimental.h index 152a72a01f6..a1ef94bd7fb 100644 --- a/tiledb/sm/cpp_api/array_schema_experimental.h +++ b/tiledb/sm/cpp_api/array_schema_experimental.h @@ -190,28 +190,49 @@ class ArraySchemaExperimental { } /** - * Retrieve an enumeration from the array schema, if it has already been - * loaded (such as via `ArrayExperimental::get_enumeration`). + * Retrieve an enumeration from the array schema using the enumeration name. * * @param ctx TileDB context. * @param array_schema Array schema. * @param enumeration_name The name of the enumeration to retrieve. + * + * @return the loaded enumeration. */ - static std::optional get_enumeration_if_loaded( + static Enumeration get_enumeration_from_name( const Context& ctx, const ArraySchema& array_schema, - const char* enumeration_name) { + const std::string& enumeration_name) { tiledb_enumeration_t* enumeration; - ctx.handle_error(tiledb_array_schema_get_enumeration_if_loaded( + ctx.handle_error(tiledb_array_schema_get_enumeration_from_name( ctx.ptr().get(), array_schema.ptr().get(), - enumeration_name, + enumeration_name.c_str(), &enumeration)); - if (enumeration) { - return Enumeration(ctx, enumeration); - } else { - return std::nullopt; - } + return Enumeration(ctx, enumeration); + } + + /** + * Retrieve an enumeration from the array schema from the attribute + * with the given name. + * + * @param ctx TileDB context. + * @param array_schema Array schema. + * @param attribute_name The name of the attribute whose enumeration to + * retrieve. + * + * @return the loaded enumeration. + */ + static Enumeration get_enumeration_from_attribute_name( + const Context& ctx, + const ArraySchema& array_schema, + const std::string& attribute_name) { + tiledb_enumeration_t* enumeration; + ctx.handle_error(tiledb_array_schema_get_enumeration_from_attribute_name( + ctx.ptr().get(), + array_schema.ptr().get(), + attribute_name.c_str(), + &enumeration)); + return Enumeration(ctx, enumeration); } /** From adfcfea42d9491c72ae638553ae1f780cecf4895 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Tue, 29 Oct 2024 09:58:21 -0400 Subject: [PATCH 10/23] ArraySchema::load_enumeration branch for remote array case --- tiledb/sm/array/array.cc | 6 ++- tiledb/sm/array_schema/array_schema.cc | 63 ++++++++++++++++++-------- tiledb/sm/array_schema/array_schema.h | 5 +- tiledb/sm/rest/rest_client.h | 3 +- tiledb/sm/rest/rest_client_remote.cc | 12 ++--- tiledb/sm/rest/rest_client_remote.h | 6 ++- tiledb/sm/serialization/enumeration.cc | 10 ++-- tiledb/sm/serialization/enumeration.h | 2 +- 8 files changed, 69 insertions(+), 38 deletions(-) diff --git a/tiledb/sm/array/array.cc b/tiledb/sm/array/array.cc index f7a708fd2de..c4bd8814fa5 100644 --- a/tiledb/sm/array/array.cc +++ b/tiledb/sm/array/array.cc @@ -827,7 +827,8 @@ Array::get_enumerations_all_schemas() { array_uri_, array_dir_timestamp_start_, array_dir_timestamp_end_, - this, + config_, + array_schema_latest(), {}, memory_tracker_); @@ -908,7 +909,8 @@ std::vector> Array::get_enumerations( array_uri_, array_dir_timestamp_start_, array_dir_timestamp_end_, - this, + config_, + array_schema_latest(), names_to_load, memory_tracker_)[array_schema_latest().name()]; } else { diff --git a/tiledb/sm/array_schema/array_schema.cc b/tiledb/sm/array_schema/array_schema.cc index 474e29dd8f6..a22cbd55548 100644 --- a/tiledb/sm/array_schema/array_schema.cc +++ b/tiledb/sm/array_schema/array_schema.cc @@ -57,6 +57,7 @@ #include "tiledb/sm/fragment/fragment_identifier.h" #include "tiledb/sm/misc/hilbert.h" #include "tiledb/sm/misc/tdb_time.h" +#include "tiledb/sm/rest/rest_client.h" #include "tiledb/sm/storage_manager/context.h" #include "tiledb/storage_format/uri/generate_uri.h" #include "tiledb/type/apply_with_type.h" @@ -1135,29 +1136,51 @@ const std::string& ArraySchema::get_enumeration_path_name( void ArraySchema::load_enumeration(Context& ctx, const std::string& enmr_name) { if (is_enumeration_loaded(enmr_name)) { return; - } + } else if (array_uri().is_tiledb()) { + auto rest_client = ctx.resources().rest_client(); + if (rest_client == nullptr) { + throw ArraySchemaException( + "Error loading enumerations; Remote array schema with no REST " + "client."); + } + + auto ret = rest_client->post_enumerations_from_rest( + array_uri(), + timestamp_start(), + timestamp_end(), + ctx.resources().config(), + *this, + {enmr_name}, + memory_tracker_); - auto& path = get_enumeration_path_name(enmr_name); + // response is a map {schema: [enumerations]} + // we should be the only schema, and expect only one enumeration + for (auto enumeration : ret.begin()->second) { + enumeration_map_[enumeration->name()] = enumeration; + enumeration_path_map_[enumeration->name()] = enumeration->path_name(); + } + } else { + auto& path = get_enumeration_path_name(enmr_name); - // Create key - tiledb::sm::EncryptionKey key; - throw_if_not_ok( - key.set_key(tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0)); + // Create key + tiledb::sm::EncryptionKey key; + throw_if_not_ok( + key.set_key(tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0)); - // Load URIs from the array directory - optional array_dir; - array_dir.emplace( - ctx.resources(), - array_uri(), - 0, - UINT64_MAX, - tiledb::sm::ArrayDirectoryMode::SCHEMA_ONLY); + // Load URIs from the array directory + tiledb::sm::ArrayDirectory array_dir( + ctx.resources(), + array_uri(), + 0, + UINT64_MAX, + tiledb::sm::ArrayDirectoryMode::SCHEMA_ONLY); - auto tracker = ctx.resources().ephemeral_memory_tracker(); - auto enumeration = array_dir->load_enumeration(path, key, tracker); + auto tracker = ctx.resources().ephemeral_memory_tracker(); + auto enumeration = array_dir.load_enumeration(path, key, tracker); - enumeration_map_[enumeration->name()] = enumeration; - enumeration_path_map_[enumeration->name()] = enumeration->path_name(); + enumeration_map_[enumeration->name()] = enumeration; + enumeration_path_map_[enumeration->name()] = enumeration->path_name(); + } } void ArraySchema::drop_enumeration(const std::string& enmr_name) { @@ -1561,6 +1584,10 @@ uint64_t ArraySchema::timestamp_start() const { return timestamp_range_.first; } +uint64_t ArraySchema::timestamp_end() const { + return timestamp_range_.second; +} + const URI& ArraySchema::uri() const { return uri_; } diff --git a/tiledb/sm/array_schema/array_schema.h b/tiledb/sm/array_schema/array_schema.h index a757cd46c66..af7f46a55f5 100644 --- a/tiledb/sm/array_schema/array_schema.h +++ b/tiledb/sm/array_schema/array_schema.h @@ -577,9 +577,12 @@ class ArraySchema { /** Returns the timestamp range. */ std::pair timestamp_range() const; - /** Returns the the first timestamp. */ + /** Returns the first timestamp. */ uint64_t timestamp_start() const; + /** Returns the end timestamp */ + uint64_t timestamp_end() const; + /** Returns the array schema uri. */ const URI& uri() const; diff --git a/tiledb/sm/rest/rest_client.h b/tiledb/sm/rest/rest_client.h index cc456e258fe..3a823f0804f 100644 --- a/tiledb/sm/rest/rest_client.h +++ b/tiledb/sm/rest/rest_client.h @@ -383,7 +383,8 @@ class RestClient { const URI&, uint64_t, uint64_t, - Array*, + const Config&, + const ArraySchema&, const std::vector&, shared_ptr) { throw RestClientDisabledException(); diff --git a/tiledb/sm/rest/rest_client_remote.cc b/tiledb/sm/rest/rest_client_remote.cc index 87926f01efe..b01095d5f9f 100644 --- a/tiledb/sm/rest/rest_client_remote.cc +++ b/tiledb/sm/rest/rest_client_remote.cc @@ -571,14 +571,10 @@ RestClientRemote::post_enumerations_from_rest( const URI& uri, uint64_t timestamp_start, uint64_t timestamp_end, - Array* array, + const Config& config, + const ArraySchema& array_schema, const std::vector& enumeration_names, shared_ptr memory_tracker) { - if (array == nullptr) { - throw RestClientException( - "Error getting enumerations from REST; array is null."); - } - if (!memory_tracker) { memory_tracker = memory_tracker_; } @@ -586,7 +582,7 @@ RestClientRemote::post_enumerations_from_rest( BufferList serialized{memory_tracker_}; auto& buff = serialized.emplace_buffer(); serialization::serialize_load_enumerations_request( - array->config(), enumeration_names, serialization_type_, buff); + config, enumeration_names, serialization_type_, buff); // Init curl and form the URL Curl curlc(logger_); @@ -617,7 +613,7 @@ RestClientRemote::post_enumerations_from_rest( // Ensure data has a null delimiter for cap'n proto if using JSON throw_if_not_ok(ensure_json_null_delimited_string(&returned_data)); return serialization::deserialize_load_enumerations_response( - *array, serialization_type_, returned_data, memory_tracker); + array_schema, serialization_type_, returned_data, memory_tracker); } void RestClientRemote::post_query_plan_from_rest( diff --git a/tiledb/sm/rest/rest_client_remote.h b/tiledb/sm/rest/rest_client_remote.h index bc2f52c312a..38849851ed3 100644 --- a/tiledb/sm/rest/rest_client_remote.h +++ b/tiledb/sm/rest/rest_client_remote.h @@ -279,7 +279,8 @@ class RestClientRemote : public RestClient { * @param uri Array URI. * @param timestamp_start Inclusive starting timestamp at which to open array. * @param timestamp_end Inclusive ending timestamp at which to open array. - * @param array Array to fetch metadata for. + * @param config Config options + * @param array_schema Array schema to fetch metadata for. * @param enumeration_names The names of the enumerations to get. */ std::unordered_map>> @@ -287,7 +288,8 @@ class RestClientRemote : public RestClient { const URI& uri, uint64_t timestamp_start, uint64_t timestamp_end, - Array* array, + const Config& config, + const ArraySchema& array_schema, const std::vector& enumeration_names, shared_ptr) override; diff --git a/tiledb/sm/serialization/enumeration.cc b/tiledb/sm/serialization/enumeration.cc index da5ea73c239..c21251ab40a 100644 --- a/tiledb/sm/serialization/enumeration.cc +++ b/tiledb/sm/serialization/enumeration.cc @@ -191,7 +191,7 @@ void load_enumerations_response_to_capnp( std::unordered_map>> load_enumerations_response_from_capnp( const capnp::LoadEnumerationsResponse::Reader& reader, - const Array& array, + const ArraySchema& array_schema, shared_ptr memory_tracker) { std::unordered_map>> ret; @@ -204,7 +204,7 @@ load_enumerations_response_from_capnp( } // The name of the latest array schema will not be serialized in the // response if we are only loading enumerations from the latest schema. - return {{array.array_schema_latest().name(), loaded_enmrs}}; + return {{array_schema.name(), loaded_enmrs}}; } else if (reader.hasAllEnumerations()) { auto all_enmrs_reader = reader.getAllEnumerations(); for (auto enmr_entry_reader : all_enmrs_reader.getEntries()) { @@ -345,7 +345,7 @@ void serialize_load_enumerations_response( std::unordered_map>> deserialize_load_enumerations_response( - const Array& array, + const ArraySchema& array_schema, SerializationType serialize_type, span response, shared_ptr memory_tracker) { @@ -359,7 +359,7 @@ deserialize_load_enumerations_response( json.decode(kj::StringPtr(response.data(), response.size()), builder); capnp::LoadEnumerationsResponse::Reader reader = builder.asReader(); return load_enumerations_response_from_capnp( - reader, array, memory_tracker); + reader, array_schema, memory_tracker); } case SerializationType::CAPNP: { const auto mBytes = reinterpret_cast(response.data()); @@ -369,7 +369,7 @@ deserialize_load_enumerations_response( capnp::LoadEnumerationsResponse::Reader reader = array_reader.getRoot(); return load_enumerations_response_from_capnp( - reader, array, memory_tracker); + reader, array_schema, memory_tracker); } default: { throw EnumerationSerializationException( diff --git a/tiledb/sm/serialization/enumeration.h b/tiledb/sm/serialization/enumeration.h index ed53f9d4e4e..210d91f9d23 100644 --- a/tiledb/sm/serialization/enumeration.h +++ b/tiledb/sm/serialization/enumeration.h @@ -92,7 +92,7 @@ void serialize_load_enumerations_response( std::unordered_map>> deserialize_load_enumerations_response( - const Array& array, + const ArraySchema& array_schema, SerializationType serialization_type, span response, shared_ptr memory_tracker); From efc7cea1afb6891e4a9426497c7e2b832d356c56 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Tue, 29 Oct 2024 13:56:18 -0400 Subject: [PATCH 11/23] ArraySchema enumeration tests pass with REST server --- tiledb/sm/array_schema/array_schema.cc | 2 +- .../array_schema/array_schema_operations.cc | 28 ++++++++++++++++--- tiledb/sm/rest/rest_client_remote.cc | 11 +++++--- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/tiledb/sm/array_schema/array_schema.cc b/tiledb/sm/array_schema/array_schema.cc index a22cbd55548..0e15cdcaf0f 100644 --- a/tiledb/sm/array_schema/array_schema.cc +++ b/tiledb/sm/array_schema/array_schema.cc @@ -1155,7 +1155,7 @@ void ArraySchema::load_enumeration(Context& ctx, const std::string& enmr_name) { // response is a map {schema: [enumerations]} // we should be the only schema, and expect only one enumeration - for (auto enumeration : ret.begin()->second) { + for (auto enumeration : ret[name()]) { enumeration_map_[enumeration->name()] = enumeration; enumeration_path_map_[enumeration->name()] = enumeration->path_name(); } diff --git a/tiledb/sm/array_schema/array_schema_operations.cc b/tiledb/sm/array_schema/array_schema_operations.cc index d17388b48d6..bb8bab73547 100644 --- a/tiledb/sm/array_schema/array_schema_operations.cc +++ b/tiledb/sm/array_schema/array_schema_operations.cc @@ -238,12 +238,35 @@ shared_ptr load_array_schema( throw std::runtime_error("Failed to load array schema; Invalid array URI"); } + // Load enumerations if config option is set. + const bool incl_enums = config.get( + "rest.load_enumerations_on_array_open", Config::must_find); + if (uri.is_tiledb()) { auto& rest_client = ctx.rest_client(); auto&& [st, array_schema_response] = rest_client.get_array_schema_from_rest(uri); throw_if_not_ok(st); - return std::move(array_schema_response).value(); + auto array_schema = std::move(array_schema_response).value(); + + if (incl_enums) { + auto tracker = ctx.resources().ephemeral_memory_tracker(); + // Pass an empty list of enumeration names. REST will use timestamps to + // load all enumerations on all schemas for the array within that range. + auto ret = rest_client.post_enumerations_from_rest( + uri, + array_schema->timestamp_start(), + array_schema->timestamp_end(), + config, + *array_schema, + {}, + tracker); + + for (auto& enmr : ret[array_schema->name()]) { + array_schema->store_enumeration(enmr); + } + } + return array_schema; } else { // Create key tiledb::sm::EncryptionKey key; @@ -264,9 +287,6 @@ shared_ptr load_array_schema( auto&& array_schema_latest = array_dir->load_array_schema_latest(key, tracker); - // Load enumerations if config option is set. - bool incl_enums = config.get( - "rest.load_enumerations_on_array_open", Config::must_find); if (incl_enums) { std::vector enmr_paths_to_load; auto enmr_names = array_schema_latest->get_enumeration_names(); diff --git a/tiledb/sm/rest/rest_client_remote.cc b/tiledb/sm/rest/rest_client_remote.cc index b01095d5f9f..fb97579ba5f 100644 --- a/tiledb/sm/rest/rest_client_remote.cc +++ b/tiledb/sm/rest/rest_client_remote.cc @@ -220,10 +220,13 @@ RestClientRemote::get_array_schema_from_rest(const URI& uri) { // Ensure data has a null delimiter for cap'n proto if using JSON RETURN_NOT_OK_TUPLE( ensure_json_null_delimited_string(&returned_data), nullopt); - return { - Status::Ok(), - serialization::array_schema_deserialize( - serialization_type_, returned_data, memory_tracker_)}; + + auto array_schema = serialization::array_schema_deserialize( + serialization_type_, returned_data, memory_tracker_); + + array_schema->set_array_uri(uri); + + return {Status::Ok(), array_schema}; } std::tuple< From 2eeda891dc07082efd4cb6c8411083c6e16af7d3 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Tue, 29 Oct 2024 14:15:32 -0400 Subject: [PATCH 12/23] Move schema enumeration loading code to different compilation unit --- .../array_schema/array_schema_api_internal.h | 6 ++- tiledb/sm/array/array_operations.cc | 54 +++++++++++++++++++ tiledb/sm/array/array_operations.h | 13 +++++ tiledb/sm/array_schema/array_schema.cc | 52 ------------------ 4 files changed, 71 insertions(+), 54 deletions(-) diff --git a/tiledb/api/c_api/array_schema/array_schema_api_internal.h b/tiledb/api/c_api/array_schema/array_schema_api_internal.h index a2cbd5b0898..ae0ccc60b2c 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_internal.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_internal.h @@ -38,6 +38,7 @@ #include "tiledb/api/c_api/context/context_api_internal.h" #include "tiledb/api/c_api_support/handle/handle.h" #include "tiledb/common/common.h" +#include "tiledb/sm/array/array_operations.h" #include "tiledb/sm/array_schema/array_schema.h" #include "tiledb/sm/enums/array_type.h" #include "tiledb/sm/enums/layout.h" @@ -91,8 +92,9 @@ struct tiledb_array_schema_handle_t dim_id, name, label_order, label_type, check_name); } - void load_enumeration(tiledb_ctx_t* ctx, const char* name) { - return array_schema_->load_enumeration(ctx->context(), name); + void load_enumeration(tiledb_ctx_t* ctx, const char* enumeration_name) { + load_enumeration_into_schema( + ctx->context(), enumeration_name, *array_schema_); } shared_ptr get_enumeration( diff --git a/tiledb/sm/array/array_operations.cc b/tiledb/sm/array/array_operations.cc index f221db80c63..ba79a7645e1 100644 --- a/tiledb/sm/array/array_operations.cc +++ b/tiledb/sm/array/array_operations.cc @@ -51,6 +51,8 @@ #include "tiledb/sm/crypto/encryption_key.h" #include "tiledb/sm/misc/parallel_functions.h" #include "tiledb/sm/query/deletes_and_updates/serialization.h" +#include "tiledb/sm/rest/rest_client.h" +#include "tiledb/sm/storage_manager/context.h" #include "tiledb/sm/tile/generic_tile_io.h" namespace tiledb::sm { @@ -120,4 +122,56 @@ load_delete_and_update_conditions( return {conditions, update_values}; } +void load_enumeration_into_schema( + Context& ctx, const std::string& enmr_name, ArraySchema& array_schema) { + if (array_schema.is_enumeration_loaded(enmr_name)) { + return; + } + + auto tracker = ctx.resources().ephemeral_memory_tracker(); + + if (array_schema.array_uri().is_tiledb()) { + auto rest_client = ctx.resources().rest_client(); + if (rest_client == nullptr) { + throw ArrayOperationsException( + "Error loading enumerations; Remote array schema with no REST " + "client."); + } + + auto ret = rest_client->post_enumerations_from_rest( + array_schema.array_uri(), + array_schema.timestamp_start(), + array_schema.timestamp_end(), + ctx.resources().config(), + array_schema, + {enmr_name}, + tracker); + + // response is a map {schema: [enumerations]} + // we should be the only schema, and expect only one enumeration + for (auto enumeration : ret[array_schema.name()]) { + array_schema.store_enumeration(enumeration); + } + } else { + auto& path = array_schema.get_enumeration_path_name(enmr_name); + + // Create key + tiledb::sm::EncryptionKey key; + throw_if_not_ok( + key.set_key(tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0)); + + // Load URIs from the array directory + tiledb::sm::ArrayDirectory array_dir( + ctx.resources(), + array_schema.array_uri(), + 0, + UINT64_MAX, + tiledb::sm::ArrayDirectoryMode::SCHEMA_ONLY); + + auto enumeration = array_dir.load_enumeration(path, key, tracker); + + array_schema.store_enumeration(enumeration); + } +} + } // namespace tiledb::sm diff --git a/tiledb/sm/array/array_operations.h b/tiledb/sm/array/array_operations.h index b35c9fec285..4f8e7754c52 100644 --- a/tiledb/sm/array/array_operations.h +++ b/tiledb/sm/array/array_operations.h @@ -42,6 +42,8 @@ using namespace tiledb::common; namespace tiledb::sm { +class ArraySchema; +class Context; class ContextResources; class OpenedArray; class QueryCondition; @@ -58,6 +60,17 @@ tuple, std::vector>> load_delete_and_update_conditions( ContextResources& resources, const OpenedArray& opened_array); +/** + * Loads an enumeration into a schema. + * Used to implement `tiledb_array_schema_get_enumeration*` APIs. + * + * @param ctx + * @param enmr_name the requested enumeration + * @param schema the target schema + */ +void load_enumeration_into_schema( + Context& ctx, const std::string& enmr_name, ArraySchema& array_schema); + } // namespace tiledb::sm #endif // TILEDB_ARRAY_OPERATIONS_H diff --git a/tiledb/sm/array_schema/array_schema.cc b/tiledb/sm/array_schema/array_schema.cc index 0e15cdcaf0f..4f050f084d3 100644 --- a/tiledb/sm/array_schema/array_schema.cc +++ b/tiledb/sm/array_schema/array_schema.cc @@ -57,8 +57,6 @@ #include "tiledb/sm/fragment/fragment_identifier.h" #include "tiledb/sm/misc/hilbert.h" #include "tiledb/sm/misc/tdb_time.h" -#include "tiledb/sm/rest/rest_client.h" -#include "tiledb/sm/storage_manager/context.h" #include "tiledb/storage_format/uri/generate_uri.h" #include "tiledb/type/apply_with_type.h" @@ -1133,56 +1131,6 @@ const std::string& ArraySchema::get_enumeration_path_name( return iter->second; } -void ArraySchema::load_enumeration(Context& ctx, const std::string& enmr_name) { - if (is_enumeration_loaded(enmr_name)) { - return; - } else if (array_uri().is_tiledb()) { - auto rest_client = ctx.resources().rest_client(); - if (rest_client == nullptr) { - throw ArraySchemaException( - "Error loading enumerations; Remote array schema with no REST " - "client."); - } - - auto ret = rest_client->post_enumerations_from_rest( - array_uri(), - timestamp_start(), - timestamp_end(), - ctx.resources().config(), - *this, - {enmr_name}, - memory_tracker_); - - // response is a map {schema: [enumerations]} - // we should be the only schema, and expect only one enumeration - for (auto enumeration : ret[name()]) { - enumeration_map_[enumeration->name()] = enumeration; - enumeration_path_map_[enumeration->name()] = enumeration->path_name(); - } - } else { - auto& path = get_enumeration_path_name(enmr_name); - - // Create key - tiledb::sm::EncryptionKey key; - throw_if_not_ok( - key.set_key(tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0)); - - // Load URIs from the array directory - tiledb::sm::ArrayDirectory array_dir( - ctx.resources(), - array_uri(), - 0, - UINT64_MAX, - tiledb::sm::ArrayDirectoryMode::SCHEMA_ONLY); - - auto tracker = ctx.resources().ephemeral_memory_tracker(); - auto enumeration = array_dir.load_enumeration(path, key, tracker); - - enumeration_map_[enumeration->name()] = enumeration; - enumeration_path_map_[enumeration->name()] = enumeration->path_name(); - } -} - void ArraySchema::drop_enumeration(const std::string& enmr_name) { std::lock_guard lock(mtx_); From 9454c2105e1eb51555f62f44c65c31c0f074b850 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 30 Oct 2024 09:51:32 -0400 Subject: [PATCH 13/23] Fix linking issues --- tiledb/api/c_api/array_schema/CMakeLists.txt | 1 + .../array_schema/array_schema_api_internal.h | 2 +- tiledb/sm/array/array.cc | 56 +++++++++++++++++++ tiledb/sm/array/array.h | 11 ++++ tiledb/sm/array/array_operations.cc | 53 ------------------ tiledb/sm/array/array_operations.h | 11 ---- 6 files changed, 69 insertions(+), 65 deletions(-) diff --git a/tiledb/api/c_api/array_schema/CMakeLists.txt b/tiledb/api/c_api/array_schema/CMakeLists.txt index 39c6d30f4de..187639ee0ce 100644 --- a/tiledb/api/c_api/array_schema/CMakeLists.txt +++ b/tiledb/api/c_api/array_schema/CMakeLists.txt @@ -37,6 +37,7 @@ commence(object_library capi_array_schema_stub) this_target_link_libraries(export) this_target_object_libraries(array_schema) this_target_object_libraries(array) + this_target_object_libraries(capi_attribute_stub) this_target_object_libraries(capi_context_stub) conclude(object_library) diff --git a/tiledb/api/c_api/array_schema/array_schema_api_internal.h b/tiledb/api/c_api/array_schema/array_schema_api_internal.h index ae0ccc60b2c..364c05262d0 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_internal.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_internal.h @@ -38,7 +38,7 @@ #include "tiledb/api/c_api/context/context_api_internal.h" #include "tiledb/api/c_api_support/handle/handle.h" #include "tiledb/common/common.h" -#include "tiledb/sm/array/array_operations.h" +#include "tiledb/sm/array/array.h" #include "tiledb/sm/array_schema/array_schema.h" #include "tiledb/sm/enums/array_type.h" #include "tiledb/sm/enums/layout.h" diff --git a/tiledb/sm/array/array.cc b/tiledb/sm/array/array.cc index c4bd8814fa5..3126a0f1f3c 100644 --- a/tiledb/sm/array/array.cc +++ b/tiledb/sm/array/array.cc @@ -56,6 +56,7 @@ #include "tiledb/sm/object/object_mutex.h" #include "tiledb/sm/query/update_value.h" #include "tiledb/sm/rest/rest_client.h" +#include "tiledb/sm/storage_manager/context.h" #include "tiledb/sm/tile/generic_tile_io.h" #include @@ -2073,4 +2074,59 @@ void ensure_supported_schema_version_for_read(format_version_t version) { } } +// NB: this is used to implement `tiledb_array_schema_get_enumeration_*` +// but is defined here instead of array_schema to avoid a circular dependency +// (array_directory depends on array_schema). +void load_enumeration_into_schema( + Context& ctx, const std::string& enmr_name, ArraySchema& array_schema) { + if (array_schema.is_enumeration_loaded(enmr_name)) { + return; + } + + auto tracker = ctx.resources().ephemeral_memory_tracker(); + + if (array_schema.array_uri().is_tiledb()) { + auto rest_client = ctx.resources().rest_client(); + if (rest_client == nullptr) { + throw ArrayException( + "Error loading enumerations; Remote array schema with no REST " + "client."); + } + + auto ret = rest_client->post_enumerations_from_rest( + array_schema.array_uri(), + array_schema.timestamp_start(), + array_schema.timestamp_end(), + ctx.resources().config(), + array_schema, + {enmr_name}, + tracker); + + // response is a map {schema: [enumerations]} + // we should be the only schema, and expect only one enumeration + for (auto enumeration : ret[array_schema.name()]) { + array_schema.store_enumeration(enumeration); + } + } else { + auto& path = array_schema.get_enumeration_path_name(enmr_name); + + // Create key + tiledb::sm::EncryptionKey key; + throw_if_not_ok( + key.set_key(tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0)); + + // Load URIs from the array directory + tiledb::sm::ArrayDirectory array_dir( + ctx.resources(), + array_schema.array_uri(), + 0, + UINT64_MAX, + tiledb::sm::ArrayDirectoryMode::SCHEMA_ONLY); + + auto enumeration = array_dir.load_enumeration(path, key, tracker); + + array_schema.store_enumeration(enumeration); + } +} + } // namespace tiledb::sm diff --git a/tiledb/sm/array/array.h b/tiledb/sm/array/array.h index babc3009a43..763878aabff 100644 --- a/tiledb/sm/array/array.h +++ b/tiledb/sm/array/array.h @@ -1214,6 +1214,17 @@ class Array { void set_array_closed(); }; +/** + * Loads an enumeration into a schema. + * Used to implement `tiledb_array_schema_get_enumeration*` APIs. + * + * @param ctx + * @param enmr_name the requested enumeration + * @param schema the target schema + */ +void load_enumeration_into_schema( + Context& ctx, const std::string& enmr_name, ArraySchema& array_schema); + } // namespace tiledb::sm #endif // TILEDB_ARRAY_H diff --git a/tiledb/sm/array/array_operations.cc b/tiledb/sm/array/array_operations.cc index ba79a7645e1..0afc61ed6dc 100644 --- a/tiledb/sm/array/array_operations.cc +++ b/tiledb/sm/array/array_operations.cc @@ -52,7 +52,6 @@ #include "tiledb/sm/misc/parallel_functions.h" #include "tiledb/sm/query/deletes_and_updates/serialization.h" #include "tiledb/sm/rest/rest_client.h" -#include "tiledb/sm/storage_manager/context.h" #include "tiledb/sm/tile/generic_tile_io.h" namespace tiledb::sm { @@ -122,56 +121,4 @@ load_delete_and_update_conditions( return {conditions, update_values}; } -void load_enumeration_into_schema( - Context& ctx, const std::string& enmr_name, ArraySchema& array_schema) { - if (array_schema.is_enumeration_loaded(enmr_name)) { - return; - } - - auto tracker = ctx.resources().ephemeral_memory_tracker(); - - if (array_schema.array_uri().is_tiledb()) { - auto rest_client = ctx.resources().rest_client(); - if (rest_client == nullptr) { - throw ArrayOperationsException( - "Error loading enumerations; Remote array schema with no REST " - "client."); - } - - auto ret = rest_client->post_enumerations_from_rest( - array_schema.array_uri(), - array_schema.timestamp_start(), - array_schema.timestamp_end(), - ctx.resources().config(), - array_schema, - {enmr_name}, - tracker); - - // response is a map {schema: [enumerations]} - // we should be the only schema, and expect only one enumeration - for (auto enumeration : ret[array_schema.name()]) { - array_schema.store_enumeration(enumeration); - } - } else { - auto& path = array_schema.get_enumeration_path_name(enmr_name); - - // Create key - tiledb::sm::EncryptionKey key; - throw_if_not_ok( - key.set_key(tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0)); - - // Load URIs from the array directory - tiledb::sm::ArrayDirectory array_dir( - ctx.resources(), - array_schema.array_uri(), - 0, - UINT64_MAX, - tiledb::sm::ArrayDirectoryMode::SCHEMA_ONLY); - - auto enumeration = array_dir.load_enumeration(path, key, tracker); - - array_schema.store_enumeration(enumeration); - } -} - } // namespace tiledb::sm diff --git a/tiledb/sm/array/array_operations.h b/tiledb/sm/array/array_operations.h index 4f8e7754c52..a8ea970fa45 100644 --- a/tiledb/sm/array/array_operations.h +++ b/tiledb/sm/array/array_operations.h @@ -60,17 +60,6 @@ tuple, std::vector>> load_delete_and_update_conditions( ContextResources& resources, const OpenedArray& opened_array); -/** - * Loads an enumeration into a schema. - * Used to implement `tiledb_array_schema_get_enumeration*` APIs. - * - * @param ctx - * @param enmr_name the requested enumeration - * @param schema the target schema - */ -void load_enumeration_into_schema( - Context& ctx, const std::string& enmr_name, ArraySchema& array_schema); - } // namespace tiledb::sm #endif // TILEDB_ARRAY_OPERATIONS_H From 1f0962591be04b6cd3f2ed2a92d4cbf599fcaf01 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 30 Oct 2024 11:15:05 -0400 Subject: [PATCH 14/23] Revert array_operations changes --- tiledb/sm/array/array_operations.cc | 1 - tiledb/sm/array/array_operations.h | 2 -- 2 files changed, 3 deletions(-) diff --git a/tiledb/sm/array/array_operations.cc b/tiledb/sm/array/array_operations.cc index 0afc61ed6dc..f221db80c63 100644 --- a/tiledb/sm/array/array_operations.cc +++ b/tiledb/sm/array/array_operations.cc @@ -51,7 +51,6 @@ #include "tiledb/sm/crypto/encryption_key.h" #include "tiledb/sm/misc/parallel_functions.h" #include "tiledb/sm/query/deletes_and_updates/serialization.h" -#include "tiledb/sm/rest/rest_client.h" #include "tiledb/sm/tile/generic_tile_io.h" namespace tiledb::sm { diff --git a/tiledb/sm/array/array_operations.h b/tiledb/sm/array/array_operations.h index a8ea970fa45..b35c9fec285 100644 --- a/tiledb/sm/array/array_operations.h +++ b/tiledb/sm/array/array_operations.h @@ -42,8 +42,6 @@ using namespace tiledb::common; namespace tiledb::sm { -class ArraySchema; -class Context; class ContextResources; class OpenedArray; class QueryCondition; From d42551ed2d3dd754958dc88159192119a99d1935 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 30 Oct 2024 11:21:24 -0400 Subject: [PATCH 15/23] Revert no-longer-used includes --- tiledb/sm/array/array.h | 1 + tiledb/sm/array_schema/array_schema.cc | 3 --- tiledb/sm/array_schema/array_schema.h | 9 --------- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/tiledb/sm/array/array.h b/tiledb/sm/array/array.h index 763878aabff..2861cbd5435 100644 --- a/tiledb/sm/array/array.h +++ b/tiledb/sm/array/array.h @@ -52,6 +52,7 @@ namespace tiledb::sm { class ArraySchema; class ArraySchemaEvolution; +class Context; class FragmentMetadata; class MemoryTracker; enum class QueryType : uint8_t; diff --git a/tiledb/sm/array_schema/array_schema.cc b/tiledb/sm/array_schema/array_schema.cc index 4f050f084d3..34046da55f8 100644 --- a/tiledb/sm/array_schema/array_schema.cc +++ b/tiledb/sm/array_schema/array_schema.cc @@ -36,7 +36,6 @@ #include "tiledb/common/heap_memory.h" #include "tiledb/common/logger.h" #include "tiledb/common/memory_tracker.h" -#include "tiledb/sm/array/array_directory.h" #include "tiledb/sm/array_schema/attribute.h" #include "tiledb/sm/array_schema/current_domain.h" #include "tiledb/sm/array_schema/dimension.h" @@ -44,12 +43,10 @@ #include "tiledb/sm/array_schema/domain.h" #include "tiledb/sm/array_schema/enumeration.h" #include "tiledb/sm/buffer/buffer.h" -#include "tiledb/sm/crypto/encryption_key.h" #include "tiledb/sm/enums/array_type.h" #include "tiledb/sm/enums/compressor.h" #include "tiledb/sm/enums/data_order.h" #include "tiledb/sm/enums/datatype.h" -#include "tiledb/sm/enums/encryption_type.h" #include "tiledb/sm/enums/filter_type.h" #include "tiledb/sm/enums/layout.h" #include "tiledb/sm/filter/compression_filter.h" diff --git a/tiledb/sm/array_schema/array_schema.h b/tiledb/sm/array_schema/array_schema.h index af7f46a55f5..4dec74ee410 100644 --- a/tiledb/sm/array_schema/array_schema.h +++ b/tiledb/sm/array_schema/array_schema.h @@ -51,7 +51,6 @@ namespace tiledb::sm { class Attribute; class Buffer; class ConstBuffer; -class Context; class Dimension; class DimensionLabel; class Domain; @@ -455,14 +454,6 @@ class ArraySchema { shared_ptr get_enumeration( const std::string& enmr_name) const; - /** - * Load an enumeration by name for subsequent retrieval. - * Throws if the enumeration is unknown. - * - * @param enmr_name The name of the Enumeration. - */ - void load_enumeration(Context& context, const std::string& enmr_name); - /** * Get an Enumeration's object name. Throws if the enumeration is unknown. * From b9a5b46e514d39f52eb0a94113ed8dea6aedc636 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 30 Oct 2024 11:24:23 -0400 Subject: [PATCH 16/23] Remove C4459 errors --- test/src/unit-cppapi-enumerations.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index 26df4bd2546..95e9027422a 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -320,8 +320,6 @@ TEST_CASE_METHOD( "[enumeration][array-schema-get-enumeration-from-name][rest]") { create_array(); - const std::string enmr_name = "an_enumeration"; - std::optional expect_enumeration; { auto array = tiledb::Array(ctx_, uri_, TILEDB_READ); @@ -367,7 +365,6 @@ TEST_CASE_METHOD( "[enumeration][array-schema-get-enumeration-from-attribute-name][rest]") { create_array(); - const std::string enmr_name = "an_enumeration"; const std::string attr_name = "attr1"; std::optional expect_enumeration; From 4008d80888a0f8426c7ed5c3df090719cf18d26d Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 30 Oct 2024 11:28:19 -0400 Subject: [PATCH 17/23] Fix duplicate definitions in unit_capi_array_schema --- tiledb/api/c_api/array_schema/test/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/tiledb/api/c_api/array_schema/test/CMakeLists.txt b/tiledb/api/c_api/array_schema/test/CMakeLists.txt index a61b624205b..f39b09d180d 100644 --- a/tiledb/api/c_api/array_schema/test/CMakeLists.txt +++ b/tiledb/api/c_api/array_schema/test/CMakeLists.txt @@ -30,7 +30,6 @@ commence(unit_test capi_array_schema) this_target_sources(unit_capi_array_schema.cc) this_target_object_libraries( capi_array_schema_stub - capi_attribute_stub capi_current_domain capi_domain_stub ) From f13411db63b4ab2d39c5bf70443c6acf46e04039 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 30 Oct 2024 12:01:17 -0400 Subject: [PATCH 18/23] Fix unit_capi_array linker errors --- tiledb/api/c_api/array/test/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/tiledb/api/c_api/array/test/CMakeLists.txt b/tiledb/api/c_api/array/test/CMakeLists.txt index f846cc1e97e..5496cc0c1fd 100644 --- a/tiledb/api/c_api/array/test/CMakeLists.txt +++ b/tiledb/api/c_api/array/test/CMakeLists.txt @@ -31,7 +31,6 @@ commence(unit_test capi_array) this_target_object_libraries( capi_array_stub capi_array_schema_stub - capi_attribute_stub capi_domain_stub ) this_target_link_libraries(tiledb_test_support_lib) From 7b42669507f8b3995faec812297d85767eed93c9 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Wed, 30 Oct 2024 14:22:43 -0400 Subject: [PATCH 19/23] Fix platform-dependent SEGV from passing NULL attribute name to tiledb_array_schema_get_attribute_from_name --- tiledb/api/c_api/array_schema/array_schema_api.cc | 4 ++++ tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/tiledb/api/c_api/array_schema/array_schema_api.cc b/tiledb/api/c_api/array_schema/array_schema_api.cc index 1a000aa6790..dfaf4d093f4 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api.cc +++ b/tiledb/api/c_api/array_schema/array_schema_api.cc @@ -414,6 +414,10 @@ capi_return_t tiledb_array_schema_get_attribute_from_name( ensure_array_schema_is_valid(array_schema); ensure_output_pointer_is_valid(attr); + if (name == nullptr) { + throw CAPIException("'attribute_name' must not be null"); + } + uint32_t attribute_num = array_schema->attribute_num(); if (attribute_num == 0) { *attr = nullptr; diff --git a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc index 02c789f42e3..98a388072bd 100644 --- a/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc +++ b/tiledb/api/c_api/array_schema/test/unit_capi_array_schema.cc @@ -902,6 +902,11 @@ TEST_CASE( x.ctx(), nullptr, "a", &attr); REQUIRE(tiledb_status(rc) == TILEDB_ERR); } + SECTION("null name") { + rc = tiledb_array_schema_get_attribute_from_name( + x.ctx(), x.schema, nullptr, &attr); + REQUIRE(tiledb_status(rc) == TILEDB_ERR); + } SECTION("invalid name") { rc = tiledb_array_schema_get_attribute_from_name( x.ctx(), x.schema, "b", &attr); From f0727b9b92ebd584f6d9a79e1f8fd3a391728094 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 31 Oct 2024 15:16:30 -0400 Subject: [PATCH 20/23] Update tiledb/api/c_api/array_schema/array_schema_api_experimental.h Co-authored-by: Shaun M Reed --- tiledb/api/c_api/array_schema/array_schema_api_experimental.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h index 2f95219a95c..68f17fba8d3 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h @@ -78,7 +78,7 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_timestamp_range( * tiledb_enumeration_t* enmr; * tiledb_array_schema_get_enumeration_from_name(ctx, * array_schema, "states", &enmr); - * tiledb_attribute_free(&enmr); + * tiledb_enumeration_free(&enmr); * @endcode * * @param[in] ctx The TileDB context. From 85ffae960014366ec7769359544b7e35da27aa92 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 31 Oct 2024 15:16:44 -0400 Subject: [PATCH 21/23] Update tiledb/api/c_api/array_schema/array_schema_api_experimental.h Co-authored-by: Shaun M Reed --- tiledb/api/c_api/array_schema/array_schema_api_experimental.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h index 68f17fba8d3..cb31631f7ea 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h @@ -106,7 +106,7 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_get_enumeration_from_name( * tiledb_enumeration_t* enmr; * tiledb_array_schema_get_enumeration_from_attribute_name(ctx, * array_schema, "states", &enmr); - * tiledb_attribute_free(&enmr); + * tiledb_enumeration_free(&enmr); * @endcode * * @param[in] ctx The TileDB context. From 373c4c2d0efec33891f32e640c14b06382c5c8dd Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 31 Oct 2024 15:17:22 -0400 Subject: [PATCH 22/23] Update tiledb/sm/rest/rest_client_remote.h Co-authored-by: Shaun M Reed --- tiledb/sm/rest/rest_client_remote.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tiledb/sm/rest/rest_client_remote.h b/tiledb/sm/rest/rest_client_remote.h index 38849851ed3..3d823d97328 100644 --- a/tiledb/sm/rest/rest_client_remote.h +++ b/tiledb/sm/rest/rest_client_remote.h @@ -280,7 +280,7 @@ class RestClientRemote : public RestClient { * @param timestamp_start Inclusive starting timestamp at which to open array. * @param timestamp_end Inclusive ending timestamp at which to open array. * @param config Config options - * @param array_schema Array schema to fetch metadata for. + * @param array_schema Array schema to fetch enumerations for. * @param enumeration_names The names of the enumerations to get. */ std::unordered_map>> From 2967b0d684176f5d3d0bac61c1da1f9155559109 Mon Sep 17 00:00:00 2001 From: Ryan Roelke Date: Thu, 31 Oct 2024 15:17:40 -0400 Subject: [PATCH 23/23] Update tiledb/api/c_api/array_schema/array_schema_api_experimental.h Co-authored-by: Shaun M Reed --- tiledb/api/c_api/array_schema/array_schema_api_experimental.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h index cb31631f7ea..ca2cd467c4e 100644 --- a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h +++ b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h @@ -111,7 +111,7 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_get_enumeration_from_name( * * @param[in] ctx The TileDB context. * @param[in] array_schema The array schema. - * @param[in] name The name of the attribute whose to retrieve. + * @param[in] name The name of the attribute whose enumeration to retrieve. * @param[out] enmr The enumeration object to retrieve. * @return `TILEDB_OK` for success and `TILEDB_ERR` for error. */