taepper commented on code in PR #50725:
URL: https://github.com/apache/arrow/pull/50725#discussion_r3692089327
##########
cpp/src/arrow/json/json_writer_internal.cc:
##########
@@ -99,140 +100,98 @@ void JsonWriter::Double(double value) {
}
Status JsonWriter::WriteValue(sj::value value) {
- sj::json_type type;
- if (auto error = value.type().get(type); error != simdjson::SUCCESS) {
- return Status::Invalid(simdjson::error_message(error));
- }
+ return internal::VisitJsonValue(
+ value,
- switch (type) {
- case sj::json_type::object: {
- StartObject();
+ [&](sj::object object) -> Status {
+ StartObject();
- sj::object object;
- if (auto error = value.get_object().get(object); error !=
simdjson::SUCCESS) {
- return Status::Invalid(simdjson::error_message(error));
- }
+ for (auto field : object) {
+ ARROW_ASSIGN_OR_RAISE(
+ auto key, internal::GetSimdjsonResult(field.unescaped_key(),
+ "Failed to get object key:
"));
- for (auto field : object) {
- std::string_view key;
- if (auto error = field.unescaped_key().get(key); error !=
simdjson::SUCCESS) {
- return Status::Invalid(simdjson::error_message(error));
- }
+ Key(key);
- Key(key);
+ ARROW_ASSIGN_OR_RAISE(
+ auto field_value,
+ internal::GetSimdjsonResult(field.value(), "Failed to get object
value: "));
- sj::value field_value;
- if (auto error = field.value().get(field_value); error !=
simdjson::SUCCESS) {
- return Status::Invalid(simdjson::error_message(error));
+ RETURN_NOT_OK(WriteValue(field_value));
}
- RETURN_NOT_OK(WriteValue(field_value));
- }
-
- EndObject();
- break;
- }
+ EndObject();
+ return Status::OK();
+ },
- case sj::json_type::array: {
- StartArray();
+ [&](sj::array array) -> Status {
+ StartArray();
- sj::array array;
- if (auto error = value.get_array().get(array); error !=
simdjson::SUCCESS) {
- return Status::Invalid(simdjson::error_message(error));
- }
+ for (auto element : array) {
+ ARROW_ASSIGN_OR_RAISE(
+ auto element_value,
+ internal::GetSimdjsonResult(element, "Failed to iterate JSON
array: "));
- for (auto element : array) {
- sj::value element_value;
- if (auto error = element.get(element_value); error !=
simdjson::SUCCESS) {
- return Status::Invalid(simdjson::error_message(error));
+ RETURN_NOT_OK(WriteValue(element_value));
}
- RETURN_NOT_OK(WriteValue(element_value));
- }
+ EndArray();
+ return Status::OK();
+ },
+
+ [&](std::string_view string_value) -> Status {
+ String(string_value);
+ return Status::OK();
+ },
+
+ [&](bool bool_value) -> Status {
+ Bool(bool_value);
+ return Status::OK();
+ },
+
+ [&]() -> Status {
+ Null();
+ return Status::OK();
+ },
+
+ [&](sj::value number_value) -> Status {
Review Comment:
Ah, that is a rather annoying problem, I see. As this will be the only user
of the library-function, we cannot see how good a solution will fit more
general usages.
I see two options:
- Provide another `BigIntegerFn` to the visitor
- Move the switch on `number_type` also to the visitor and provide all four
call-backs {`DoubleFn`,`Int64Fn`,`Uint64Fn`,`BigIntegerFn`}, dropping the
argument `NumberFn`, instead. This would correspond to the `number_type` enum
in `simdjson`.
```
enum class number_type {
floating_point_number=1, /// a binary64 number
signed_integer, /// a signed integer that fits in a 64-bit word
using two's complement
unsigned_integer, /// a positive integer larger or equal to 1<<63
big_integer /// a big integer that does not fit in a 64-bit
word
};
```
If all options of `number_type` could fit into the `number`-struct's
representation this would not be a problem, but I guess this has been done
because implementations that want to error on `big_integer` would need to pay a
performance cost otherwise.
--
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]
