github-actions[bot] commented on code in PR #65693:
URL: https://github.com/apache/doris/pull/65693#discussion_r3595218040
##########
be/src/exprs/function/cast/cast_to_decimal.h:
##########
@@ -35,45 +39,166 @@ 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;
+ while (pos < end) {
+ unsigned char ch = static_cast<unsigned char>(data[pos]);
+ if (!std::isdigit(ch)) {
+ return false;
+ }
+ if (exponent_magnitude > (std::numeric_limits<int64_t>::max()
- (ch - '0')) / 10) {
Review Comment:
[P1] Finish validating the exponent before accepting zero — For
`0e9223372036854775808x`, this returns true as soon as the exponent overflows
because the significand is zero, without ever validating the trailing `x`.
`StringParser::string_to_decimal` also stops at its smaller exponent cap,
returns `PARSE_OVERFLOW` with value 0, and `from_string_lossless` accepts that
status, so this malformed VARCHAR becomes a non-NULL zero and incorrectly
matches DECIMAL zero. Please keep scanning the exponent grammar after magnitude
overflow and only accept zero once the whole token is valid; this is the
opposite direction from the existing cancelling-exponent thread.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java:
##########
@@ -449,7 +449,7 @@ public Expr visitCaseWhen(CaseWhen caseWhen,
PlanTranslatorContext context) {
public Expr visitCast(Cast cast, PlanTranslatorContext context) {
// left child of cast is expression, right child of cast is target type
CastExpr castExpr = new
CastExpr(cast.getDataType().toCatalogDataType(),
- cast.child().accept(this, context), cast.nullable());
+ cast.child().accept(this, context), cast.nullable(),
cast.isLosslessDecimalCast());
Review Comment:
[P1] Keep this semantic cast out of generic remote pushdown — This bit
survives BE translation here, but both `ExprToExternalSqlVisitor` and
`ExprToConnectorExpressionConverter` erase every `CastExpr`. Remote Doris/JDBC
can therefore execute a lossy `d = s` predicate and also receive the scan
LIMIT. For remote rows `(9007199254740992,'9007199254740992.0000004')` before
the exact `(9007199254740992,'9007199254740992')`, legacy DOUBLE coercion loses
the fraction and the newer pre-fix fixed-numeric path rounds it at scale six,
so `WHERE d = s LIMIT 1` can return only the false positive; the local lossless
residual drops it and cannot recover the exact row. Please preserve an
equivalent exact predicate only for connectors that support it, or leave this
as a local residual and suppress remote LIMIT pushdown.
##########
be/src/exprs/function/cast/function_cast_decimal.cpp:
##########
@@ -32,7 +32,18 @@ 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()) {
Review Comment:
[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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
