Merge pull request #282 from apache/quantiles_patch

fixes for type converting constructor, align documentation with kll

Author: jmalkin

quantiles/include/quantiles_sketch.hpp

@@ -162,6 +162,11 @@ class quantiles_sketch {
   quantiles_sketch& operator=(const quantiles_sketch& other);
   quantiles_sketch& operator=(quantiles_sketch&& other) noexcept;
 
+  /**
+   * @brief Type converting constructor
+   * @param other quantiles sketch of a different type
+   * @param allocator instance of an Allocator
+   */
   template<typename From, typename FC, typename FA>
   explicit quantiles_sketch(const quantiles_sketch<From, FC, FA>& other, const Allocator& allocator = Allocator());
 

quantiles/include/quantiles_sketch_impl.hpp

@@ -151,9 +151,8 @@ min_value_(nullptr),
 max_value_(nullptr),
 is_sorted_(false)
 {
-  static_assert(std::is_convertible<From, T>::value
-                || std::is_constructible<From, T>::value,
-                "Copy constructor across types requires std::is_convertible or std::is_constructible");
+  static_assert(std::is_constructible<T, From>::value,
+                "Type converting constructor requires new type to be constructible from existing type");
 
   base_buffer_.reserve(2 * std::min(quantiles_constants::MIN_K, k_));
 
@@ -174,7 +173,7 @@ is_sorted_(false)
     for (auto pair : other) {
       const uint64_t wt = pair.second;
       if (wt == 1) {
-        base_buffer_.push_back(pair.first);
+        base_buffer_.push_back(T(pair.first));
         // resize where needed as if adding points via update()
         if (base_buffer_.size() + 1 > base_buffer_.capacity()) {
           const size_t new_size = std::max(std::min(static_cast<size_t>(2 * k_), 2 * base_buffer_.size()), static_cast<size_t>(1));
@@ -183,7 +182,7 @@ is_sorted_(false)
       }
       else {
         const uint8_t idx = count_trailing_zeros_in_u64(pair.second) - 1;
-        levels_[idx].push_back(pair.first);
+        levels_[idx].push_back(T(pair.first));
       }
     }
 

quantiles/test/quantiles_sketch_test.cpp

@@ -934,6 +934,38 @@ TEST_CASE("quantiles sketch", "[quantiles_sketch]") {
     }
   }
 
+  class A {
+    int val;
+  public:
+    A(int val): val(val) {}
+    int get_val() const { return val; }
+  };
+
+  struct less_A {
+    bool operator()(const A& a1, const A& a2) const { return a1.get_val() < a2.get_val(); }
+  };
+
+  class B {
+    int val;
+  public:
+    explicit B(const A& a): val(a.get_val()) {}
+    int get_val() const { return val; }
+  };
+
+  struct less_B {
+    bool operator()(const B& b1, const B& b2) const { return b1.get_val() < b2.get_val(); }
+  };
+
+  SECTION("type conversion: custom types") {
+    quantiles_sketch<A, less_A> sa;
+    sa.update(1);
+    sa.update(2);
+    sa.update(3);
+
+    quantiles_sketch<B, less_B> sb(sa);
+    REQUIRE(sb.get_n() == 3);
+  }
+
   // cleanup
   if (test_allocator_total_bytes != 0) {
     REQUIRE(test_allocator_total_bytes == 0);