Address feedback from @eric-hughes-tiledb

TileDB-Inc · Oct 23, 2023 · c96c03e · c96c03e
1 parent c61f909
commit c96c03e
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 44 deletions.
diff --git a/test/src/test-cppapi-aggregates.cc b/test/src/test-cppapi-aggregates.cc
@@ -1987,13 +1987,9 @@ TEST_CASE_METHOD(
         // second attribute.
         query.ptr()->query_->add_aggregator_to_default_channel(
             "NullCount2",
-            std::make_shared<tiledb::sm::NullCountAggregator>(
-                tiledb::sm::FieldInfo(
-                    "a2",
-                    true,
-                    nullable_,
-                    TILEDB_VAR_NUM,
-                    tdb_type<std::string>())));
+            std::make_shared<
+                tiledb::sm::NullCountAggregator>(tiledb::sm::FieldInfo(
+                "a2", true, nullable_, TILEDB_VAR_NUM, tdb_type<std::string>)));
 
         set_ranges_and_condition_if_needed(array, query, true);
 
@@ -2129,13 +2125,9 @@ TEST_CASE_METHOD(
         // the first one hence throw an exception.
         query.ptr()->query_->add_aggregator_to_default_channel(
             "NullCount2",
-            std::make_shared<tiledb::sm::NullCountAggregator>(
-                tiledb::sm::FieldInfo(
-                    "a2",
-                    true,
-                    nullable_,
-                    TILEDB_VAR_NUM,
-                    tdb_type<std::string>())));
+            std::make_shared<
+                tiledb::sm::NullCountAggregator>(tiledb::sm::FieldInfo(
+                "a2", true, nullable_, TILEDB_VAR_NUM, tdb_type<std::string>)));
 
         set_ranges_and_condition_if_needed(array, query, true);
 

diff --git a/test/support/src/helper_type.h b/test/support/src/helper_type.h
@@ -1,5 +1,5 @@
 /**
- * @file   type.h
+ * @file   helper_type.h
  *
  * @section LICENSE
  *
@@ -42,87 +42,88 @@ using Datatype = tiledb::sm::Datatype;
  * types is char.
  */
 template <typename T>
-struct type_to_tiledb {
-  using type = char;
-  static const Datatype tiledb_type = Datatype::STRING_ASCII;
-};
+struct type_to_tiledb {};
 
 template <>
 struct type_to_tiledb<std::byte> {
   using type = std::byte;
-  static const Datatype tiledb_type = Datatype::BLOB;
+  static constexpr Datatype tiledb_type = Datatype::BLOB;
 };
 
 template <>
 struct type_to_tiledb<bool> {
   using type = bool;
-  static const Datatype tiledb_type = Datatype::BOOL;
+  static constexpr Datatype tiledb_type = Datatype::BOOL;
 };
 
 template <>
 struct type_to_tiledb<int8_t> {
   using type = int8_t;
-  static const Datatype tiledb_type = Datatype::INT8;
+  static constexpr Datatype tiledb_type = Datatype::INT8;
 };
 
 template <>
 struct type_to_tiledb<uint8_t> {
   using type = uint8_t;
-  static const Datatype tiledb_type = Datatype::UINT8;
+  static constexpr Datatype tiledb_type = Datatype::UINT8;
 };
 
 template <>
 struct type_to_tiledb<int16_t> {
   using type = int16_t;
-  static const Datatype tiledb_type = Datatype::INT16;
+  static constexpr Datatype tiledb_type = Datatype::INT16;
 };
 
 template <>
 struct type_to_tiledb<uint16_t> {
   using type = uint16_t;
-  static const Datatype tiledb_type = Datatype::UINT16;
+  static constexpr Datatype tiledb_type = Datatype::UINT16;
 };
 
 template <>
 struct type_to_tiledb<int32_t> {
   using type = int32_t;
-  static const Datatype tiledb_type = Datatype::INT32;
+  static constexpr Datatype tiledb_type = Datatype::INT32;
 };
 
 template <>
 struct type_to_tiledb<uint32_t> {
   using type = uint32_t;
-  static const Datatype tiledb_type = Datatype::UINT32;
+  static constexpr Datatype tiledb_type = Datatype::UINT32;
 };
 
 template <>
 struct type_to_tiledb<int64_t> {
   using type = int64_t;
-  static const Datatype tiledb_type = Datatype::INT64;
+  static constexpr Datatype tiledb_type = Datatype::INT64;
 };
 
 template <>
 struct type_to_tiledb<uint64_t> {
   using type = uint64_t;
-  static const Datatype tiledb_type = Datatype::UINT64;
+  static constexpr Datatype tiledb_type = Datatype::UINT64;
 };
 
 template <>
 struct type_to_tiledb<float> {
   using type = float;
-  static const Datatype tiledb_type = Datatype::FLOAT32;
+  static constexpr Datatype tiledb_type = Datatype::FLOAT32;
 };
 
 template <>
 struct type_to_tiledb<double> {
   using type = double;
-  static const Datatype tiledb_type = Datatype::FLOAT64;
+  static constexpr Datatype tiledb_type = Datatype::FLOAT64;
+};
+
+template <>
+struct type_to_tiledb<std::string> {
+  using type = char;
+  static const Datatype tiledb_type = Datatype::STRING_ASCII;
 };
 
 template <typename T>
