From ef6d3be53a72b7e5708fe1d2ea701c6e20ff6a6e Mon Sep 17 00:00:00 2001
From: Alexey Milovidov <milovidov@yandex-team.ru>
Date: Thu, 21 Dec 2017 04:19:25 +0300
Subject: [PATCH] Simplification of aggregate functions: compatibility details
 [#CLICKHOUSE-2].

---
 .../AggregateFunctionQuantile.cpp             | 56 +++++++++----------
 .../AggregateFunctionQuantile.h               | 16 +++---
 dbms/src/AggregateFunctions/QuantileExact.h   |  4 +-
 .../QuantileExactWeighted.h                   |  4 +-
 .../QuantileReservoirSampler.h                |  4 +-
 .../QuantileReservoirSamplerDeterministic.h   |  4 +-
 dbms/src/AggregateFunctions/QuantileTDigest.h |  6 +-
 ...quantiles_empty_levels_segfaults.reference |  8 ---
 ...00382_quantiles_empty_levels_segfaults.sql |  8 ---
 9 files changed, 48 insertions(+), 62 deletions(-)
 delete mode 100644 dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.reference
 delete mode 100644 dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.sql

diff --git a/dbms/src/AggregateFunctions/AggregateFunctionQuantile.cpp b/dbms/src/AggregateFunctions/AggregateFunctionQuantile.cpp
index d925cd583c9..1978e15ab41 100644
--- a/dbms/src/AggregateFunctions/AggregateFunctionQuantile.cpp
+++ b/dbms/src/AggregateFunctions/AggregateFunctionQuantile.cpp
@@ -26,7 +26,7 @@ namespace ErrorCodes
 namespace
 {
 
-template <template <typename> class Data, typename Name, bool have_second_arg, bool returns_float, bool returns_many>
+template <template <typename> class Data, typename Name, bool have_second_arg, typename FloatReturnType, bool returns_many>
 AggregateFunctionPtr createAggregateFunctionQuantile(const std::string & name, const DataTypes & argument_types, const Array & params)
 {
     if (have_second_arg)
@@ -38,17 +38,17 @@ AggregateFunctionPtr createAggregateFunctionQuantile(const std::string & name, c
 
 #define CREATE(TYPE) \
     if (typeid_cast<const DataType ## TYPE *>(argument_type.get())) \
-        return std::make_shared<AggregateFunctionQuantile<TYPE, Data<TYPE>, Name, have_second_arg, returns_float, returns_many>>(argument_type, params);
+        return std::make_shared<AggregateFunctionQuantile<TYPE, Data<TYPE>, Name, have_second_arg, FloatReturnType, returns_many>>(argument_type, params);
 
     FOR_NUMERIC_TYPES(CREATE)
 #undef CREATE
 
     if (typeid_cast<const DataTypeDate *>(argument_type.get()))
         return std::make_shared<AggregateFunctionQuantile<
-            DataTypeDate::FieldType, Data<DataTypeDate::FieldType>, Name, have_second_arg, false, returns_many>>(argument_type, params);
+            DataTypeDate::FieldType, Data<DataTypeDate::FieldType>, Name, have_second_arg, void, returns_many>>(argument_type, params);
     if (typeid_cast<const DataTypeDateTime *>(argument_type.get()))
         return std::make_shared<AggregateFunctionQuantile<
-            DataTypeDateTime::FieldType, Data<DataTypeDateTime::FieldType>, Name, have_second_arg, false, returns_many>>(argument_type, params);
+            DataTypeDateTime::FieldType, Data<DataTypeDateTime::FieldType>, Name, have_second_arg, void, returns_many>>(argument_type, params);
 
     throw Exception("Illegal type " + argument_type->getName() + " of argument for aggregate function " + name, ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT);
 }
@@ -81,70 +81,70 @@ struct NameQuantilesTDigestWeighted { static constexpr auto name = "quantilesTDi
 void registerAggregateFunctionsQuantile(AggregateFunctionFactory & factory)
 {
     factory.registerFunction(NameQuantile::name,
-        createAggregateFunctionQuantile<QuantileReservoirSampler, NameQuantile, false, true, false>);
+        createAggregateFunctionQuantile<QuantileReservoirSampler, NameQuantile, false, Float64, false>);
     factory.registerFunction(NameQuantiles::name,
-        createAggregateFunctionQuantile<QuantileReservoirSampler, NameQuantiles, false, true, true>);
+        createAggregateFunctionQuantile<QuantileReservoirSampler, NameQuantiles, false, Float64, true>);
 
     factory.registerFunction(NameQuantileDeterministic::name,
-        createAggregateFunctionQuantile<QuantileReservoirSamplerDeterministic, NameQuantileDeterministic, true, true, false>);
+        createAggregateFunctionQuantile<QuantileReservoirSamplerDeterministic, NameQuantileDeterministic, true, Float64, false>);
     factory.registerFunction(NameQuantilesDeterministic::name,
-        createAggregateFunctionQuantile<QuantileReservoirSamplerDeterministic, NameQuantilesDeterministic, true, true, true>);
+        createAggregateFunctionQuantile<QuantileReservoirSamplerDeterministic, NameQuantilesDeterministic, true, Float64, true>);
 
     factory.registerFunction(NameQuantileExact::name,
-        createAggregateFunctionQuantile<QuantileExact, NameQuantileExact, false, false, false>);
+        createAggregateFunctionQuantile<QuantileExact, NameQuantileExact, false, void, false>);
     factory.registerFunction(NameQuantilesExact::name,
-        createAggregateFunctionQuantile<QuantileExact, NameQuantilesExact, false, false, true>);
+        createAggregateFunctionQuantile<QuantileExact, NameQuantilesExact, false, void, true>);
 
     factory.registerFunction(NameQuantileExactWeighted::name,
-        createAggregateFunctionQuantile<QuantileExactWeighted, NameQuantileExactWeighted, true, false, false>);
+        createAggregateFunctionQuantile<QuantileExactWeighted, NameQuantileExactWeighted, true, void, false>);
     factory.registerFunction(NameQuantilesExactWeighted::name,
-        createAggregateFunctionQuantile<QuantileExactWeighted, NameQuantilesExactWeighted, true, false, true>);
+        createAggregateFunctionQuantile<QuantileExactWeighted, NameQuantilesExactWeighted, true, void, true>);
 
     factory.registerFunction(NameQuantileTiming::name,
-        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTiming, false, true, false>);
+        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTiming, false, Float32, false>);
     factory.registerFunction(NameQuantilesTiming::name,
-        createAggregateFunctionQuantile<QuantileTiming, NameQuantilesTiming, false, true, true>);
+        createAggregateFunctionQuantile<QuantileTiming, NameQuantilesTiming, false, Float32, true>);
 
     factory.registerFunction(NameQuantileTimingWeighted::name,
-        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTimingWeighted, true, true, false>);
+        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTimingWeighted, true, Float32, false>);
     factory.registerFunction(NameQuantilesTimingWeighted::name,
-        createAggregateFunctionQuantile<QuantileTiming, NameQuantilesTimingWeighted, true, true, true>);
+        createAggregateFunctionQuantile<QuantileTiming, NameQuantilesTimingWeighted, true, Float32, true>);
 
     factory.registerFunction(NameQuantileTDigest::name,
-        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigest, false, true, false>);
+        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigest, false, Float32, false>);
     factory.registerFunction(NameQuantilesTDigest::name,
-        createAggregateFunctionQuantile<QuantileTDigest, NameQuantilesTDigest, false, true, true>);
+        createAggregateFunctionQuantile<QuantileTDigest, NameQuantilesTDigest, false, Float32, true>);
 
     factory.registerFunction(NameQuantileTDigestWeighted::name,
-        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigestWeighted, true, true, false>);
+        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigestWeighted, true, Float32, false>);
     factory.registerFunction(NameQuantilesTDigestWeighted::name,
-        createAggregateFunctionQuantile<QuantileTDigest, NameQuantilesTDigestWeighted, true, true, true>);
+        createAggregateFunctionQuantile<QuantileTDigest, NameQuantilesTDigestWeighted, true, Float32, true>);
 
     /// 'median' is an alias for 'quantile'
 
     factory.registerFunction("median",
-        createAggregateFunctionQuantile<QuantileReservoirSampler, NameQuantile, false, true, false>);
+        createAggregateFunctionQuantile<QuantileReservoirSampler, NameQuantile, false, Float64, false>);
 
     factory.registerFunction("medianDeterministic",
-        createAggregateFunctionQuantile<QuantileReservoirSamplerDeterministic, NameQuantileDeterministic, true, true, false>);
+        createAggregateFunctionQuantile<QuantileReservoirSamplerDeterministic, NameQuantileDeterministic, true, Float64, false>);
 
     factory.registerFunction("medianExact",
-        createAggregateFunctionQuantile<QuantileExact, NameQuantileExact, false, false, false>);
+        createAggregateFunctionQuantile<QuantileExact, NameQuantileExact, false, void, false>);
 
     factory.registerFunction("medianExactWeighted",
-        createAggregateFunctionQuantile<QuantileExactWeighted, NameQuantileExactWeighted, true, false, false>);
+        createAggregateFunctionQuantile<QuantileExactWeighted, NameQuantileExactWeighted, true, void, false>);
 
     factory.registerFunction("medianTiming",
-        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTiming, false, false, false>);
+        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTiming, false, Float32, false>);
 
     factory.registerFunction("medianTimingWeighted",
-        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTimingWeighted, true, false, false>);
+        createAggregateFunctionQuantile<QuantileTiming, NameQuantileTimingWeighted, true, Float32, false>);
 
     factory.registerFunction("medianTDigest",
-        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigest, false, true, false>);
+        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigest, false, Float32, false>);
 
     factory.registerFunction("medianTDigestWeighted",
-        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigestWeighted, true, true, false>);
+        createAggregateFunctionQuantile<QuantileTDigest, NameQuantileTDigestWeighted, true, Float32, false>);
 }
 
 }
