github-actions[bot] commented on code in PR #68484:
URL: https://github.com/apache/doris/pull/68484#discussion_r4102954846
##########
be/src/util/jsonb_parser_simd.h:
##########
@@ -244,38 +233,139 @@ struct JsonbParser {
return Status::OK();
}
- static Status write_number(simdjson::ondemand::number num,
- simdjson ::ondemand::number_type num_type,
+ // raw_json_token() spans up to the start of the next token, so it may end
with JSON
+ // whitespace that is not part of the number.
+ static std::string_view trim_trailing_whitespace(std::string_view token) {
+ while (!token.empty() && (token.back() == ' ' || token.back() == '\t'
||
+ token.back() == '\n' || token.back() ==
'\r')) {
+ token.remove_suffix(1);
+ }
+ return token;
+ }
+
+ // Error messages quote the offending token so that the bad value can be
located, but
+ // the token is as long as the input (a malformed row may be a
multi-megabyte digit run)
+ // and tolerant callers such as json_valid or the error-to-null variants
discard the
+ // message right away. Keep the quoted part bounded and report the full
length instead.
+ static std::string quote_token(std::string_view token) {
+ constexpr size_t kMaxQuotedTokenLen = 64;
+ token = trim_trailing_whitespace(token);
+ if (token.size() <= kMaxQuotedTokenLen) {
+ return std::string(token);
+ }
+ return fmt::format("{}... (truncated, {} bytes)", token.substr(0,
kMaxQuotedTokenLen),
+ token.size());
+ }
+
+ // Matches the JSON number grammar exactly:
+ // -?(0|[1-9][0-9]*)(\.[0-9]+)?([eE][+-]?[0-9]+)?
+ static bool is_json_number(std::string_view token) {
+ size_t i = 0;
+ const size_t n = token.size();
+ auto skip_digits = [&]() {
+ const size_t start = i;
+ while (i < n && token[i] >= '0' && token[i] <= '9') {
+ ++i;
+ }
+ return i > start;
+ };
+ if (i < n && token[i] == '-') {
+ ++i;
+ }
+ if (i < n && token[i] == '0') {
+ ++i;
+ } else if (!skip_digits()) {
+ return false;
+ }
+ if (i < n && token[i] == '.') {
+ ++i;
+ if (!skip_digits()) {
+ return false;
+ }
+ }
+ if (i < n && (token[i] == 'e' || token[i] == 'E')) {
+ ++i;
+ if (i < n && (token[i] == '+' || token[i] == '-')) {
+ ++i;
+ }
+ if (!skip_digits()) {
+ return false;
+ }
+ }
+ return i == n;
+ }
+
+ // According to https://github.com/simdjson/simdjson/pull/2139, integers
that do not fit
+ // in 64 bits can be handled by parsing the raw_json_token ourselves:
simdjson returns
+ // NUMBER_ERROR for 18446744073709551616 (one above uint64 max) and
BIGINT_ERROR for
+ // longer integers such as 18446744073709551616231231.
+ // However NUMBER_ERROR is also what simdjson returns for malformed tokens
(leading
+ // zeros like 01, a trailing dot like 1., 1e, trailing garbage like 1x)
and for values
+ // beyond the double range. `num` carries nothing usable in any of these
cases, so the
+ // raw token is first checked against the JSON number grammar and then
parsed as int128
+ // or double.
+ static Status write_number_from_token(simdjson::error_code res,
std::string_view raw_string,
+ JsonbWriter& writer) {
+ std::string_view token = trim_trailing_whitespace(raw_string);
+ if (!is_json_number(token)) {
+ return Status::InvalidArgument("simdjson get_number failed: {},
raw string is: {}",
+ simdjson::error_message(res),
quote_token(token));
+ }
+
+ // StringParser::string_to_int silently truncates a fraction, so only
a token made of
+ // digits may be parsed as an integer.
+ if (token.find_first_of(".eE") == std::string_view::npos) {
+ StringParser::ParseResult result;
+ auto val = StringParser::string_to_int<int128_t>(token.data(),
token.size(), &result);
+ if (result == StringParser::PARSE_SUCCESS) {
+ if (!writer.writeInt128(val)) {
+ return Status::InvalidArgument("writeInt128 failed");
+ }
+ return Status::OK();
+ }
+ }
+
+ // Either a floating point number or an integer beyond int128.
Converting it to double
+ // may lose precision, but for JSON, exchanging data as plain text
between different
+ // systems may inherently cause precision loss.
+ StringParser::ParseResult result;
+ double double_val =
+ StringParser::string_to_float<double>(token.data(),
token.size(), &result);
+ if (result != StringParser::PARSE_SUCCESS ||
!std::isfinite(double_val)) {
+ return Status::InvalidArgument("invalid number, raw string is:
{}", quote_token(token));
+ }
+ if (!writer.writeDouble(double_val)) {
+ return Status::InvalidArgument("writeDouble failed");
+ }
+ return Status::OK();
+ }
+
+ static Status write_number(simdjson::error_code res,
simdjson::ondemand::number num,
std::string_view raw_string, JsonbWriter&
writer) {
- // The simdjson library supports four types of numbers:
+ switch (res) {
+ case simdjson::error_code::SUCCESS:
Review Comment:
[P2] Preserve the bounded diagnostic through CAST-to-JSONB.
`write_number_from_token()` now limits the parser's raw-token context, but
`ParseJsonbFromString::parse_json` still calls `str.to_string()` when wrapping
every failure (`be/src/exprs/function/cast/cast_to_jsonb.h:162-165`). For a
multi-megabyte malformed value, both strict and tolerant CAST allocate/copy the
complete input, and tolerant mode then discards the status. Please use a
bounded prefix plus length, or otherwise avoid materializing the full source,
so this downstream caller receives the same memory-amplification protection.
This is distinct from the existing parser-side diagnostic thread.
--
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]