-Datatype tdb_type() {
-  return type_to_tiledb<T>::tiledb_type;
-}
+constexpr Datatype tdb_type = type_to_tiledb<T>::tiledb_type;
 
 }  // namespace tiledb::test
 

diff --git a/tiledb/sm/query/readers/aggregators/test/bench_aggregators.cc b/tiledb/sm/query/readers/aggregators/test/bench_aggregators.cc
@@ -158,7 +158,7 @@ void run_bench() {
                     << ", Segmented: " << (segmented ? "true" : "false")) {
     BENCHMARK("Bench") {
       AggregateWithCount<T, AggregateT, PolicyT, ValidityPolicyT> aggregator(
-          FieldInfo("a1", var_sized, nullable, 1, tdb_type<T>()));
+          FieldInfo("a1", var_sized, nullable, 1, tdb_type<T>));
       for (uint64_t s = 0; s < num_cells; s += increment) {
         AggregateBuffer input_data{
             s,

diff --git a/tiledb/sm/query/readers/aggregators/test/unit_aggregate_with_count.cc b/tiledb/sm/query/readers/aggregators/test/unit_aggregate_with_count.cc
@@ -61,9 +61,9 @@ TEMPLATE_LIST_TEST_CASE(
     FixedTypesUnderTest) {
   typedef TestType T;
   AggregateWithCount<T, typename sum_type_data<T>::sum_type, SafeSum, NonNull>
-      aggregator(FieldInfo("a1", false, false, 1, tdb_type<T>()));
+      aggregator(FieldInfo("a1", false, false, 1, tdb_type<T>));
   AggregateWithCount<T, typename sum_type_data<T>::sum_type, SafeSum, NonNull>
-      aggregator_nullable(FieldInfo("a2", false, true, 1, tdb_type<T>()));
+      aggregator_nullable(FieldInfo("a2", false, true, 1, tdb_type<T>));
 
   std::vector<T> fixed_data = {1, 2, 3, 4, 5, 5, 4, 3, 2, 1};
   std::vector<uint8_t> validity_data = {0, 0, 1, 0, 1, 0, 1, 0, 1, 0};

diff --git a/tiledb/sm/query/readers/aggregators/test/unit_aggregators.cc b/tiledb/sm/query/readers/aggregators/test/unit_aggregators.cc
@@ -444,12 +444,12 @@ void basic_aggregation_test(std::vector<double> expected_results) {
     if constexpr (std::is_same<AGGREGATOR, CountAggregator>::value) {
       aggregator.emplace();
     } else {
-      aggregator.emplace(FieldInfo("a1", false, false, 1, tdb_type<T>()));
+      aggregator.emplace(FieldInfo("a1", false, false, 1, tdb_type<T>));
     }
   }
 
   auto aggregator_nullable{make_aggregator<AGGREGATOR>(
-      FieldInfo("a1", false, true, 1, tdb_type<T>()))};
+      FieldInfo("a1", false, true, 1, tdb_type<T>))};
 
   std::unordered_map<std::string, QueryBuffer> buffers;
 
@@ -710,7 +710,7 @@ TEMPLATE_LIST_TEST_CASE(
 TEST_CASE(
     "Sum aggregator: signed overflow", "[sum-aggregator][signed-overflow]") {
   SumAggregator<int64_t> aggregator(
-      FieldInfo("a1", false, false, 1, tdb_type<int64_t>()));
+      FieldInfo("a1", false, false, 1, tdb_type<int64_t>));
 
   std::unordered_map<std::string, QueryBuffer> buffers;
 
@@ -795,7 +795,7 @@ TEST_CASE(
     "Sum aggregator: unsigned overflow",
     "[sum-aggregator][unsigned-overflow]") {
   SumAggregator<uint64_t> aggregator(
-      FieldInfo("a1", false, false, 1, tdb_type<int64_t>()));
+      FieldInfo("a1", false, false, 1, tdb_type<int64_t>));
 
   std::unordered_map<std::string, QueryBuffer> buffers;
 
@@ -833,7 +833,7 @@ TEMPLATE_LIST_TEST_CASE(
     "Sum aggregator: double overflow",
     "[sum-aggregator][double-overflow]",
     DoubleAggUnderTest) {
-  TestType aggregator(FieldInfo("a1", false, false, 1, tdb_type<double>()));
+  TestType aggregator(FieldInfo("a1", false, false, 1, tdb_type<double>));
 
   std::unordered_map<std::string, QueryBuffer> buffers;
 
@@ -910,11 +910,11 @@ void basic_string_aggregation_test(std::vector<RES> expected_results) {
   optional<AGGREGATOR> aggregator;
   if constexpr (!std::is_same<AGGREGATOR, NullCountAggregator>::value) {
     aggregator.emplace(FieldInfo(
-        "a1", true, false, constants::var_num, tdb_type<std::string>()));
+        "a1", true, false, constants::var_num, tdb_type<std::string>));
   }
 
   AGGREGATOR aggregator_nullable(
-      FieldInfo("a2", true, true, constants::var_num, tdb_type<std::string>()));
+      FieldInfo("a2", true, true, constants::var_num, tdb_type<std::string>));
 
   std::unordered_map<std::string, QueryBuffer> buffers;