diff --git a/dbms/src/AggregateFunctions/AggregateFunctionQuantile.h b/dbms/src/AggregateFunctions/AggregateFunctionQuantile.h
index 0eb176b5dd7..7e806f52571 100644
--- a/dbms/src/AggregateFunctions/AggregateFunctionQuantile.h
+++ b/dbms/src/AggregateFunctions/AggregateFunctionQuantile.h
@@ -39,18 +39,20 @@ template <
     /// (in can be "weight" to calculate quantiles or "determinator" that is used instead of PRNG).
     /// Second argument is always obtained through 'getUInt' method.
     bool have_second_arg,
-    /// If true, the function will return float with possibly interpolated results and NaN if there was no values.
+    /// If non-void, the function will return float of specified type with possibly interpolated results and NaN if there was no values.
     /// Otherwise it will return Value type and default value if there was no values.
     /// As an example, the function cannot return floats, if the SQL type of argument is Date or DateTime.
-    bool returns_float,
+    typename FloatReturnType,
     /// If true, the function will accept multiple parameters with quantile levels
     ///  and return an Array filled with many values of that quantiles.
     bool returns_many
 >
 class AggregateFunctionQuantile final : public IAggregateFunctionDataHelper<Data,
-    AggregateFunctionQuantile<Value, Data, Name, have_second_arg, returns_float, returns_many>>
+    AggregateFunctionQuantile<Value, Data, Name, have_second_arg, FloatReturnType, returns_many>>
 {
 private:
+    static constexpr bool returns_float = !std::is_same_v<FloatReturnType, void>;
+
     QuantileLevels<Float64> levels;
 
     /// Used when there are single level to get.
@@ -73,7 +75,7 @@ public:
         DataTypePtr res;
 
         if constexpr (returns_float)
-            res = std::make_shared<DataTypeFloat32>();
+            res = std::make_shared<DataTypeNumber<FloatReturnType>>();
         else
             res = argument_type;
 
@@ -128,7 +130,7 @@ public:
 
             if constexpr (returns_float)
             {
-                typename ColumnFloat32::Container & data_to = static_cast<ColumnFloat32 &>(arr_to.getData()).getData();
+                auto & data_to = static_cast<ColumnVector<FloatReturnType> &>(arr_to.getData()).getData();
                 size_t old_size = data_to.size();
                 data_to.resize(data_to.size() + size);
 
@@ -136,7 +138,7 @@ public:
             }
             else
             {
-                typename ColumnVector<Value>::Container & data_to = static_cast<ColumnVector<Value> &>(arr_to.getData()).getData();
+                auto & data_to = static_cast<ColumnVector<Value> &>(arr_to.getData()).getData();
                 size_t old_size = data_to.size();
                 data_to.resize(data_to.size() + size);
 
@@ -146,7 +148,7 @@ public:
         else
         {
             if constexpr (returns_float)
-                static_cast<ColumnFloat32 &>(to).getData().push_back(data.getFloat(level));
+                static_cast<ColumnVector<FloatReturnType> &>(to).getData().push_back(data.getFloat(level));
             else
                 static_cast<ColumnVector<Value> &>(to).getData().push_back(data.get(level));
         }
diff --git a/dbms/src/AggregateFunctions/QuantileExact.h b/dbms/src/AggregateFunctions/QuantileExact.h
index ee0360fec1f..122dcd0c04c 100644
--- a/dbms/src/AggregateFunctions/QuantileExact.h
+++ b/dbms/src/AggregateFunctions/QuantileExact.h
@@ -100,12 +100,12 @@ struct QuantileExact
     }
 
     /// The same, but in the case of an empty state, NaN is returned.
-    float getFloat(Float64) const
+    Float64 getFloat(Float64) const
     {
         throw Exception("Method getFloat is not implemented for QuantileExact", ErrorCodes::NOT_IMPLEMENTED);
     }
 
-    void getManyFloat(const Float64 *, const size_t *, size_t, float *) const
+    void getManyFloat(const Float64 *, const size_t *, size_t, Float64 *) const
     {
         throw Exception("Method getManyFloat is not implemented for QuantileExact", ErrorCodes::NOT_IMPLEMENTED);
     }
diff --git a/dbms/src/AggregateFunctions/QuantileExactWeighted.h b/dbms/src/AggregateFunctions/QuantileExactWeighted.h
index 8c0736e8cf6..f524426392a 100644
--- a/dbms/src/AggregateFunctions/QuantileExactWeighted.h
+++ b/dbms/src/AggregateFunctions/QuantileExactWeighted.h
@@ -170,12 +170,12 @@ struct QuantileExactWeighted
     }
 
     /// The same, but in the case of an empty state, NaN is returned.
