From 3d12161986b1ddc5a67c8a243a4fe8ae38dede21 Mon Sep 17 00:00:00 2001 From: "Paul J. Davis" Date: Tue, 19 Sep 2023 03:18:32 -0500 Subject: [PATCH] Fix Enumeration API when passed std::vector (#4362) I forgot to account for the fact that `std::vector` is a specialized template which uses an internal bitmap instead of a standard backing array. (cherry picked from commit 41bc0f2c16f732047f8f24aa1ca4ef40df342723) --- test/src/unit-cppapi-enumerations.cc | 16 ++++++++ test/src/unit-enumerations.cc | 33 +++++++++++++++- tiledb/sm/cpp_api/enumeration_experimental.h | 40 +++++++++++++++----- 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc index ddbea7fb2da..c9c7ff68488 100644 --- a/test/src/unit-cppapi-enumerations.cc +++ b/test/src/unit-cppapi-enumerations.cc @@ -58,6 +58,22 @@ struct CPPEnumerationFx { const static std::string enmr_name = "an_enumeration"; const static std::string dump_name = "enumeration_dump_test.txt"; +TEST_CASE_METHOD( + CPPEnumerationFx, + "CPP: Enumeration API - Create Boolean", + "[enumeration][create][bool]") { + std::vector values = {true, false}; + auto enmr = Enumeration::create(ctx_, enmr_name, values); + REQUIRE(enmr.ptr() != nullptr); + REQUIRE(enmr.name() == enmr_name); + REQUIRE(enmr.type() == TILEDB_BOOL); + REQUIRE(enmr.cell_val_num() == 1); + REQUIRE(enmr.ordered() == false); + + auto data = enmr.as_vector(); + REQUIRE(data == values); +} + TEST_CASE_METHOD( CPPEnumerationFx, "CPP: Enumeration API - Create Fixed Size", diff --git a/test/src/unit-enumerations.cc b/test/src/unit-enumerations.cc index 3fbef15d0c7..4c0e1c0c07e 100644 --- a/test/src/unit-enumerations.cc +++ b/test/src/unit-enumerations.cc @@ -143,6 +143,15 @@ QueryCondition create_qc( /* Testing Enumeration */ /* ********************************* */ +TEST_CASE_METHOD( + EnumerationFx, + "Basic Boolean Enumeration Creation", + "[enumeration][basic][boolean]") { + std::vector values = {true, false}; + auto enmr = create_enumeration(values); + check_enumeration(enmr, default_enmr_name, values, Datatype::BOOL, 1, false); +} + TEST_CASE_METHOD( EnumerationFx, "Basic Fixed Size Enumeration Creation", @@ -156,7 +165,7 @@ TEST_CASE_METHOD( TEST_CASE_METHOD( EnumerationFx, "Basic Variable Size Enumeration Creation", - "[enumeration][basic][fixed]") { + "[enumeration][basic][var-size]") { std::vector values = {"foo", "bar", "baz", "bingo", "bango"}; auto enmr = create_enumeration(values); check_enumeration( @@ -1672,6 +1681,10 @@ struct TypeParams { return TypeParams(Datatype::STRING_ASCII, constants::var_num); } + static TypeParams get(const std::vector&) { + return TypeParams(Datatype::BOOL, 1); + } + static TypeParams get(const std::vector&) { return TypeParams(Datatype::INT32, 1); } @@ -1719,7 +1732,23 @@ shared_ptr EnumerationFx::create_enumeration( tp.type_ = type; } - if constexpr (std::is_pod_v) { + if constexpr (std::is_same_v) { + // We have to call out bool specifically because of the stdlib + // specialization for std::vector + std::vector raw_values(values.size()); + for (size_t i = 0; i < values.size(); i++) { + raw_values[i] = values[i] ? 1 : 0; + } + return Enumeration::create( + name, + tp.type_, + tp.cell_val_num_, + ordered, + raw_values.data(), + raw_values.size() * sizeof(uint8_t), + nullptr, + 0); + } else if constexpr (std::is_pod_v) { return Enumeration::create( name, tp.type_, diff --git a/tiledb/sm/cpp_api/enumeration_experimental.h b/tiledb/sm/cpp_api/enumeration_experimental.h index 0fac073fedb..ca9c37820a0 100644 --- a/tiledb/sm/cpp_api/enumeration_experimental.h +++ b/tiledb/sm/cpp_api/enumeration_experimental.h @@ -243,16 +243,36 @@ class Enumeration { using DataT = impl::TypeHandler; tiledb_datatype_t dtype = type.value_or(DataT::tiledb_type); - return create( - ctx, - name, - dtype, - DataT::tiledb_num, - ordered, - values.data(), - values.size() * sizeof(T), - nullptr, - 0); + if constexpr (!std::is_same_v) { + return create( + ctx, + name, + dtype, + DataT::tiledb_num, + ordered, + values.data(), + values.size() * sizeof(T), + nullptr, + 0); + } else { + // This awkward dance for std::vector is due to the fact that + // C++ provides a template specialization that uses a bitmap which does + // not provide `data()` member method. + std::vector converted(values.size()); + for (size_t i = 0; i < values.size(); i++) { + converted[i] = values[i] ? 1 : 0; + } + return create( + ctx, + name, + dtype, + DataT::tiledb_num, + ordered, + converted.data(), + converted.size() * sizeof(uint8_t), + nullptr, + 0); + } } /**