Gabriel39 commented on code in PR #69672:
URL: https://github.com/apache/doris/pull/69672#discussion_r4409478449
##########
be/src/format/table/es/es_scroll_parser.cpp:
##########
@@ -723,15 +801,20 @@ Status ScrollParser::fill_columns(const
std::vector<SlotDescriptor*>& slots,
}
}
- const rapidjson::Value& col = (*line)[col_name];
-
auto type = slot_desc->type()->get_primitive_type();
-
- // when the column value is null, the subsequent type casting will
report an error
- if (col.IsNull() && slot_desc->is_nullable()) {
+ const rapidjson::Value& source_value = (*line)[col_name];
+ // Source fallback changes every sibling's decoder. A scalar
numeric/Boolean/date field
+ // may hold [value], [] or [null]; preserve its single value or
missing-value semantics.
+ // STRING keeps its JSON representation, and declared ARRAY keeps its
container.
+ bool scalar_source_array = !pure_doc_value && source_value.IsArray() &&
+ type != TYPE_ARRAY && !is_string_type(type);
Review Comment:
Fixed in c11be7b2769ebf43424deade2b5df79c8a1a30f8. Scalar source-array
normalization now excludes JSONB, preserving singleton, empty and null-element
containers. Added JSONB parser controls and live source/docvalue controls for
both readers; the new regression also compares each query with an independent
internal table.
##########
be/src/format/table/es/es_scroll_parser.cpp:
##########
@@ -156,36 +146,153 @@ static const std::string INVALID_NULL_VALUE =
return Status::RuntimeError(ss.str()); \
} while (false)
+// ES integer coercion truncates decimal/scientific notation, e.g. "1.2e3" ->
1200.
+// Shift the decimal point using digits so values near INT64_MAX never pass
through double.
template <typename T>
-Status get_int_value(const rapidjson::Value& col, PrimitiveType type, void*
slot,
- bool pure_doc_value) {
- if (col.IsNumber()) {
- *reinterpret_cast<T*>(slot) = (T)(sizeof(T) < 8 ? col.GetInt() :
col.GetInt64());
- return Status::OK();
+static T parse_es_integer(const char* text, size_t length,
StringParser::ParseResult* result) {
+ text = skip_ascii_whitespaces(text, length);
+ size_t i = 0;
+ bool negative = length > 0 && text[0] == '-';
+ if (length > 0 && (text[0] == '-' || text[0] == '+')) {
+ ++i;
+ }
+ size_t start = i;
+ int64_t digits = 0;
+ int64_t integer_digits = 0;
+ int64_t first_nonzero = -1;
+ bool dot = false;
+ for (; i < length; ++i) {
+ char c = text[i];
+ if (c >= '0' && c <= '9') {
+ if (c != '0' && first_nonzero < 0) {
+ first_nonzero = digits;
+ }
+ ++digits;
+ integer_digits += !dot;
+ } else if (c == '.' && !dot) {
+ dot = true;
+ } else {
+ break;
+ }
}
-
- if (pure_doc_value && col.IsArray() && !col.Empty()) {
- RETURN_ERROR_IF_COL_IS_NOT_NUMBER(col[0], type);
- *reinterpret_cast<T*>(slot) = (T)(sizeof(T) < 8 ? col[0].GetInt() :
col[0].GetInt64());
- return Status::OK();
+ size_t end = i;
+ int64_t exponent = 0;
+ bool negative_exponent = false;
+ if (i < length && (text[i] == 'e' || text[i] == 'E')) {
+ ++i;
+ if (i < length && (text[i] == '+' || text[i] == '-')) {
+ negative_exponent = text[i++] == '-';
+ }
+ size_t exponent_start = i;
+ for (; i < length && text[i] >= '0' && text[i] <= '9'; ++i) {
+ // Saturation avoids overflow and work proportional to an
untrusted exponent.
+ exponent = std::min(exponent * 10 + text[i] - '0',
static_cast<int64_t>(length) + 64);
+ }
+ if (i == exponent_start) {
+ *result = StringParser::PARSE_FAILURE;
+ return 0;
+ }
}
+ if (digits == 0 || i != length) {
+ *result = StringParser::PARSE_FAILURE;
+ return 0;
+ }
+ int64_t count = integer_digits + (negative_exponent ? -exponent :
exponent) - first_nonzero;
+ if (first_nonzero < 0 || count <= 0) {
+ *result = StringParser::PARSE_SUCCESS;
+ return 0;
+ }
+ // 39 digits cover every signed 128-bit value; the final parser checks the
exact type bounds.
+ if (count > 39) {
+ *result = StringParser::PARSE_OVERFLOW;
+ return 0;
+ }
+ char integral[40];
+ size_t size = 0;
+ if (negative) {
+ integral[size++] = '-';
+ }
+ int64_t digit = 0;
+ for (i = start; i < end && count > 0; ++i) {
+ if (text[i] != '.' && digit++ >= first_nonzero) {
+ integral[size++] = text[i];
+ --count;
+ }
+ }
+ while (count-- > 0) {
+ integral[size++] = '0';
+ }
+ return StringParser::string_to_int<T, true>(integral, size, result);
+}
- RETURN_ERROR_IF_COL_IS_ARRAY(col, type, true);
- RETURN_ERROR_IF_COL_IS_NOT_STRING(col, type);
-
- StringParser::ParseResult result;
- const std::string& val = col.GetString();
- size_t len = col.GetStringLength();
- T v = StringParser::string_to_int<T>(val.c_str(), len, &result);
- RETURN_ERROR_IF_PARSING_FAILED(result, col, type);
+template <typename T>
+static T parse_es_narrow_integer(const std::string& text,
StringParser::ParseResult* result) {
+ // Unlike long's BigDecimal conversion, ES parses byte/short/integer
strings through double:
+ // "0.99999999999999999" rounds to 1 before truncation. Keep long's exact
path separate.
+ auto number = StringParser::string_to_float<double>(text.data(),
text.size(), result);
+ if (*result != StringParser::PARSE_SUCCESS) {
+ return 0;
+ }
+ // Java converts NaN to zero; casting it directly in C++ would be
undefined.
+ if (std::isnan(number)) {
+ return 0;
+ }
+ using ParsedType = std::conditional_t<sizeof(T) == 1, int32_t, T>;
+ if (number < StringParser::numeric_limits<ParsedType>(true) ||
+ number > StringParser::numeric_limits<ParsedType>(false)) {
+ *result = StringParser::PARSE_OVERFLOW;
+ return 0;
+ }
+ auto parsed = static_cast<ParsedType>(number);
+ // ES byte uses intValue() then checks the byte range, so "127.9" is
valid. Short and
+ // integer instead check their own double bounds before casting (e.g.
short "32767.9" fails).
+ if constexpr (sizeof(T) == 1) {
+ if (parsed < StringParser::numeric_limits<T>(true) ||
+ parsed > StringParser::numeric_limits<T>(false)) {
+ *result = StringParser::PARSE_OVERFLOW;
+ return 0;
+ }
+ }
+ return static_cast<T>(parsed);
+}
- if (sizeof(T) < 16) {
- *reinterpret_cast<T*>(slot) = v;
+template <typename T>
+Status get_int_value(const rapidjson::Value& col, PrimitiveType type, void*
slot,
+ bool pure_doc_value) {
+ const rapidjson::Value* value = &col;
+ // A singleton source array has the same scalar value as its one docvalue.
Keep rejecting
+ // multi-value source arrays: selecting an arbitrary element would discard
source semantics.
+ if (col.IsArray() && !col.Empty() && (pure_doc_value || col.Size() == 1)) {
+ value = &col[0];
+ }
+ RETURN_ERROR_IF_COL_IS_ARRAY((*value), type, true);
+ T parsed;
+ if (value->IsInt64() || value->IsUint64()) {
+ __int128 number = value->IsInt64() ?
static_cast<__int128>(value->GetInt64())
+ :
static_cast<__int128>(value->GetUint64());
+ if (number < StringParser::numeric_limits<T>(true) ||
+ number > StringParser::numeric_limits<T>(false)) {
+ return Status::RuntimeError("ES integer value is out of range for
{}",
+ type_to_string(type));
+ }
+ parsed = static_cast<T>(number);
} else {
- DCHECK(sizeof(T) == 16);
- memcpy(slot, &v, sizeof(v));
+ if (!value->IsString() && !value->IsNumber()) {
+ RETURN_ERROR_IF_COL_IS_NOT_STRING((*value), type);
+ }
+ std::string text = value->IsString()
+ ? std::string(value->GetString(),
value->GetStringLength())
+ : json_value_to_string(*value);
Review Comment:
Fixed in c11be7b2769ebf43424deade2b5df79c8a1a30f8. Numeric floating tokens
now use a checked direct-double conversion instead of being reserialized.
Bounds are checked before casting, including the Java upper-endpoint
saturation; exact decimal parsing remains specific to long strings. Numeric
INT64_MIN controls reproduced the failure before the fix and now pass,
including live ES source projections and the upper endpoint.
--
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]