-    float getFloat(Float64) const
+    Float64 getFloat(Float64) const
     {
         throw Exception("Method getFloat is not implemented for QuantileExact", ErrorCodes::NOT_IMPLEMENTED);
     }
 
-    void getManyFloat(const Float64 *, const size_t *, size_t, float *) const
+    void getManyFloat(const Float64 *, const size_t *, size_t, Float64 *) const
     {
         throw Exception("Method getManyFloat is not implemented for QuantileExact", ErrorCodes::NOT_IMPLEMENTED);
     }
diff --git a/dbms/src/AggregateFunctions/QuantileReservoirSampler.h b/dbms/src/AggregateFunctions/QuantileReservoirSampler.h
index b6a8bed9fa2..6b4af04a5fb 100644
--- a/dbms/src/AggregateFunctions/QuantileReservoirSampler.h
+++ b/dbms/src/AggregateFunctions/QuantileReservoirSampler.h
@@ -66,12 +66,12 @@ struct QuantileReservoirSampler
     }
 
     /// The same, but in the case of an empty state, NaN is returned.
-    float getFloat(Float64 level)
+    Float64 getFloat(Float64 level)
     {
         return data.quantileInterpolated(level);
     }
 
-    void getManyFloat(const Float64 * levels, const size_t * indices, size_t size, float * result)
+    void getManyFloat(const Float64 * levels, const size_t * indices, size_t size, Float64 * result)
     {
         for (size_t i = 0; i < size; ++i)
             result[indices[i]] = data.quantileInterpolated(levels[indices[i]]);
diff --git a/dbms/src/AggregateFunctions/QuantileReservoirSamplerDeterministic.h b/dbms/src/AggregateFunctions/QuantileReservoirSamplerDeterministic.h
index 9ab9b834306..303e6518e9c 100644
--- a/dbms/src/AggregateFunctions/QuantileReservoirSamplerDeterministic.h
+++ b/dbms/src/AggregateFunctions/QuantileReservoirSamplerDeterministic.h
@@ -66,12 +66,12 @@ struct QuantileReservoirSamplerDeterministic
     }
 
     /// The same, but in the case of an empty state, NaN is returned.
