xiaokang commented on code in PR #39546:
URL: https://github.com/apache/doris/pull/39546#discussion_r1740265914
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinAggregateFunctions.java:
##########
@@ -115,6 +116,7 @@ public class BuiltinAggregateFunctions implements
FunctionHelper {
agg(HllUnion.class, "hll_raw_agg", "hll_union"),
agg(HllUnionAgg.class, "hll_union_agg"),
agg(IntersectCount.class, "intersect_count"),
+ agg(LinearHistogram.class, "linear_histogram"),
Review Comment:
FunctionSet.LINEAR_HISTOGRAM
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/LinearHistogram.java:
##########
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.expressions.functions.agg;
+
+import org.apache.doris.catalog.FunctionSet;
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNotNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import org.apache.doris.nereids.trees.expressions.functions.SearchSignature;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.DoubleType;
+import org.apache.doris.nereids.types.VarcharType;
+import org.apache.doris.nereids.types.coercion.AnyDataType;
+import org.apache.doris.nereids.types.coercion.PrimitiveType;
+
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+
+/**
+ * AggregateFunction 'linear_histogram'.
+ */
+public class LinearHistogram extends AggregateFunction implements
ExplicitlyCastableSignature, AlwaysNotNullable {
+
+ public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .args(AnyDataType.INSTANCE_WITHOUT_INDEX, DoubleType.INSTANCE),
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .args(AnyDataType.INSTANCE_WITHOUT_INDEX, DoubleType.INSTANCE,
DoubleType.INSTANCE)
+ );
+
+ private LinearHistogram(boolean distinct, List<Expression> args) {
+ super(FunctionSet.LINEAR_HISTOGRAM, distinct, args);
+ }
+
+ public LinearHistogram(Expression arg0, Expression arg1) {
+ super(FunctionSet.LINEAR_HISTOGRAM, arg0, arg1);
+ }
+
+ public LinearHistogram(Expression arg0, Expression arg1, Expression arg2) {
+ super(FunctionSet.LINEAR_HISTOGRAM, arg0, arg1, arg2);
+ }
+
+ public LinearHistogram(boolean distinct, Expression arg0, Expression arg1)
{
Review Comment:
move function with distinct together.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/LinearHistogram.java:
##########
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.expressions.functions.agg;
+
+import org.apache.doris.catalog.FunctionSet;
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNotNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import org.apache.doris.nereids.trees.expressions.functions.SearchSignature;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.DoubleType;
+import org.apache.doris.nereids.types.VarcharType;
+import org.apache.doris.nereids.types.coercion.AnyDataType;
+import org.apache.doris.nereids.types.coercion.PrimitiveType;
+
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+
+/**
+ * AggregateFunction 'linear_histogram'.
+ */
+public class LinearHistogram extends AggregateFunction implements
ExplicitlyCastableSignature, AlwaysNotNullable {
+
+ public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .args(AnyDataType.INSTANCE_WITHOUT_INDEX, DoubleType.INSTANCE),
Review Comment:
In fact, this function requires numeric type instead of AnyDataType.
##########
be/src/vec/aggregate_functions/aggregate_function_linear_histogram.cpp:
##########
@@ -0,0 +1,61 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "vec/aggregate_functions/aggregate_function_linear_histogram.h"
+
+#include "vec/aggregate_functions/helpers.h"
+
+namespace doris::vectorized {
+
+template <typename T>
+AggregateFunctionPtr create_agg_function_linear_histogram(const DataTypes&
argument_types,
+ const bool
result_is_nullable) {
+ bool has_offset = (argument_types.size() == 3);
+
+ if (has_offset) {
+ return creator_without_type::create<
+ AggregateFunctionLinearHistogram<T,
AggregateFunctionLinearHistogramData<T>, true>>(
+ argument_types, result_is_nullable);
+ } else {
+ return creator_without_type::create<AggregateFunctionLinearHistogram<
+ T, AggregateFunctionLinearHistogramData<T>,
false>>(argument_types,
+
result_is_nullable);
+ }
+}
+
+AggregateFunctionPtr create_aggregate_function_linear_histogram(const
std::string& name,
+ const
DataTypes& argument_types,
+ const bool
result_is_nullable) {
+ WhichDataType type(remove_nullable(argument_types[0]));
+
+#define DISPATCH(TYPE) \
+ if (type.idx == TypeIndex::TYPE) \
+ return create_agg_function_linear_histogram<TYPE>(argument_types,
result_is_nullable);
+ FOR_NUMERIC_TYPES(DISPATCH)
+ FOR_DECIMAL_TYPES(DISPATCH)
+#undef DISPATCH
+
+ LOG(WARNING) << fmt::format("unsupported input type {} for aggregate
function {}",
+ argument_types[0]->get_name(), name);
+ return nullptr;
+}
+
+void
register_aggregate_function_linear_histogram(AggregateFunctionSimpleFactory&
factory) {
+ factory.register_function_both("linear_histogram",
create_aggregate_function_linear_histogram);
Review Comment:
define and use a const for "linear_histogram"
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/LinearHistogram.java:
##########
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.expressions.functions.agg;
+
+import org.apache.doris.catalog.FunctionSet;
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNotNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import org.apache.doris.nereids.trees.expressions.functions.SearchSignature;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.DoubleType;
+import org.apache.doris.nereids.types.VarcharType;
+import org.apache.doris.nereids.types.coercion.AnyDataType;
+import org.apache.doris.nereids.types.coercion.PrimitiveType;
+
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+
+/**
+ * AggregateFunction 'linear_histogram'.
+ */
+public class LinearHistogram extends AggregateFunction implements
ExplicitlyCastableSignature, AlwaysNotNullable {
Review Comment:
I think if input arg is NULL, the result should be NULL. So this function
should not be AlwaysNotNullable.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/LinearHistogram.java:
##########
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.nereids.trees.expressions.functions.agg;
+
+import org.apache.doris.catalog.FunctionSet;
+import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.functions.AlwaysNotNullable;
+import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
+import org.apache.doris.nereids.trees.expressions.functions.SearchSignature;
+import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
+import org.apache.doris.nereids.types.DoubleType;
+import org.apache.doris.nereids.types.VarcharType;
+import org.apache.doris.nereids.types.coercion.AnyDataType;
+import org.apache.doris.nereids.types.coercion.PrimitiveType;
+
+import com.google.common.collect.ImmutableList;
+
+import java.util.List;
+
+/**
+ * AggregateFunction 'linear_histogram'.
+ */
+public class LinearHistogram extends AggregateFunction implements
ExplicitlyCastableSignature, AlwaysNotNullable {
+
+ public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .args(AnyDataType.INSTANCE_WITHOUT_INDEX, DoubleType.INSTANCE),
+ FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
+ .args(AnyDataType.INSTANCE_WITHOUT_INDEX, DoubleType.INSTANCE,
DoubleType.INSTANCE)
+ );
+
+ private LinearHistogram(boolean distinct, List<Expression> args) {
+ super(FunctionSet.LINEAR_HISTOGRAM, distinct, args);
+ }
+
+ public LinearHistogram(Expression arg0, Expression arg1) {
+ super(FunctionSet.LINEAR_HISTOGRAM, arg0, arg1);
+ }
+
+ public LinearHistogram(Expression arg0, Expression arg1, Expression arg2) {
+ super(FunctionSet.LINEAR_HISTOGRAM, arg0, arg1, arg2);
+ }
+
+ public LinearHistogram(boolean distinct, Expression arg0, Expression arg1)
{
+ super(FunctionSet.LINEAR_HISTOGRAM, distinct, arg0, arg1);
+ }
+
+ public LinearHistogram(boolean distinct, Expression arg0, Expression arg1,
Expression arg2) {
+ super(FunctionSet.LINEAR_HISTOGRAM, distinct, arg0, arg1, arg2);
+ }
+
+ @Override
+ public void checkLegalityBeforeTypeCoercion() {
+ if (!(child(0).getDataType() instanceof PrimitiveType)) {
+ SearchSignature.throwCanNotFoundFunctionException(this.getName(),
getArguments());
+ }
+ }
+
+ @Override
+ public AggregateFunction withDistinctAndChildren(boolean distinct,
List<Expression> children) {
+ return new LinearHistogram(distinct, children);
+ }
+
+ @Override
+ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
+ return visitor.visitLinearHistogram(this, context);
+ }
+
+ @Override
+ public List<FunctionSignature> getSignatures() {
Review Comment:
move it just after definition of SIGNATURES
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java:
##########
@@ -1985,7 +1985,8 @@ && collectChildReturnTypes()[0].isDecimalV3()) {
if ((fnName.getFunction().equalsIgnoreCase("money_format") ||
fnName.getFunction()
.equalsIgnoreCase("histogram")
- || fnName.getFunction().equalsIgnoreCase("hist"))
+ || fnName.getFunction().equalsIgnoreCase("hist")
+ ||
fnName.getFunction().equalsIgnoreCase("linear_histogram"))
Review Comment:
Is this special logic really necessary?
##########
be/src/vec/aggregate_functions/aggregate_function_linear_histogram.h:
##########
@@ -0,0 +1,243 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <rapidjson/document.h>
+#include <rapidjson/prettywriter.h>
+#include <rapidjson/stringbuffer.h>
+
+#include <map>
+
+#include "vec/aggregate_functions/aggregate_function.h"
+#include "vec/aggregate_functions/aggregate_function_simple_factory.h"
+#include "vec/core/types.h"
+#include "vec/data_types/data_type_decimal.h"
+#include "vec/io/io_helper.h"
+
+// TODO: optimize count=0
+// TODO: support datetime
+// TODO: support foreach
+
+namespace doris::vectorized {
+
+template <typename T>
+struct AggregateFunctionLinearHistogramData {
+ // bucket key limits
+ const static int32_t MIN_BUCKET_KEY = std::numeric_limits<int32_t>::min();
+ const static int32_t MAX_BUCKET_KEY = std::numeric_limits<int32_t>::max();
+
+private:
+ // influxdb use double
+ double interval = 0;
+ double offset;
+ double lower; // not used yet
+ double upper; // not used yet
+ std::map<int32_t, size_t> buckets;
+
+public:
+ // reset
+ void reset() {
+ offset = 0;
+ interval = 0;
+ buckets.clear();
+ }
+
+ void set_parameters(double input_interval, double input_offset) {
+ interval = input_interval;
+ offset = input_offset;
+ }
+
+ // add
+ void add(const T& value, UInt32 scale) {
+ double val = 0;
+ if constexpr (IsDecimalNumber<T>) {
+ using NativeType = typename T::NativeType;
+ val = static_cast<double>(value.value) /
decimal_scale_multiplier<NativeType>(scale);
+ } else {
+ val = static_cast<double>(value);
+ }
+ double key = std::floor((val - offset) / interval);
+ if (key <= MIN_BUCKET_KEY || key >= MAX_BUCKET_KEY) {
+ throw doris::Exception(ErrorCode::INVALID_ARGUMENT, "{} exceeds
the bucket range limit",
+ value);
+ }
+ buckets[static_cast<int32_t>(key)]++;
+ }
+
+ // merge
+ void merge(const AggregateFunctionLinearHistogramData& rhs) {
+ if (rhs.interval == 0) {
+ return;
+ }
+
+ interval = rhs.interval;
+ offset = rhs.offset;
+
+ for (const auto& [key, count] : rhs.buckets) {
+ buckets[key] += count;
+ }
+ }
+
+ // write
+ void write(BufferWritable& buf) const {
+ write_binary(offset, buf);
+ write_binary(interval, buf);
+ write_binary(lower, buf);
+ write_binary(upper, buf);
+ write_binary(buckets.size(), buf);
+ for (const auto& [key, count] : buckets) {
+ write_binary(key, buf);
+ write_binary(count, buf);
+ }
+ }
+
+ // read
+ void read(BufferReadable& buf) {
+ read_binary(offset, buf);
+ read_binary(interval, buf);
+ read_binary(lower, buf);
+ read_binary(upper, buf);
+ size_t size;
+ read_binary(size, buf);
+ for (size_t i = 0; i < size; i++) {
+ int32_t key;
+ size_t count;
+ read_binary(key, buf);
+ read_binary(count, buf);
+ buckets[key] = count;
+ }
+ }
+
+ // insert_result_into
+ void insert_result_into(IColumn& to) const {
+ rapidjson::Document doc;
+ doc.SetObject();
+ rapidjson::Document::AllocatorType& allocator = doc.GetAllocator();
+
+ unsigned num_buckets =
+ buckets.empty() ? 0 : buckets.rbegin()->first -
buckets.begin()->first + 1;
+ doc.AddMember("num_buckets", num_buckets, allocator);
Review Comment:
define and use const for all strings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
