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.
This commit is contained in:
Azat Khuzhin 2021-04-11 21:59:49 +03:00
parent ce9fd8a091
commit 0bf6b61b59
3 changed files with 26 additions and 8 deletions

View File

@ -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 {};
}

View File

@ -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:

View File

@ -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());
}
}