-    float getFloat(Float64 level)
+    Float64 getFloat(Float64 level)
     {
         return data.quantileInterpolated(level);
     }
 
-    void getManyFloat(const Float64 * levels, const size_t * indices, size_t size, float * result)
+    void getManyFloat(const Float64 * levels, const size_t * indices, size_t size, Float64 * result)
     {
         for (size_t i = 0; i < size; ++i)
             result[indices[i]] = data.quantileInterpolated(levels[indices[i]]);
diff --git a/dbms/src/AggregateFunctions/QuantileTDigest.h b/dbms/src/AggregateFunctions/QuantileTDigest.h
index 29cf99cd901..d23a8dbc878 100644
--- a/dbms/src/AggregateFunctions/QuantileTDigest.h
+++ b/dbms/src/AggregateFunctions/QuantileTDigest.h
@@ -316,9 +316,9 @@ public:
         return getImpl<T>(level);
     }
 
-    float getFloat(Float64 level)
+    Float32 getFloat(Float64 level)
     {
-        return getImpl<float>(level);
+        return getImpl<Float32>(level);
     }
 
     void getMany(const Float64 * levels, const size_t * indices, size_t size, T * result)
@@ -326,7 +326,7 @@ public:
         getManyImpl(levels, indices, size, result);
     }
 
