Skip to content

[fix](join) preserve decimal precision for string comparison #65693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yx-keith wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[fix](join) preserve decimal precision for string comparison #65693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50454d1
[fix](join) preserve decimal precision for string comparison
Jul 16, 2026
3f84b6e
[fix](join) address lossless decimal cast CI findings
Jul 16, 2026
9e70032
[chore](be) format lossless decimal cast
Jul 16, 2026
cb59866
[fix](nereids) preserve decimal string comparison width
Jul 17, 2026
4b47820
[fix](join) address lossless cast review findings
Jul 17, 2026
18ad9eb
[test](nereids) update lossless decimal coercion baselines
Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion be/src/exprs/function/cast/cast_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,18 @@ WrapperType create_identity_wrapper(const DataTypePtr&);

} // namespace CastWrapper

enum class CastModeType { StrictMode, NonStrictMode };
enum class CastModeType { StrictMode, NonStrictMode, LosslessMode, StrictLosslessMode };

inline std::string cast_mode_type_to_string(CastModeType cast_mode) {
switch (cast_mode) {
case CastModeType::StrictMode:
return "StrictMode";
case CastModeType::NonStrictMode:
return "NonStrictMode";
case CastModeType::LosslessMode:
return "LosslessMode";
case CastModeType::StrictLosslessMode:
return "StrictLosslessMode";
default:
return "Unknown";
}
Expand Down
210 changes: 193 additions & 17 deletions be/src/exprs/function/cast/cast_to_decimal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@

#pragma once

#include <algorithm>
#include <cctype>
#include <cmath>
#include <cstdint>
#include <limits>
#include <type_traits>

#include "common/status.h"
Expand All @@ -35,45 +39,174 @@ namespace doris {
value, from_type_name, precision, scale))

