Copilot commented on code in PR #51038:
URL: https://github.com/apache/arrow/pull/51038#discussion_r4190308493
##########
cpp/src/arrow/json/parser.cc:
##########
@@ -27,17 +27,14 @@
#include <utility>
#include <vector>
-#include "arrow/json/rapidjson_defs.h"
-#include "rapidjson/error/en.h"
-#include "rapidjson/reader.h"
-
#include "arrow/array.h"
#include "arrow/array/builder_binary.h"
#include "arrow/buffer_builder.h"
#include "arrow/type.h"
#include "arrow/util/bitset_stack_internal.h"
#include "arrow/util/checked_cast.h"
#include "arrow/util/logging_internal.h"
+#include "arrow/util/simdjson_internal.h"
Review Comment:
This source no longer includes RapidJSON, but `ARROW_JSON` still enables
`ARROW_WITH_RAPIDJSON` and the JSON CMake/Meson test and benchmark targets
still depend on `RapidJSON`/`rapidjson_dep` (for example,
`cpp/src/arrow/cmake_modules/ThirdpartyToolchain.cmake:413-415` and
`cpp/src/arrow/json/CMakeLists.txt:28-37`). Builds therefore continue to fetch
and require the dependency that this migration is meant to remove. Remove those
build dependencies (retaining the compatibility header only if it is
intentionally part of the public API).
##########
cpp/src/arrow/json/parser.cc:
##########
@@ -762,102 +744,206 @@ class HandlerBase : public BlockParser,
}
protected:
- template <typename Handler, typename Stream>
- Status DoParse(Handler& handler, Stream&& json, size_t json_size) {
- constexpr auto parse_flags = rj::kParseIterativeFlag |
rj::kParseNanAndInfFlag |
- rj::kParseStopWhenDoneFlag |
- rj::kParseNumbersAsStringsFlag;
-
- rj::Reader reader;
- // ensure that the loop can exit when the block too large.
- for (; num_rows_ < std::numeric_limits<int32_t>::max(); ++num_rows_) {
- auto ok = reader.Parse<parse_flags>(json, handler);
- switch (ok.Code()) {
- case rj::kParseErrorNone:
- // parse the next object
- continue;
- case rj::kParseErrorDocumentEmpty:
- if (json.Tell() < json_size) {
- return ParseError(rj::GetParseError_En(ok.Code()));
- }
- // parsed all objects, finish
- return Status::OK();
- case rj::kParseErrorTermination:
- // handler emitted an error
- return handler.Error();
- default:
- // rj emitted an error
- return ParseError(rj::GetParseError_En(ok.Code()), " in row ",
num_rows_);
+ Status Parse(const std::shared_ptr<Buffer>& json) override {
+ RETURN_NOT_OK(ReserveScalarStorage(json->size()));
+
+ const std::string_view input(reinterpret_cast<const char*>(json->data()),
+ json->size());
+ // TODO(GH-51809): Remove this check once Arrow requires simdjson 5.0.2,
+ // which fixes truncated_bytes() for empty input.
+ const int64_t input_size = input.size();
+ if (internal::ConsumeJsonWhitespace(input, /*trailing=*/false) ==
input_size) {
+ return Status::OK();
+ }
+
+ auto parse = [&](const auto& input) -> Status {
+ ARROW_ASSIGN_OR_RAISE(auto stream,
arrow::internal::ResolveSimdjsonResult(
+ parser_.iterate_many(input),
+ "Failed to create JSON document
stream"));
+
+ for (auto document_result : stream) {
+ ARROW_ASSIGN_OR_RAISE(
+ auto document,
+ arrow::internal::ResolveSimdjsonResult(
+ document_result, "Failed to iterate JSON document stream"));
+
+ if (num_rows_ == std::numeric_limits<int32_t>::max()) {
+ return Status::Invalid("Row count overflowed int32_t");
+ }
+
+ ARROW_ASSIGN_OR_RAISE(
+ auto value,
+ arrow::internal::ResolveSimdjsonResult(
+ document.get_value(), "JSON parse error: Failed to get JSON
value"));
+
+ RETURN_NOT_OK(ParseValue(value));
+
+ ++num_rows_;
}
+
+ if (stream.truncated_bytes() != 0) {
+ return ParseError("JSON document was truncated");
+ }
+
+ return Status::OK();
+ };
+
+ if (json->capacity() - json->size() >=
+ static_cast<int64_t>(simdjson::SIMDJSON_PADDING)) {
+ const auto padded_json = simdjson::padded_string_view(
+ reinterpret_cast<const char*>(json->data()), json->size(),
json->capacity());
+ return parse(padded_json);
}
- return Status::Invalid("Row count overflowed int32_t");
+
+ // TODO(GH-51463): Investigate allocating input buffers with enough
capacity
+ // for SIMDJSON padding to avoid copying in this case.
+ simdjson::padded_string padded_json(reinterpret_cast<const
char*>(json->data()),
+ json->size());
+ return parse(padded_json);
}
- template <typename Handler>
- Status DoParse(Handler& handler, const std::shared_ptr<Buffer>& json) {
- RETURN_NOT_OK(ReserveScalarStorage(json->size()));
- rj::MemoryStream ms(reinterpret_cast<const char*>(json->data()),
json->size());
- using InputStream = rj::EncodedInputStream<rj::UTF8<>, rj::MemoryStream>;
- return DoParse(handler, InputStream(ms),
static_cast<size_t>(json->size()));
+ template <Kind::type kind>
+ Status MaybePromoteFromNull() {
+ if (builder_.kind != Kind::kNull) {
+ return Status::OK();
+ }
+
+ auto parent = builder_stack_.back();
+
+ if (parent.kind == Kind::kArray) {
+ auto list_builder = Cast<Kind::kArray>(parent);
+ DCHECK_EQ(list_builder->value_builder(), builder_);
+
+ RETURN_NOT_OK(builder_set_.MakeBuilder<kind>(builder_.index, &builder_));
+
+ list_builder = Cast<Kind::kArray>(parent);
+ list_builder->value_builder(builder_);
+ } else {
+ auto struct_builder = Cast<Kind::kObject>(parent);
+ DCHECK_EQ(struct_builder->field_builder(field_index_), builder_);
+
+ RETURN_NOT_OK(builder_set_.MakeBuilder<kind>(builder_.index, &builder_));
+
+ struct_builder = Cast<Kind::kObject>(parent);
+ struct_builder->field_builder(field_index_, builder_);
+ }
+
+ return Status::OK();
}
- /// \defgroup handlerbase-append-methods append non-nested values
- ///
- /// @{
+ Status ParseValue(sj::value value) {
+ ARROW_ASSIGN_OR_RAISE(auto type, arrow::internal::ResolveSimdjsonResult(
+ value.type(), "Failed to determine
JSON type"));
- template <Kind::type kind>
- Status AppendScalar(BuilderPtr builder, std::string_view scalar) {
- if (ARROW_PREDICT_FALSE(builder.kind != kind)) {
- return IllegallyChangedTo(kind);
+ switch (type) {
+ case sj::json_type::null: {
+ ARROW_ASSIGN_OR_RAISE([[maybe_unused]] auto is_null,
+ arrow::internal::ResolveSimdjsonResult(
+ value.is_null(), "Failed to validate JSON
null"));
+ return Null();
+ }
+
+ case sj::json_type::boolean: {
+ RETURN_NOT_OK(MaybePromoteFromNull<Kind::kBoolean>());
+
+ ARROW_ASSIGN_OR_RAISE(auto boolean,
+ arrow::internal::ResolveSimdjsonResult(
+ value.get_bool(), "Failed to get JSON
boolean"));
+ return Bool(boolean);
+ }
+
+ case sj::json_type::string: {
+ RETURN_NOT_OK(MaybePromoteFromNull<Kind::kString>());
+
+ ARROW_ASSIGN_OR_RAISE(auto string,
+ arrow::internal::ResolveSimdjsonResult(
+ value.get_string(), "Failed to get JSON
string"));
+ return String(string);
+ }
+
+ case sj::json_type::number: {
+ RETURN_NOT_OK(MaybePromoteFromNull<Kind::kNumber>());
+ auto raw_number = value.raw_json_token();
+ RETURN_NOT_OK(arrow::internal::ResolveSimdjsonResult(
+ value.get_number(), "Failed to parse JSON number"));
+ raw_number.remove_suffix(
+ internal::ConsumeJsonWhitespace(raw_number, /*trailing=*/true));
+ return RawNumber(raw_number);
+ }
+
+ case sj::json_type::array:
+ RETURN_NOT_OK(MaybePromoteFromNull<Kind::kArray>());
+ return ParseArray(value);
+
+ case sj::json_type::object:
+ RETURN_NOT_OK(MaybePromoteFromNull<Kind::kObject>());
+ return ParseObject(value);
+
+ case sj::json_type::unknown:
+ return ParseError("Invalid JSON value");
Review Comment:
This strict simdjson path drops the previous `kParseNanAndInfFlag` behavior:
the JSON reader used to accept `NaN`, `Inf`, and `Infinity` as numeric tokens,
while the ondemand parser now classifies them as an invalid/unknown value
before the raw token can be stored. That is a user-visible regression despite
the PR claiming no user-facing changes; preserve the existing non-finite-number
handling (and add a regression test) or explicitly document the breaking change.
--
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]
