Fix Enumeration API when passed std::vector<bool> (#4362)

I forgot to account for the fact that `std::vector<bool>` is a specialized template which uses an internal bitmap instead of a standard backing array. (cherry picked from commit 41bc0f2)
TileDB-Inc · Sep 24, 2023 · 3d12161 · 3d12161
1 parent 54473d4
commit 3d12161
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 12 deletions.
diff --git 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<bool> 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<bool>();
+  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
@@ -143,6 +143,15 @@ QueryCondition create_qc(
 /*        Testing Enumeration        */
 /* ********************************* */
 
+TEST_CASE_METHOD(
+    EnumerationFx,
+    "Basic Boolean Enumeration Creation",
+    "[enumeration][basic][boolean]") {
+  std::vector<bool> 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<std::string> 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<bool>&) {
+    return TypeParams(Datatype::BOOL, 1);
+  }
+
   static TypeParams get(const std::vector<int>&) {
     return TypeParams(Datatype::INT32, 1);
   }
@@ -1719,7 +1732,23 @@ shared_ptr<const Enumeration> EnumerationFx::create_enumeration(
     tp.type_ = type;
   }
 
-  if constexpr (std::is_pod_v<T>) {
+  if constexpr (std::is_same_v<T, bool>) {
+    // We have to call out bool specifically because of the stdlib
+    // specialization for std::vector<bool>
+    std::vector<uint8_t> 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<T>) {
     return Enumeration::create(
         name,
         tp.type_,

diff --git 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<T>;
     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<T, bool>) {
+      return create(
+          ctx,
+          name,
+          dtype,
+          DataT::tiledb_num,
+          ordered,
+          values.data(),
+          values.size() * sizeof(T),
+          nullptr,
+          0);
+    } else {
+      // This awkward dance for std::vector<bool> is due to the fact that
+      // C++ provides a template specialization that uses a bitmap which does
+      // not provide `data()` member method.
+      std::vector<uint8_t> 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);
+    }
   }
 
   /**