struct CastToDecimal {
static bool is_lossless_decimal_string(const char* data, size_t size, UInt32 precision,
UInt32 target_scale) {
size_t begin = 0;
size_t end = size;
while (begin < end && std::isspace(static_cast<unsigned char>(data[begin]))) {
++begin;
}
while (begin < end && std::isspace(static_cast<unsigned char>(data[end - 1]))) {
--end;
}
if (begin == end) {
return false;
}

size_t pos = begin;
if (data[pos] == '+' || data[pos] == '-') {
++pos;
}

bool seen_dot = false;
bool seen_digit = false;
size_t digits_before_dot = 0;
size_t digits = 0;
size_t first_non_zero = 0;
size_t last_non_zero = 0;
while (pos < end && data[pos] != 'e' && data[pos] != 'E') {
unsigned char ch = static_cast<unsigned char>(data[pos]);
if (std::isdigit(ch)) {
seen_digit = true;
++digits;
if (!seen_dot) {
++digits_before_dot;
}
if (ch != '0') {
if (first_non_zero == 0) {
first_non_zero = digits;
}
last_non_zero = digits;
}
} else if (data[pos] == '.' && !seen_dot) {
seen_dot = true;
} else {
return false;
}
++pos;
}
if (!seen_digit) {
return false;
}

int64_t exponent = 0;
if (pos < end) {
++pos;
bool negative_exponent = false;
if (pos < end && (data[pos] == '+' || data[pos] == '-')) {
negative_exponent = data[pos] == '-';
++pos;
}
if (pos == end) {
return false;
}

int64_t exponent_magnitude = 0;
bool exponent_overflow = false;
while (pos < end) {
unsigned char ch = static_cast<unsigned char>(data[pos]);
if (!std::isdigit(ch)) {
return false;
}
if (!exponent_overflow) {
if (exponent_magnitude >
(std::numeric_limits<int64_t>::max() - (ch - '0')) / 10) {
exponent_overflow = true;
} else {
exponent_magnitude = exponent_magnitude * 10 + (ch - '0');
}
}
++pos;
}
if (exponent_overflow) {
return first_non_zero == 0;
}
exponent = negative_exponent ? -exponent_magnitude : exponent_magnitude;
}

// Zero is exactly representable at every decimal precision and scale.
if (first_non_zero == 0) {
return true;
}

size_t first_non_zero_offset = first_non_zero - 1;
if (digits_before_dot > std::numeric_limits<int64_t>::max() ||
first_non_zero_offset > std::numeric_limits<int64_t>::max()) {
return false;
}
int64_t base_decimal_point = static_cast<int64_t>(digits_before_dot) -
static_cast<int64_t>(first_non_zero_offset);

size_t significant_digits = last_non_zero - first_non_zero + 1;
if (significant_digits > precision) {
return false;
}

if ((exponent > 0 && base_decimal_point > std::numeric_limits<int64_t>::max() - exponent) ||
(exponent < 0 && base_decimal_point < std::numeric_limits<int64_t>::min() - exponent)) {
return false;
}
int64_t decimal_point = base_decimal_point + exponent;
int64_t max_integer_digits = precision - target_scale;
if (decimal_point > max_integer_digits ||
decimal_point < -static_cast<int64_t>(target_scale)) {
return false;
}
int64_t required_scale =
std::max(static_cast<int64_t>(significant_digits) - decimal_point, int64_t {0});
return required_scale <= target_scale;
}

template <typename ToCppT>
requires(IsDecimalNumber<ToCppT>)
static inline bool from_string(const StringRef& from, ToCppT& to, UInt32 precision,
UInt32 scale, CastParameters& params) {
static inline StringParser::ParseResult parse_string(const StringRef& from, ToCppT& to,
UInt32 precision, UInt32 scale) {
StringParser::ParseResult result = StringParser::PARSE_SUCCESS;
if constexpr (IsDecimalV2<ToCppT>) {
StringParser::ParseResult result = StringParser::PARSE_SUCCESS;
to = DecimalV2Value(StringParser::string_to_decimal<TYPE_DECIMALV2>(
from.data, (int)from.size, DecimalV2Value::PRECISION, DecimalV2Value::SCALE,
from.data, from.size, DecimalV2Value::PRECISION, DecimalV2Value::SCALE,
&result));
return result == StringParser::PARSE_SUCCESS;
}

if constexpr (IsDecimal32<ToCppT>) {
StringParser::ParseResult result = StringParser::PARSE_SUCCESS;
to.value = StringParser::string_to_decimal<TYPE_DECIMAL32>(from.data, (int)from.size,
to.value = StringParser::string_to_decimal<TYPE_DECIMAL32>(from.data, from.size,
precision, scale, &result);
return result == StringParser::PARSE_SUCCESS;
}

if constexpr (IsDecimal64<ToCppT>) {
StringParser::ParseResult result = StringParser::PARSE_SUCCESS;
to.value = StringParser::string_to_decimal<TYPE_DECIMAL64>(from.data, (int)from.size,
to.value = StringParser::string_to_decimal<TYPE_DECIMAL64>(from.data, from.size,
precision, scale, &result);
return result == StringParser::PARSE_SUCCESS;
}

if constexpr (IsDecimal128V3<ToCppT>) {
StringParser::ParseResult result = StringParser::PARSE_SUCCESS;
to.value = StringParser::string_to_decimal<TYPE_DECIMAL128I>(from.data, (int)from.size,
to.value = StringParser::string_to_decimal<TYPE_DECIMAL128I>(from.data, from.size,
precision, scale, &result);
return result == StringParser::PARSE_SUCCESS;
}

if constexpr (IsDecimal256<ToCppT>) {
StringParser::ParseResult result = StringParser::PARSE_SUCCESS;
to.value = StringParser::string_to_decimal<TYPE_DECIMAL256>(from.data, (int)from.size,
to.value = StringParser::string_to_decimal<TYPE_DECIMAL256>(from.data, from.size,
precision, scale, &result);
return result == StringParser::PARSE_SUCCESS;
}
return result;
}

template <typename ToCppT>
requires(IsDecimalNumber<ToCppT>)
static inline bool from_string(const StringRef& from, ToCppT& to, UInt32 precision,
UInt32 scale, CastParameters& params) {
return parse_string(from, to, precision, scale) == StringParser::PARSE_SUCCESS;
}

template <typename ToCppT>
requires(IsDecimalNumber<ToCppT>)
static inline bool from_string_lossless(const StringRef& from, ToCppT& to, UInt32 precision,
UInt32 scale, CastParameters& params) {
if (!is_lossless_decimal_string(from.data, from.size, precision, scale)) {
return false;
}
StringParser::ParseResult result = parse_string(from, to, precision, scale);
return result == StringParser::PARSE_SUCCESS || result == StringParser::PARSE_OVERFLOW ||
result == StringParser::PARSE_UNDERFLOW;
}

// cast int to decimal
Expand Down Expand Up @@ -704,6 +837,49 @@ class CastToImpl<Mode, DataTypeString, ToDataType> : public CastToBase {
RETURN_IF_ERROR(
serde->from_string_strict_mode_batch(*col_from, *column_to, {}, null_map));
block.get_by_position(result).column = std::move(column_to);
} else if constexpr (Mode == CastModeType::LosslessMode ||
Mode == CastModeType::StrictLosslessMode) {
auto nullable_col_to = create_empty_nullable_column(nested_to_type);
auto& nullable_to = *nullable_col_to;
nullable_to.resize(col_from->size());
auto& column_to =
assert_cast<typename ToDataType::ColumnType&>(nullable_to.get_nested_column());
auto& values_to = column_to.get_data();
auto& null_map_to = nullable_to.get_null_map_data();
auto* decimal_to_type = assert_cast<const ToDataType*>(nested_to_type.get());
const auto& chars = col_from->get_chars();
const auto& offsets = col_from->get_offsets();
size_t current_offset = 0;
UInt32 precision = decimal_to_type->get_precision();
UInt32 scale = decimal_to_type->get_scale();
[[maybe_unused]] CastParameters params;
params.is_strict = false;
for (size_t i = 0; i < col_from->size(); ++i) {
size_t next_offset = offsets[i];
size_t string_size = next_offset - current_offset;
bool source_is_null = null_map != nullptr && null_map[i];
values_to[i] = {};
StringRef source(reinterpret_cast<const char*>(&chars[current_offset]),
string_size);
bool parsed = false;
if constexpr (Mode == CastModeType::StrictLosslessMode) {
if (!source_is_null &&
CastToDecimal::parse_string(source, values_to[i], precision, scale) !=
StringParser::PARSE_SUCCESS) {
return Status::InvalidArgument("parse number fail, string: '{}'",
std::string(source.data, source.size));
}
parsed = !source_is_null && CastToDecimal::is_lossless_decimal_string(
source.data, source.size, precision, scale);
} else {
parsed = !source_is_null &&
CastToDecimal::from_string_lossless(source, values_to[i], precision,
scale, params);
}
null_map_to[i] = !parsed;
current_offset = next_offset;
}
block.get_by_position(result).column = std::move(nullable_col_to);
} else {
return Status::InternalError("Unsupported cast mode");
}
Expand Down
18 changes: 17 additions & 1 deletion be/src/exprs/function/cast/function_cast_decimal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,23 @@ WrapperType create_decimal_wrapper(FunctionContext* context, const DataTypePtr&
using Types = std::decay_t<decltype(types)>;
using FromDataType = typename Types::LeftType;
if constexpr (type_allow_cast_to_decimal<FromDataType>) {
if (context->enable_strict_mode()) {
if constexpr (std::is_same_v<FromDataType, DataTypeString>) {
if (context->enable_lossless_decimal_cast()) {

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve strict-cast failures in lossless comparisons — Because this branch runs before enable_strict_mode(), a marked comparison under enable_strict_cast=true no longer uses from_string_strict_mode_batch: malformed text such as 100abc and decimal overflow become NULL non-matches instead of failing the query. The new regression explicitly demonstrates that behavior, but it contradicts the existing session contract (“Use strict mode for cast”) and changes failure into silent success. Please keep strict lexical/overflow errors while separately rejecting well-formed but inexact comparison values, and add expected-error coverage.

if (context->enable_strict_mode()) {
cast_impl = std::make_shared<CastToImpl<CastModeType::StrictLosslessMode,
FromDataType, ToDataType>>();
} else {
cast_impl = std::make_shared<
CastToImpl<CastModeType::LosslessMode, FromDataType, ToDataType>>();
}
} else if (context->enable_strict_mode()) {
cast_impl = std::make_shared<
CastToImpl<CastModeType::StrictMode, FromDataType, ToDataType>>();
} else {
cast_impl = std::make_shared<
CastToImpl<CastModeType::NonStrictMode, FromDataType, ToDataType>>();
}
} else if (context->enable_strict_mode()) {
cast_impl = std::make_shared<
CastToImpl<CastModeType::StrictMode, FromDataType, ToDataType>>();
} else {
Expand Down
1 change: 1 addition & 0 deletions be/src/exprs/function_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ std::unique_ptr<FunctionContext> FunctionContext::clone() {
new_context->_fragment_local_fn_state = _fragment_local_fn_state;
new_context->_check_overflow_for_decimal = _check_overflow_for_decimal;
new_context->_enable_strict_mode = _enable_strict_mode;
new_context->_enable_lossless_decimal_cast = _enable_lossless_decimal_cast;
new_context->_string_as_jsonb_string = _string_as_jsonb_string;
new_context->_jsonb_string_as_string = _jsonb_string_as_string;
return new_context;
Expand Down
7 changes: 7 additions & 0 deletions be/src/exprs/function_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class FunctionContext {

bool enable_strict_mode() const { return _enable_strict_mode; }

bool enable_lossless_decimal_cast() const { return _enable_lossless_decimal_cast; }

bool set_check_overflow_for_decimal(bool check_overflow_for_decimal) {
return _check_overflow_for_decimal = check_overflow_for_decimal;
}
Expand All @@ -92,6 +94,10 @@ class FunctionContext {
return _enable_strict_mode = enable_strict_mode;
}

bool set_enable_lossless_decimal_cast(bool enable_lossless_decimal_cast) {
return _enable_lossless_decimal_cast = enable_lossless_decimal_cast;
}

void set_string_as_jsonb_string(bool string_as_jsonb_string) {
_string_as_jsonb_string = string_as_jsonb_string;
}
Expand Down Expand Up @@ -198,6 +204,7 @@ class FunctionContext {
RuntimeProfile::Counter* _udf_execute_timer = nullptr;
bool _check_overflow_for_decimal = false;
bool _enable_strict_mode = false;
bool _enable_lossless_decimal_cast = false;

bool _string_as_jsonb_string = false;
bool _jsonb_string_as_string = false;
Expand Down
2 changes: 2 additions & 0 deletions be/src/exprs/vcast_expr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ doris::Status VCastExpr::prepare(doris::RuntimeState* state, const doris::RowDes
child->data_type()->get_name(), _target_data_type_name);
}
VExpr::register_function_context(state, context);
context->fn_context(_fn_context_index)
->set_enable_lossless_decimal_cast(_lossless_decimal_cast);
_expr_name = fmt::format("({} {}({}) TO {})", cast_name(), child_name,
child->data_type()->get_name(), _target_data_type_name);
_prepare_finished = true;
Expand Down
14 changes: 10 additions & 4 deletions be/src/exprs/vcast_expr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class VCastExpr : public VExpr {
#ifdef BE_TEST
VCastExpr() = default;
#endif
VCastExpr(const TExprNode& node) : VExpr(node) {}
VCastExpr(const TExprNode& node)
: VExpr(node), _lossless_decimal_cast(node.lossless_decimal_cast) {}

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness] Preserve the lossless mode in prefilter clones. VCastExpr::clone_node() rebuilds from clone_texpr_node(), which omits this field, while the format-v2 override recreates a cast from only its target type. For an external row (d=0, s='0e2147483648'), the original lossless cast treats zero as exact and accepts the parser's overflow status as value 0, but the localized ordinary cast returns NULL and filters the row before Scanner::_filter_output_block() can run the original residual. Copy the flag through both clone paths or refuse localization, and add this external-file false-negative regression.

~VCastExpr() override = default;
Status execute_column_impl(VExprContext* context, const Block* block, const Selector* selector,
size_t count, ColumnPtr& result_column) const override;
Expand All @@ -55,20 +56,24 @@ class VCastExpr : public VExpr {
const std::string& expr_name() const override;
std::string debug_string() const override;
const DataTypePtr& get_target_type() const;
bool is_lossless_decimal_cast() const { return _lossless_decimal_cast; }
bool is_safe_to_execute_on_selected_rows() const override { return false; }

virtual std::string cast_name() const { return "CAST"; }
Status clone_node(VExprSPtr* cloned_expr) const override {
DORIS_CHECK(cloned_expr != nullptr);
*cloned_expr = VCastExpr::create_shared(clone_texpr_node());
auto node = clone_texpr_node();
node.__set_lossless_decimal_cast(_lossless_decimal_cast);
*cloned_expr = VCastExpr::create_shared(node);
return Status::OK();
}

uint64_t get_digest(uint64_t seed) const override {
auto res = VExpr::get_digest(seed);
if (res) {
return HashUtil::hash64(_target_data_type_name.data(), _target_data_type_name.size(),
res);
res = HashUtil::hash64(_target_data_type_name.data(), _target_data_type_name.size(),
res);
return HashUtil::hash64(&_lossless_decimal_cast, sizeof(_lossless_decimal_cast), res);
}
return 0;
}
Expand All @@ -82,6 +87,7 @@ class VCastExpr : public VExpr {
std::string _target_data_type_name;

DataTypePtr _cast_param_data_type;
bool _lossless_decimal_cast = false;

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[correctness] Include this mode in VCastExpr::get_digest(). The current digest hashes the child/base expression and target type only, so ordinary and lossless casts collide despite different results. For a granule containing (d=100, s='100.4'), the implicit lossless predicate can cache the granule as all-false; a later explicit d = CAST(s AS DECIMAL(38,0)) has the same condition-cache key even though ordinary casting rounds and should match, so the cached bitmap suppresses the row. Hash _lossless_decimal_cast (or disable condition caching for this mode) and add a digest/cache distinction test.


static const constexpr char* function_name = "CAST";
};
Expand Down
3 changes: 2 additions & 1 deletion be/src/format_v2/column_mapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,8 @@ static Status clone_table_expr_node(const VExpr& expr, VExprSPtr* cloned_expr) {
split_literal->original_type(), split_literal->original_field());
} else if (const auto* vcast_expr = dynamic_cast<const VCastExpr*>(&expr);
vcast_expr != nullptr && vcast_expr->node_type() == TExprNodeType::CAST_EXPR) {
*cloned_expr = Cast::create_shared(vcast_expr->data_type());
*cloned_expr = Cast::create_shared(vcast_expr->data_type(),
vcast_expr->is_lossless_decimal_cast());
}
return Status::OK();
}
Expand Down
Loading
Loading