Skip to content

Commit

Permalink
[Bug](function) catch error state in function cast to avoid core dump (
Browse files Browse the repository at this point in the history
…#20751)

catch error state in function cast to avoid core dump
  • Loading branch information
BiteTheDDDDt authored Jun 14, 2023
1 parent 1c9f107 commit a0d4f11
Show file tree
Hide file tree
Showing 64 changed files with 536 additions and 603 deletions.
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Checks: |
modernize-use-override,
modernize-use-equals-default,
modernize-use-equals-delete,
modernize-use-nodiscard,
modernize-use-nullptr,
modernize-use-bool-literals,
modernize-use-using,
Expand Down
15 changes: 12 additions & 3 deletions be/src/common/exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ class Exception : public std::exception {

int code() const { return _code; }

std::string to_string() const;
const std::string& to_string() const;

const char* what() const noexcept override { return to_string().c_str(); }

Status to_status() const { return Status::Error(code(), to_string()); }

private:
int _code;
Expand All @@ -59,9 +63,13 @@ class Exception : public std::exception {
};
std::unique_ptr<ErrMsg> _err_msg;
std::unique_ptr<Exception> _nested_excption;
mutable std::string _cache_string;
};

inline std::string Exception::to_string() const {
inline const std::string& Exception::to_string() const {
if (!_cache_string.empty()) {
return _cache_string;
}
std::stringstream ostr;
ostr << "[E" << _code << "] ";
ostr << (_err_msg ? _err_msg->_msg : "");
Expand All @@ -71,7 +79,8 @@ inline std::string Exception::to_string() const {
if (_nested_excption != nullptr) {
ostr << '\n' << "Caused by:" << _nested_excption->to_string();
}
return ostr.str();
_cache_string = ostr.str();
return _cache_string;
}

} // namespace doris
Expand Down
2 changes: 1 addition & 1 deletion be/src/exprs/runtime_filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ Status create_literal(const TypeDescriptor& type, const void* data, vectorized::
try {
expr = vectorized::VLiteral::create_shared(node);
} catch (const Exception& e) {
return Status::Error(e.code(), e.to_string());
return e.to_status();
}

return Status::OK();
Expand Down
2 changes: 1 addition & 1 deletion be/src/pipeline/task_scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ void TaskScheduler::_do_work(size_t index) {
try {
status = task->execute(&eos);
} catch (const Exception& e) {
status = Status::Error(e.code(), e.to_string());
status = e.to_status();
}

task->set_previous_core_id(index);
Expand Down
2 changes: 1 addition & 1 deletion be/src/service/internal_service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ void PInternalServiceImpl::exec_plan_fragment(google::protobuf::RpcController* c
try {
st = _exec_plan_fragment(request->request(), version, compact);
} catch (const Exception& e) {
st = Status::Error(e.code(), e.to_string());
st = e.to_status();
}
if (!st.ok()) {
LOG(WARNING) << "exec plan fragment failed, errmsg=" << st;
Expand Down
8 changes: 8 additions & 0 deletions be/src/vec/data_types/data_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <string>
#include <vector>

#include "common/exception.h"
#include "common/status.h"
#include "runtime/define_primitive_type.h"
#include "vec/common/cow.h"
Expand Down Expand Up @@ -236,6 +237,13 @@ class IDataType : private boost::noncopyable {

static PGenericType_TypeId get_pdata_type(const IDataType* data_type);

[[nodiscard]] virtual UInt32 get_precision() const {
throw Exception(ErrorCode::INTERNAL_ERROR, "type {} not support get_precision", get_name());
}
[[nodiscard]] virtual UInt32 get_scale() const {
throw Exception(ErrorCode::INTERNAL_ERROR, "type {} not support get_scale", get_name());
}

private:
friend class DataTypeFactory;
};
Expand Down
65 changes: 47 additions & 18 deletions be/src/vec/data_types/data_type_decimal.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,8 @@ class DataTypeDecimal final : public IDataType {
static constexpr size_t max_precision() { return max_decimal_precision<T>(); }

DataTypeDecimal(UInt32 precision = 27, UInt32 scale = 9) : precision(precision), scale(scale) {
if (UNLIKELY(precision < 1 || precision > max_precision())) {
LOG(FATAL) << fmt::format("Precision {} is out of bounds", precision);
}

if (UNLIKELY(static_cast<UInt32>(scale) > max_precision())) {
LOG(FATAL) << fmt::format("Scale {} is out of bounds", scale);
}
check_type_precision(precision);
check_type_scale(scale);
}

DataTypeDecimal(const DataTypeDecimal& rhs) : precision(rhs.precision), scale(rhs.scale) {}
Expand Down Expand Up @@ -259,26 +254,34 @@ class DataTypeDecimal final : public IDataType {

/// Decimal specific

UInt32 get_precision() const { return precision; }
UInt32 get_scale() const { return scale; }
[[nodiscard]] UInt32 get_precision() const override { return precision; }
[[nodiscard]] UInt32 get_scale() const override { return scale; }
T get_scale_multiplier() const { return get_scale_multiplier(scale); }

T whole_part(T x) const {
if (scale == 0) return x;
if (scale == 0) {
return x;
}
return x / get_scale_multiplier();
}

T fractional_part(T x) const {
if (scale == 0) return 0;
if (x < T(0)) x *= T(-1);
if (scale == 0) {
return 0;
}
if (x < T(0)) {
x *= T(-1);
}
return x % get_scale_multiplier();
}

T max_whole_value() const { return get_scale_multiplier(max_precision() - scale) - T(1); }

bool can_store_whole(T x) const {
T max = max_whole_value();
if (x > max || x < -max) return false;
if (x > max || x < -max) {
return false;
}
return true;
}

Expand All @@ -295,14 +298,33 @@ class DataTypeDecimal final : public IDataType {

template <typename U>
T scale_factor_for(const DataTypeNumber<U>&, bool is_multiply_or_divisor) const {
if (is_multiply_or_divisor) return 1;
if (is_multiply_or_divisor) {
return 1;
}
return get_scale_multiplier();
}

static T get_scale_multiplier(UInt32 scale);

bool parse_from_string(const std::string& str, T* res) const;

static void check_type_precision(const vectorized::UInt32 precision) {
if (precision > max_decimal_precision<T>() || precision < 1) {
throw Exception(ErrorCode::INTERNAL_ERROR,
"meet invalid precision: real_precision={}, max_decimal_precision={}, "
"min_decimal_precision=1",
precision, max_decimal_precision<T>());
}
}

static void check_type_scale(const vectorized::UInt32 scale) {
if (scale > max_decimal_precision<T>()) {
throw Exception(ErrorCode::INTERNAL_ERROR,
"meet invalid scale: real_scale={}, max_decimal_precision={}", scale,
max_decimal_precision<T>());
}
}

private:
const UInt32 precision;
const UInt32 scale;
Expand Down Expand Up @@ -342,11 +364,18 @@ const DataTypeDecimal<T>* check_decimal(const IDataType& data_type) {
}

inline UInt32 get_decimal_scale(const IDataType& data_type, UInt32 default_value = 0) {
if (auto* decimal_type = check_decimal<Decimal32>(data_type)) return decimal_type->get_scale();
if (auto* decimal_type = check_decimal<Decimal64>(data_type)) return decimal_type->get_scale();
if (auto* decimal_type = check_decimal<Decimal128>(data_type)) return decimal_type->get_scale();
if (auto* decimal_type = check_decimal<Decimal128I>(data_type))
if (auto* decimal_type = check_decimal<Decimal32>(data_type)) {
return decimal_type->get_scale();
}
if (auto* decimal_type = check_decimal<Decimal64>(data_type)) {
return decimal_type->get_scale();
}
if (auto* decimal_type = check_decimal<Decimal128>(data_type)) {
return decimal_type->get_scale();
}
if (auto* decimal_type = check_decimal<Decimal128I>(data_type)) {
return decimal_type->get_scale();
}
return default_value;
}

Expand Down
2 changes: 1 addition & 1 deletion be/src/vec/data_types/data_type_time_v2.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class DataTypeDateTimeV2 final : public DataTypeNumberBase<UInt64> {
}
MutableColumnPtr create_column() const override;

UInt32 get_scale() const { return _scale; }
UInt32 get_scale() const override { return _scale; }

void to_pb_column_meta(PColumnMeta* col_meta) const override;

Expand Down
21 changes: 15 additions & 6 deletions be/src/vec/exprs/vcast_expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@
#include <stddef.h>

#include <algorithm>
#include <exception>
#include <memory>
#include <ostream>
#include <vector>

#include "common/exception.h"
#include "common/status.h"
#include "vec/core/block.h"
#include "vec/core/column_with_type_and_name.h"
Expand Down Expand Up @@ -72,7 +74,8 @@ doris::Status VCastExpr::prepare(doris::RuntimeState* state, const doris::RowDes
return Status::NotSupported("Function {} is not implemented", _fn.name.function_name);
}
VExpr::register_function_context(state, context);
_expr_name = fmt::format("(CAST {} TO {})", child_name, _target_data_type_name);
_expr_name = fmt::format("(CAST {}({}) TO {})", child_name, child->data_type()->get_name(),
_target_data_type_name);
return Status::OK();
}

Expand Down Expand Up @@ -102,11 +105,17 @@ doris::Status VCastExpr::execute(VExprContext* context, doris::vectorized::Block
size_t num_columns_without_result = block->columns();
// prepare a column to save result
block->insert({nullptr, _data_type, _expr_name});
RETURN_IF_ERROR(_function->execute(context->fn_context(_fn_context_index), *block,
{static_cast<size_t>(column_id), const_param_id},
num_columns_without_result, block->rows(), false));
*result_column_id = num_columns_without_result;
return Status::OK();

auto state = Status::OK();
try {
state = _function->execute(context->fn_context(_fn_context_index), *block,
{static_cast<size_t>(column_id), const_param_id},
num_columns_without_result, block->rows(), false);
*result_column_id = num_columns_without_result;
} catch (const Exception& e) {
state = e.to_status();
}
return state;
}

const std::string& VCastExpr::expr_name() const {
Expand Down
2 changes: 1 addition & 1 deletion be/src/vec/exprs/vexpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ Status VExpr::create_expr(const doris::TExprNode& expr_node, VExprSPtr& expr) {
return Status::InternalError("Unknown expr node type: {}", expr_node.node_type);
}
} catch (const Exception& e) {
return Status::Error(e.code(), e.to_string());
return e.to_status();
}
if (!expr->data_type()) {
return Status::InvalidArgument("Unknown expr type: {}", expr_node.node_type);
Expand Down
19 changes: 10 additions & 9 deletions be/src/vec/functions/function_cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ struct ConvertImpl {
typename ColVecTo::MutablePtr col_to = nullptr;
if constexpr (IsDataTypeDecimal<ToDataType>) {
UInt32 scale = additions;
ToDataType::check_type_scale(scale);
col_to = ColVecTo::create(0, scale);
} else {
col_to = ColVecTo::create();
Expand Down Expand Up @@ -1074,19 +1075,15 @@ class FunctionConvert : public IFunction {
return true;
}

const ColumnWithTypeAndName& scale_column = block.get_by_position(arguments[1]);
UInt32 scale = extract_to_decimal_scale(scale_column);

const ColumnWithTypeAndName& scale_column = block.get_by_position(result);
ret_status = ConvertImpl<LeftDataType, RightDataType, Name>::execute(
context, block, arguments, result, input_rows_count,
context->check_overflow_for_decimal(), scale);
context->check_overflow_for_decimal(), scale_column.type->get_scale());
} else if constexpr (IsDataTypeDateTimeV2<RightDataType>) {
const ColumnWithTypeAndName& scale_column = block.get_by_position(result);
auto type =
check_and_get_data_type<DataTypeDateTimeV2>(scale_column.type.get());
ret_status = ConvertImpl<LeftDataType, RightDataType, Name>::execute(
context, block, arguments, result, input_rows_count,
context->check_overflow_for_decimal(), type->get_scale());
context->check_overflow_for_decimal(), scale_column.type->get_scale());
} else {
ret_status = ConvertImpl<LeftDataType, RightDataType, Name>::execute(
context, block, arguments, result, input_rows_count);
Expand Down Expand Up @@ -1296,6 +1293,7 @@ struct ConvertThroughParsing {

if constexpr (IsDataTypeDecimal<ToDataType>) {
UInt32 scale = additions;
ToDataType::check_type_scale(scale);
col_to = ColVecTo::create(size, scale);
} else {
col_to = ColVecTo::create(size);
Expand Down Expand Up @@ -1588,9 +1586,12 @@ class FunctionCast final : public IFunctionBase {
using LeftDataType = typename Types::LeftType;
using RightDataType = typename Types::RightType;

ConvertImpl<LeftDataType, RightDataType, NameCast>::execute(
auto state = ConvertImpl<LeftDataType, RightDataType, NameCast>::execute(
context, block, arguments, result, input_rows_count,
context->check_overflow_for_decimal(), scale);
if (!state) {
throw Exception(state.code(), state.to_string());
}
return true;
});

Expand Down Expand Up @@ -2059,7 +2060,7 @@ class FunctionBuilderCast : public FunctionBuilderImpl {
static constexpr auto name = "CAST";
static FunctionBuilderPtr create() { return std::make_shared<FunctionBuilderCast>(); }

FunctionBuilderCast() {}
FunctionBuilderCast() = default;

String get_name() const override { return name; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
-- !select_default --
1 a
3 c

Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ all 1
all 2

-- !select2 --
1.0 1
1 1

-- !select3 --
1.0 1
1 1

2 changes: 1 addition & 1 deletion regression-test/data/correctness_p0/test_implict_cast.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !select --
0.0
0.00

Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@

-- !select_geo1 --
POINT (123.123456789 89.123456789)

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
1.6674

-- !select_decimal64 --
1.3290205760164607E11
1.3290205760164606E11

-- !select_decimal128 --
564.666667
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@

-- !sql2 --
2

Binary file modified regression-test/data/datatype_p0/bitmap/test_bitmap_int.out
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@
1 -1.0
2 0.5
3 1.0

Loading

0 comments on commit a0d4f11

Please sign in to comment.