-    void getManyFloat(const Float64 * levels, const size_t * indices, size_t size, float * result)
+    void getManyFloat(const Float64 * levels, const size_t * indices, size_t size, Float32 * result)
     {
         getManyImpl(levels, indices, size, result);
     }
diff --git a/dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.reference b/dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.reference
deleted file mode 100644
index 3804f8a692d..00000000000
--- a/dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.reference
+++ /dev/null
@@ -1,8 +0,0 @@
-[]
-[]
-[]
-[]
-[]
-[]
-[]
-[]
diff --git a/dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.sql b/dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.sql
deleted file mode 100644
index dfacd78b962..00000000000
--- a/dbms/tests/queries/0_stateless/00382_quantiles_empty_levels_segfaults.sql
+++ /dev/null
@@ -1,8 +0,0 @@
-select quantiles(number) as q from (select * from system.numbers LIMIT 1000);
-select quantilesExact(number) as q from (select * from system.numbers LIMIT 1000);
-select quantilesExactWeighted(number, number) as q from (select * from system.numbers LIMIT 1000);
-select quantilesDeterministic(number, 10000000) as q from (select * from system.numbers LIMIT 1000);
-select quantilesTiming(number) as q from (select * from system.numbers LIMIT 1000);
-select quantilesTimingWeighted(number, number) as q from (select * from system.numbers LIMIT 1000);
-select quantilesTDigest(number) as q from (select * from system.numbers LIMIT 1000);
-select quantilesTDigestWeighted(number, number) as q from (select * from system.numbers LIMIT 1000);