kou commented on code in PR #51038:
URL: https://github.com/apache/arrow/pull/51038#discussion_r4033243001
##########
cpp/src/arrow/json/parser.cc:
##########
@@ -762,43 +739,196 @@ 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_);
+ template <typename Handler>
+ Status DoParse(Handler& handler, const std::shared_ptr<Buffer>& json) {
+ RETURN_NOT_OK(ReserveScalarStorage(json->size()));
+
+ const std::string_view input(reinterpret_cast<const char*>(json->data()),
+ json->size());
+
+ if (IsWhitespaceOnly(input)) {
+ return Status::OK();
+ }
+
+ auto parse = [&](const auto& input) -> Status {
+ simdjson::ondemand::parser parser;
+ 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(handler, value));
+
+ ++num_rows_;
}
+
+ if (stream.truncated_bytes() != 0) {
+ return ParseError("The document is empty");
+ }
+
+ 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");
+
+ simdjson::padded_string padded_json(reinterpret_cast<const
char*>(json->data()),
+ json->size());
+ return parse(padded_json);
+ }
+
+ 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();
}
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()));
+ Status ParseValue(Handler& handler, sj::value value) {
+ ARROW_ASSIGN_OR_RAISE(auto type, arrow::internal::ResolveSimdjsonResult(
+ value.type(), "Failed to determine
JSON type"));
+
+ switch (type) {
+ case sj::json_type::null:
Review Comment:
It seems that we need to call `value.is_null()` to avoid accepting `nXXX` as
the `null` value:
https://github.com/simdjson/simdjson/blob/master/doc/basics.md#using-the-parsed-json
> You must still validate and consume the values (e.g., call is_null())
after calling type().
##########
cpp/src/arrow/json/parser.cc:
##########
@@ -762,43 +739,196 @@ 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_);
+ template <typename Handler>
+ Status DoParse(Handler& handler, const std::shared_ptr<Buffer>& json) {
+ RETURN_NOT_OK(ReserveScalarStorage(json->size()));
+
+ const std::string_view input(reinterpret_cast<const char*>(json->data()),
+ json->size());
+
+ if (IsWhitespaceOnly(input)) {
+ return Status::OK();
+ }
+
+ auto parse = [&](const auto& input) -> Status {
+ simdjson::ondemand::parser parser;
Review Comment:
Can we reuse `parser_` instead for performance?
##########
cpp/src/arrow/json/parser.cc:
##########
@@ -665,68 +678,32 @@ class HandlerBase : public BlockParser,
/// Accessor for a stored error Status
Status Error() { return status_; }
- /// \defgroup rapidjson-handler-interface functions expected by rj::Reader
- ///
- /// bool Key(const char* data, rj::SizeType size, ...) is omitted since
- /// the behavior varies greatly between UnexpectedFieldBehaviors
- ///
- /// @{
- bool Null() {
- status_ = builder_set_.AppendNull(builder_stack_.back(), field_index_,
builder_);
- return status_.ok();
+ Status Null() {
+ return builder_set_.AppendNull(builder_stack_.back(), field_index_,
builder_);
}
- bool Bool(bool value) {
+ Status Bool(bool value) {
constexpr auto kind = Kind::kBoolean;
if (ARROW_PREDICT_FALSE(builder_.kind != kind)) {
- status_ = IllegallyChangedTo(kind);
- return status_.ok();
+ return IllegallyChangedTo(kind);
}
- status_ = Cast<kind>(builder_)->Append(value);
- return status_.ok();
+ return Cast<kind>(builder_)->Append(value);
}
- bool RawNumber(const char* data, rj::SizeType size, ...) {
+ Status RawNumber(std::string_view value) {
if (builder_.kind == Kind::kNumberOrString) {
- status_ =
- AppendScalar<Kind::kNumberOrString>(builder_, std::string_view(data,
size));
- } else {
- status_ = AppendScalar<Kind::kNumber>(builder_, std::string_view(data,
size));
+ return AppendScalar<Kind::kNumberOrString>(builder_, value);
}
- return status_.ok();
+ return AppendScalar<Kind::kNumber>(builder_, value);
Review Comment:
How about using `if {} else {}` instead of early return because both of them
are normal cases (not error cases)?
```cpp
if (builder_.kind == Kind::kNumberOrString) {
return AppendScalar<Kind::kNumberOrString>(builder_, value);
} else {
return AppendScalar<Kind::kNumber>(builder_, value);
}
```
--
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]
