From 0d6094a3eae8c0268fa66064bf92088dfe2db322 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C3=A9o=20Ercolanelli?= <leo.ercolanelli@contentsquare.com>
Date: Fri, 25 Jan 2019 14:08:16 +0100
Subject: [PATCH] sumMap: return types less prone to oveflows

It used to be that sumMap would return the same type as the values
columns. If columns of Array(UInt8) were to be given, that would really
easily cause oveflow. It now uses `getWidenDataType` (and ultimately
`NearestFieldType`) in order to define the result type.
---
 .../src/AggregateFunctions/AggregateFunctionSumMap.h | 12 +++++++++++-
 .../queries/0_stateless/00502_sum_map.reference      |  1 +
 dbms/tests/queries/0_stateless/00502_sum_map.sql     |  9 +++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/dbms/src/AggregateFunctions/AggregateFunctionSumMap.h b/dbms/src/AggregateFunctions/AggregateFunctionSumMap.h
index 1e5f3e38cd2..8c7c24faed5 100644
--- a/dbms/src/AggregateFunctions/AggregateFunctionSumMap.h
+++ b/dbms/src/AggregateFunctions/AggregateFunctionSumMap.h
@@ -72,7 +72,7 @@ public:
         types.emplace_back(std::make_shared<DataTypeArray>(keys_type));
 
         for (const auto & value_type : values_types)
-            types.emplace_back(std::make_shared<DataTypeArray>(value_type));
+            types.emplace_back(std::make_shared<DataTypeArray>(widenDataType(value_type)));
 
         return std::make_shared<DataTypeTuple>(types);
     }
@@ -260,6 +260,16 @@ public:
     const char * getHeaderFilePath() const override { return __FILE__; }
 
     bool keepKey(const T & key) const { return static_cast<const Derived &>(*this).keepKey(key); }
+
+private:
+    static DataTypePtr widenDataType(const DataTypePtr & data_type)
+    {
+        if (!data_type->canBeWiden())
+            throw new Exception{"Values to be summed are expected to be Numeric, Float or Decimal.",
+                ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT};
+
+        return data_type->getWidenDataType();
+    }
 };
 
 template <typename T>
diff --git a/dbms/tests/queries/0_stateless/00502_sum_map.reference b/dbms/tests/queries/0_stateless/00502_sum_map.reference
index 7bb325be814..a8d9fe95af3 100644
--- a/dbms/tests/queries/0_stateless/00502_sum_map.reference
+++ b/dbms/tests/queries/0_stateless/00502_sum_map.reference
@@ -10,6 +10,7 @@
 2000-01-01 00:01:00	[4,5,6,7,8]	[10,10,20,10,10]
 ([1],[10])
 ([1,4,8],[10,20,10])
+([1],[257])
 ([1],[1])
 ([1],[1])
 (['a'],[1])
diff --git a/dbms/tests/queries/0_stateless/00502_sum_map.sql b/dbms/tests/queries/0_stateless/00502_sum_map.sql
index 9cf941dd908..24eab44d3d0 100644
--- a/dbms/tests/queries/0_stateless/00502_sum_map.sql
+++ b/dbms/tests/queries/0_stateless/00502_sum_map.sql
@@ -17,6 +17,15 @@ SELECT sumMapFiltered([1, 4, 8])(statusMap.status, statusMap.requests) FROM test
 
 DROP TABLE test.sum_map;
 
+DROP TABLE IF EXISTS test.sum_map_overflow;
+CREATE TABLE test.sum_map_overflow(events Array(UInt8), counts Array(UInt8)) ENGINE = Log;
+
+INSERT INTO test.sum_map_overflow VALUES ([1], [255]), ([1], [2]);
+
+SELECT sumMap(events, counts) FROM test.sum_map_overflow;
+
+DROP TABLE test.sum_map_overflow;
+
 select sumMap(val, cnt) from ( SELECT [ CAST(1, 'UInt64') ] as val, [1] as cnt );
 select sumMap(val, cnt) from ( SELECT [ CAST(1, 'Float64') ] as val, [1] as cnt );
 select sumMap(val, cnt) from ( SELECT [ CAST('a', 'Enum16(\'a\'=1)') ] as val, [1] as cnt );