From 0bf6b61b590719e4f0d17dc7b9171d6f8cdd7c29 Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Sun, 11 Apr 2021 21:59:49 +0300
Subject: [PATCH] Fix combinators with common prefix name (State and
 SimpleState) with libstdc++

Previously sort order of the std::unordered_map in libstdc++ was
different and any *SimpleState() reports an error that function does not
exists.

Fix this by using proper order in container, and use std::vector over
std::unordered_map, since there linear traversing anyway in the single
method -- tryFindSuffix()

Note that test is not required, since it either fail with unknown
function or not.
---
 .../AggregateFunctionCombinatorFactory.cpp     | 18 +++++++++++++-----
 .../AggregateFunctionCombinatorFactory.h       | 12 +++++++++++-
 ...orageSystemAggregateFunctionCombinators.cpp |  4 ++--
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/AggregateFunctions/AggregateFunctionCombinatorFactory.cpp b/src/AggregateFunctions/AggregateFunctionCombinatorFactory.cpp
index a20d355bb2f..e4ff8c134c5 100644
--- a/src/AggregateFunctions/AggregateFunctionCombinatorFactory.cpp
+++ b/src/AggregateFunctions/AggregateFunctionCombinatorFactory.cpp
@@ -13,17 +13,25 @@ namespace ErrorCodes
 
 void AggregateFunctionCombinatorFactory::registerCombinator(const AggregateFunctionCombinatorPtr & value)
 {
-    if (!dict.emplace(value->getName(), value).second)
-        throw Exception("AggregateFunctionCombinatorFactory: the name '" + value->getName() + "' is not unique",
-            ErrorCodes::LOGICAL_ERROR);
+    CombinatorPair pair{
+        .name = value->getName(),
+        .combinator_ptr = value,
+    };
+
+    /// lower_bound() cannot be used since sort order of the dict is by length of the combinator
+    /// but there are just a few combiners, so not a problem.
+    if (std::find(dict.begin(), dict.end(), pair) != dict.end())
+        throw Exception(ErrorCodes::LOGICAL_ERROR, "AggregateFunctionCombinatorFactory: the name '{}' is not unique",
+            value->getName());
+    dict.emplace(std::lower_bound(dict.begin(), dict.end(), pair), pair);
 }
 
 AggregateFunctionCombinatorPtr AggregateFunctionCombinatorFactory::tryFindSuffix(const std::string & name) const
 {
     /// O(N) is ok for just a few combinators.
     for (const auto & suffix_value : dict)
-        if (endsWith(name, suffix_value.first))
-            return suffix_value.second;
+        if (endsWith(name, suffix_value.name))
+            return suffix_value.combinator_ptr;
     return {};
 }
 
diff --git a/src/AggregateFunctions/AggregateFunctionCombinatorFactory.h b/src/AggregateFunctions/AggregateFunctionCombinatorFactory.h
index b535475d111..5f7658c16af 100644
--- a/src/AggregateFunctions/AggregateFunctionCombinatorFactory.h
+++ b/src/AggregateFunctions/AggregateFunctionCombinatorFactory.h
@@ -15,7 +15,17 @@ namespace DB
 class AggregateFunctionCombinatorFactory final: private boost::noncopyable
 {
 private:
-    using Dict = std::unordered_map<std::string, AggregateFunctionCombinatorPtr>;
+    struct CombinatorPair
+    {
+        std::string name;
+        AggregateFunctionCombinatorPtr combinator_ptr;
+
+        bool operator==(const CombinatorPair & rhs) const { return name == rhs.name; }
+        /// Sort by the length of the combinator name for proper tryFindSuffix()
+        /// for combiners with common prefix (i.e. "State" and "SimpleState").
+        bool operator<(const CombinatorPair & rhs) const { return name.length() > rhs.name.length(); }
+    };
+    using Dict = std::vector<CombinatorPair>;
     Dict dict;
 
 public:
diff --git a/src/Storages/System/StorageSystemAggregateFunctionCombinators.cpp b/src/Storages/System/StorageSystemAggregateFunctionCombinators.cpp
index c1de054ffcd..c2d82c6cd7c 100644
--- a/src/Storages/System/StorageSystemAggregateFunctionCombinators.cpp
+++ b/src/Storages/System/StorageSystemAggregateFunctionCombinators.cpp
@@ -17,8 +17,8 @@ void StorageSystemAggregateFunctionCombinators::fillData(MutableColumns & res_co
     const auto & combinators = AggregateFunctionCombinatorFactory::instance().getAllAggregateFunctionCombinators();
     for (const auto & pair : combinators)
     {
-        res_columns[0]->insert(pair.first);
-        res_columns[1]->insert(pair.second->isForInternalUsageOnly());
+        res_columns[0]->insert(pair.name);
+        res_columns[1]->insert(pair.combinator_ptr->isForInternalUsageOnly());
     }
 }