fix review

This commit is contained in:
Mikhail Surin 2018-06-22 17:56:53 +03:00
parent 021f3b564b
commit 0188538b33
2 changed files with 26 additions and 17 deletions

View File

@ -7,27 +7,36 @@
namespace DB { namespace DB {
namespace ErrorCodes
{
extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;
extern const int ILLEGAL_TYPE_OF_ARGUMENT;
extern const int BAD_ARGUMENTS;
}
namespace {
AggregateFunctionPtr createAggregateFunctionHistogram(const std::string & name, const DataTypes & arguments, const Array & params) AggregateFunctionPtr createAggregateFunctionHistogram(const std::string & name, const DataTypes & arguments, const Array & params)
{ {
if (params.size() != 1) if (params.size() != 1)
{ throw Exception("Function " + name + " requires bins count", ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH);
throw Exception("Function " + name + " requires only bins count");
} UInt32 bins_count = applyVisitor(FieldVisitorConvertToNumber<UInt32>(), params[0]);
if (bins_count == 0)
throw Exception("Bin count should be positive", ErrorCodes::BAD_ARGUMENTS);
assertUnary(name, arguments); assertUnary(name, arguments);
UInt32 bins_count;
#define READ(VAL, PARAM) \
VAL = applyVisitor(FieldVisitorConvertToNumber<decltype(VAL)>(), PARAM);
READ(bins_count, params[0]);
#undef READ
AggregateFunctionPtr res(createWithNumericType<AggregateFunctionHistogram>(*arguments[0], bins_count)); AggregateFunctionPtr res(createWithNumericType<AggregateFunctionHistogram>(*arguments[0], bins_count));
if (!res)
throw Exception("Illegal type " + arguments[0]->getName() + " of argument for aggregate function " + name, ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT);
return res; return res;
} }
}
void registerAggregateFunctionHistogram(AggregateFunctionFactory & factory) void registerAggregateFunctionHistogram(AggregateFunctionFactory & factory)
{ {
factory.registerFunction("histogram", createAggregateFunctionHistogram); factory.registerFunction("histogram", createAggregateFunctionHistogram);

View File

@ -86,11 +86,11 @@ private:
// Maintain doubly-linked list of "active" points // Maintain doubly-linked list of "active" points
// and store neighbour pairs in priority queue by distance // and store neighbour pairs in priority queue by distance
AutoArray<int> previous(size + 1); AutoArray<UInt32> previous(size + 1);
AutoArray<int> next(size + 1); AutoArray<UInt32> next(size + 1);
AutoArray<bool> active(size + 1, true); AutoArray<bool> active(size + 1, true);
active[size] = false; active[size] = false;
auto delete_node = [&](int i) auto delete_node = [&](UInt32 i)
{ {
previous[next[i]] = previous[i]; previous[next[i]] = previous[i];
next[previous[i]] = next[i]; next[previous[i]] = next[i];
@ -104,13 +104,13 @@ private:
next[size] = 0; next[size] = 0;
previous[0] = size; previous[0] = size;
using QueueItem = std::pair<Mean, int>; using QueueItem = std::pair<Mean, UInt32>;
std::vector<QueueItem> storage; std::vector<QueueItem> storage;
storage.reserve(2 * size - max_bins); storage.reserve(2 * size - max_bins);
std::priority_queue<QueueItem, std::vector<QueueItem>, std::greater<QueueItem>> queue; std::priority_queue<QueueItem, std::vector<QueueItem>, std::greater<QueueItem>> queue;
auto quality = [&](int i) { return points[next[i]].mean - points[i].mean; }; auto quality = [&](UInt32 i) { return points[next[i]].mean - points[i].mean; };
for (size_t i = 0; i + 1 < size; i++) for (size_t i = 0; i + 1 < size; i++)
queue.push({quality(i), i}); queue.push({quality(i), i});