Skip to content

Commit

Permalink
[fix](arrayfuncs) fix array min/max func (apache#39307)
Browse files Browse the repository at this point in the history
in fe we will check the params same with be 
in be we throw exception if params is not support 
otherwise we will meet coredump like this 
```
start BE in local mode
/mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/array/function_array_aggregation.cpp:150:26: runtime error: member call on null pointer of type 'doris::vectorized::IAggregateFunction'
    #0 0x55dc04c6b409 in doris::vectorized::ArrayAggregateImpl<(doris::vectorized::AggregateOperation)1>::get_return_type(std::vector<std::shared_ptr<doris::vectorized::IDataType const>, std::allocator<std::shared_ptr<doris::vectorized::IDataType const>>> const&) /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/array/function_array_aggregation.cpp:150:26
    #1 0x55dc04c6a41f in doris::vectorized::FunctionArrayMapped<doris::vectorized::ArrayAggregateImpl<(doris::vectorized::AggregateOperation)1>, doris::vectorized::NameArrayMax>::get_return_type_impl(std::vector<std::shared_ptr<doris::vectorized::IDataType const>, std::allocator<std::shared_ptr<doris::vectorized::IDataType const>>> const&) const /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/array/function_array_mapped.h:71:16
    #2 0x55dc049b06cb in doris::vectorized::FunctionBuilderImpl::get_return_type_impl(std::vector<doris::vectorized::ColumnWithTypeAndName, std::allocator<doris::vectorized::ColumnWithTypeAndName>> const&) const /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/function.h:339:16
    #3 0x55dc049c1385 in doris::vectorized::DefaultFunctionBuilder::get_return_type_impl(std::vector<doris::vectorized::ColumnWithTypeAndName, std::allocator<doris::vectorized::ColumnWithTypeAndName>> const&) const /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/function.h:579:26
    #4 0x55dc0b0a5652 in doris::vectorized::FunctionBuilderImpl::get_return_type_without_low_cardinality(std::vector<doris::vectorized::ColumnWithTypeAndName, std::allocator<doris::vectorized::ColumnWithTypeAndName>> const&) const /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/function.cpp:283:12
    #5 0x55dc0b0a5fd4 in doris::vectorized::FunctionBuilderImpl::get_return_type(std::vector<doris::vectorized::ColumnWithTypeAndName, std::allocator<doris::vectorized::ColumnWithTypeAndName>> const&) const /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/function.cpp:298:17
    #6 0x55dc04997050 in doris::vectorized::FunctionBuilderImpl::build(std::vector<doris::vectorized::ColumnWithTypeAndName, std::allocator<doris::vectorized::ColumnWithTypeAndName>> const&, std::shared_ptr<doris::vectorized::IDataType const> const&) const /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/function.h:296:47
    #7 0x55dbde3d727f in doris::vectorized::SimpleFunctionFactory::get_function(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, std::vector<doris::vectorized::ColumnWithTypeAndName, std::allocator<doris::vectorized::ColumnWithTypeAndName>> const&, std::shared_ptr<doris::vectorized::IDataType const> const&, int) /mnt/disk1/wangqiannan/amory/doris/be/src/vec/functions/simple_function_factory.h:193:32
    #8 0x55dc04984b4d in doris::vectorized::VectorizedFnCall::prepare(doris::RuntimeState*, doris::RowDescriptor const&, doris::vectorized::VExprContext*) /mnt/disk1/wangqiannan/amory/doris/be/src/vec/exprs/vectorized_fn_call.cpp:111:55
    #9 0x55dc04ab9b7a in doris::vectorized::VExprContext::prepare(doris::RuntimeState*, doris::RowDescriptor const&) /mnt/disk1/wangqiannan/amory/doris/be/src/vec/exprs/vexpr_context.cpp:64:5
    #10 0x55dbdf625845 in doris::Status doris::FoldConstantExecutor::_prepare_and_open<doris::vectorized::VExprContext>(doris::vectorized::VExprContext*) /mnt/disk1/wangqiannan/amory/doris/be/src/runtime/fold_constant_executor.cpp:185:5
    apache#11 0x55dbdf60db76 in doris::FoldConstantExecutor::fold_constant_vexpr(doris::TFoldConstantParams const&, doris::PConstantExprResult*) /mnt/disk1/wangqiannan/amory/doris/be/src/runtime/fold_constant_executor.cpp:94:13
    apache#12 0x55dbdf4f58d6 in doris::PInternalService::_fold_constant_expr(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, doris::PConstantExprResult*) /mnt/disk1/wangqiannan/amory/doris/be/src/service/internal_service.cpp:1498:5
    apache#13 0x55dbdf544e3e in doris::PInternalService::fold_constant_expr(google::protobuf::RpcController*, doris::PConstantExprRequest const*, doris::PConstantExprResult*, google::protobuf::Closure*)::$_0::operator()() const /mnt/disk1/wangqiannan/amory/doris/be/src/service/internal_service.cpp:1476:14
    apache#14 0x55dbdf54431e in void std::__invoke_impl<void, doris::PInternalService::fold_constant_expr(google::protobuf::RpcController*, doris::PConstantExprRequest const*, doris::PConstantExprResult*, google::protobuf::Closure*)::$_0&>(std::__invoke_other, doris::PInternalService::fold_constant_expr(google::protobuf::RpcController*, doris::PConstantExprRequest const*, doris::PConstantExprResult*, google::protobuf::Closure*)::$_0&) /mnt/disk1/wangqiannan/tool/ldb_toolchain_16/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14
    apache#15 0x55dbdf54425e in std::enable_if<is_invocable_r_v<void, doris::PInternalService::fold_constant_expr(google::protobuf::RpcController*, doris::PConstantExprRequest const*, doris::PConstantExprResult*, google::protobuf::Closure*)::$_0&>, void>::type std::__invoke_r<void, doris::PInternalService::fold_constant_expr(google::protobuf::RpcController*, doris::PConstantExprRequest const*, doris::PConstantExprResult*, google::protobuf::Closure*)::$_0&>(doris::PInternalService::fold_constant_expr(google::protobuf::RpcController*, doris::PConstantExprRequest const*, doris::PConstantExprResult*, google::protobuf::Closure*)::$_0&) /mnt/disk1/wangqiannan/tool/ldb_toolchain_16/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:111:2
    apache#16 0x55dbdf543f35 in std::_Function_handler<void (), doris::PInternalService::fold_constant_expr(google::protobuf::RpcController*, doris::PConstantExprRequest const*, doris::PConstantExprResult*, google::protobuf::Closure*)::$_0>::_M_invoke(std::_Any_data const&) /mnt/disk1/wangqiannan/tool/ldb_toolchain_16/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:291:9
    apache#17 0x55dbd8cd425a in std::function<void ()>::operator()() const /mnt/disk1/wangqiannan/tool/ldb_toolchain_16/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:560:9
    apache#18 0x55dbdf59ee14 in doris::WorkThreadPool<false>::work_thread(int) /mnt/disk1/wangqiannan/amory/doris/be/src/util/work_thread_pool.hpp:158:17
@@@
```
  • Loading branch information
amorynan committed Aug 16, 2024
1 parent 7372c99 commit 3ab85f3
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 1 deletion.
8 changes: 7 additions & 1 deletion be/src/vec/functions/array/function_array_aggregation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,13 @@ struct ArrayAggregateImpl {
const DataTypeArray* data_type_array =
static_cast<const DataTypeArray*>(remove_nullable(arguments[0]).get());
auto function = Function::create(data_type_array->get_nested_type());
return function->get_return_type();
if (function) {
return function->get_return_type();
} else {
throw doris::Exception(ErrorCode::INVALID_ARGUMENT,
"Unexpected type {} for aggregation {}",
data_type_array->get_nested_type()->get_name(), operation);
}
}

static Status execute(Block& block, const ColumnNumbers& arguments, size_t result,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.doris.nereids.trees.expressions.functions.scalar;

import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
Expand Down Expand Up @@ -50,6 +51,14 @@ public ArrayMax(Expression arg) {
super("array_max", arg);
}

@Override
public void checkLegalityBeforeTypeCoercion() {
DataType argType = child().getDataType();
if (((ArrayType) argType).getItemType().isComplexType()) {
throw new AnalysisException("array_max does not support complex types: " + toSql());
}
}

@Override
public DataType getDataType() {
return ((ArrayType) (child().getDataType())).getItemType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.doris.nereids.trees.expressions.functions.scalar;

import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
Expand Down Expand Up @@ -50,6 +51,14 @@ public ArrayMin(Expression arg) {
super("array_min", arg);
}

@Override
public void checkLegalityBeforeTypeCoercion() {
DataType argType = child().getDataType();
if (((ArrayType) argType).getItemType().isComplexType()) {
throw new AnalysisException("array_min does not support complex types: " + toSql());
}
}

@Override
public DataType getDataType() {
return ((ArrayType) (child().getDataType())).getItemType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
package org.apache.doris.nereids.trees.expressions.functions.scalar;

import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable;
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.ArrayType;
import org.apache.doris.nereids.types.DataType;
import org.apache.doris.nereids.types.coercion.AnyDataType;

import com.google.common.base.Preconditions;
Expand All @@ -48,6 +50,14 @@ public ArrayReverseSort(Expression arg) {
super("array_reverse_sort", arg);
}

@Override
public void checkLegalityBeforeTypeCoercion() {
DataType argType = child().getDataType();
if (((ArrayType) argType).getItemType().isComplexType()) {
throw new AnalysisException("array_reverse_sort does not support complex types: " + toSql());
}
}

/**
* withChildren.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
package org.apache.doris.nereids.trees.expressions.functions.scalar;

import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
import org.apache.doris.nereids.trees.expressions.functions.PropagateNullable;
import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.ArrayType;
import org.apache.doris.nereids.types.DataType;
import org.apache.doris.nereids.types.coercion.AnyDataType;

import com.google.common.base.Preconditions;
Expand All @@ -48,6 +50,14 @@ public ArraySort(Expression arg) {
super("array_sort", arg);
}

@Override
public void checkLegalityBeforeTypeCoercion() {
DataType argType = child().getDataType();
if (((ArrayType) argType).getItemType().isComplexType()) {
throw new AnalysisException("array_sort does not support complex types: " + toSql());
}
}

/**
* withChildren.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,36 @@ suite("nereids_scalar_fn_Array") {
sql """ set debug_skip_fold_constant=true; """
qt_array_empty_be """select array()"""

// array_min/max with nested array for args
test {
sql "select array_min(array(1,2,3),array(4,5,6));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}
test {
sql "select array_max(array(1,2,3),array(4,5,6));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}

test {
sql "select array_min(array(split_by_string('a,b,c',',')));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}
test {
sql "select array_max(array(split_by_string('a,b,c',',')));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}
// array_map with string is can be succeed
qt_sql_array_map """select array_map(x->x!='', split_by_string('amory,is,better,committing', ','))"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,37 @@ suite("test_array_functions_by_literal") {
assert("${ex}".contains("errCode = 2, detailMessage = No matching function with signature: array_intersect"))
}

// array_min/max with nested array for args
test {
sql "select array_min(array(1,2,3),array(4,5,6));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}
test {
sql "select array_max(array(1,2,3),array(4,5,6));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}

test {
sql "select array_min(array(split_by_string('a,b,c',',')));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}
test {
sql "select array_max(array(split_by_string('a,b,c',',')));"
check{result, exception, startTime, endTime ->
assertTrue(exception != null)
logger.info(exception.message)
}
}

// array_map with string is can be succeed
qt_sql_array_map """ select array_map(x->x!='', split_by_string('amory,is,better,committing', ',')) """

Expand Down

0 comments on commit 3ab85f3

Please sign in to comment.