From ad6f313d8fa7f7f042272d67690e5ad93ae6610f Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 30 Jul 2026 06:17:27 +0530 Subject: [PATCH 1/8] Add JsonWriter::WriteValue for simdjson values --- cpp/src/arrow/json/json_writer_internal.cc | 113 ++++++++++++++++++ cpp/src/arrow/json/json_writer_internal.h | 3 + .../arrow/json/json_writer_internal_test.cc | 77 +++++++++++- 3 files changed, 192 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index 446c7f06f4bf..1a887258a640 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -19,6 +19,8 @@ namespace arrow::json { +namespace sj = simdjson::ondemand; + void JsonWriter::StartObject() { MaybeComma(); builder_.start_object(); @@ -96,6 +98,117 @@ void JsonWriter::Double(double value) { needs_comma_ = true; } +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)); + } + + switch (type) { + case sj::json_type::object: { + 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) { + std::string_view key; + if (auto error = field.unescaped_key().get(key); error != simdjson::SUCCESS) { + return Status::Invalid(simdjson::error_message(error)); + } + + Key(key); + + 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)); + } + + EndObject(); + break; + } + + case sj::json_type::array: { + 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) { + 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)); + } + + EndArray(); + break; + } + + case sj::json_type::string: { + std::string_view string_value; + if (auto error = value.get_string().get(string_value); error != simdjson::SUCCESS) { + return Status::Invalid(simdjson::error_message(error)); + } + + String(string_value); + break; + } + + case sj::json_type::boolean: { + bool bool_value; + if (auto error = value.get_bool().get(bool_value); error != simdjson::SUCCESS) { + return Status::Invalid(simdjson::error_message(error)); + } + + Bool(bool_value); + break; + } + + case sj::json_type::null: { + Null(); + break; + } + + case sj::json_type::number: { + int64_t int_value; + if (value.get_int64().get(int_value) == simdjson::SUCCESS) { + Int64(int_value); + break; + } + + uint64_t uint_value; + if (value.get_uint64().get(uint_value) == simdjson::SUCCESS) { + Uint64(uint_value); + break; + } + + double double_value; + if (value.get_double().get(double_value) == simdjson::SUCCESS) { + Double(double_value); + break; + } + + return Status::Invalid("Failed to convert JSON number"); + } + + case sj::json_type::unknown: { + return Status::Invalid("Unknown JSON type"); + } + } + + return Status::OK(); +} + void JsonWriter::Null() { MaybeComma(); builder_.append_null(); diff --git a/cpp/src/arrow/json/json_writer_internal.h b/cpp/src/arrow/json/json_writer_internal.h index e6043bd7e5ac..4a8200f1df30 100644 --- a/cpp/src/arrow/json/json_writer_internal.h +++ b/cpp/src/arrow/json/json_writer_internal.h @@ -23,6 +23,7 @@ #include #include "arrow/result.h" +#include "arrow/status.h" #include "arrow/util/visibility.h" namespace arrow::json { @@ -51,6 +52,8 @@ class ARROW_EXPORT JsonWriter { void Double(double value); + Status WriteValue(simdjson::ondemand::value value); + void Null(); void StringField(std::string_view key, std::string_view value); diff --git a/cpp/src/arrow/json/json_writer_internal_test.cc b/cpp/src/arrow/json/json_writer_internal_test.cc index abdf9be7af09..b97d503c0069 100644 --- a/cpp/src/arrow/json/json_writer_internal_test.cc +++ b/cpp/src/arrow/json/json_writer_internal_test.cc @@ -16,9 +16,11 @@ // under the License. #include +#include "arrow/testing/gtest_util.h" #include "arrow/json/json_writer_internal.h" -#include "arrow/testing/gtest_util.h" + +namespace sj = simdjson::ondemand; namespace arrow::json { @@ -170,4 +172,77 @@ TEST(JsonWriter, StringWithExplicitLength) { EXPECT_EQ(json, R"({"value":"abc"})"); } +TEST(JsonWriter, WriteValueSimpleObject) { + sj::parser parser; + std::string json_str = R"({"a":42,"b":"hello"})"; + simdjson::padded_string json(json_str); + + sj::document doc; + ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + + JsonWriter writer; + ASSERT_OK(writer.WriteValue(doc)); + + EXPECT_EQ(writer.GetString(), R"({"a":42,"b":"hello"})"); +} + +TEST(JsonWriter, WriteValueNestedObject) { + sj::parser parser; + std::string json_str = R"({"child":{"x":true}})"; + simdjson::padded_string json(json_str); + + sj::document doc; + ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + + JsonWriter writer; + ASSERT_OK(writer.WriteValue(doc)); + + EXPECT_EQ(writer.GetString(), R"({"child":{"x":true}})"); +} + +TEST(JsonWriter, WriteValueObjectWithArray) { + sj::parser parser; + std::string json_str = R"({"values":[1,2,3]})"; + simdjson::padded_string json(json_str); + + sj::document doc; + ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + + JsonWriter writer; + ASSERT_OK(writer.WriteValue(doc)); + + EXPECT_EQ(writer.GetString(), R"({"values":[1,2,3]})"); +} + +TEST(JsonWriter, WriteValueComplexObject) { + sj::parser parser; + std::string json_str = + R"({"name":"arrow","version":1,"enabled":true,"values":[1,2.5,null,{"nested":[false,{"x":10}]}]})"; + simdjson::padded_string json(json_str); + + sj::document doc; + ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + + JsonWriter writer; + ASSERT_OK(writer.WriteValue(doc)); + + EXPECT_EQ( + writer.GetString(), + R"({"name":"arrow","version":1,"enabled":true,"values":[1,2.5,null,{"nested":[false,{"x":10}]}]})"); +} + +TEST(JsonWriter, WriteValueEmptyObject) { + sj::parser parser; + std::string json_str = "{}"; + simdjson::padded_string json(json_str); + + sj::document doc; + ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + + JsonWriter writer; + ASSERT_OK(writer.WriteValue(doc)); + + EXPECT_EQ(writer.GetString(), "{}"); +} + } // namespace arrow::json From 042951d6612d5bd2842b4d35f86280c0ac9ad938 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 30 Jul 2026 19:50:19 +0530 Subject: [PATCH 2/8] Use document::get_value() in JsonWriter tests --- cpp/src/arrow/json/json_writer_internal.cc | 30 ++++++++++--------- .../arrow/json/json_writer_internal_test.cc | 27 +++++++++++++---- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index 1a887258a640..6d11230f93eb 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -180,25 +180,27 @@ Status JsonWriter::WriteValue(sj::value value) { } case sj::json_type::number: { - int64_t int_value; - if (value.get_int64().get(int_value) == simdjson::SUCCESS) { - Int64(int_value); - break; + sj::number number; + if (auto error = value.get_number().get(number); error != simdjson::SUCCESS) { + return Status::Invalid("Failed to convert JSON number: ", + simdjson::error_message(error)); } - uint64_t uint_value; - if (value.get_uint64().get(uint_value) == simdjson::SUCCESS) { - Uint64(uint_value); - break; - } + switch (number.get_number_type()) { + case sj::number_type::signed_integer: + Int64(number.get_int64()); + break; + + case sj::number_type::unsigned_integer: + Uint64(number.get_uint64()); + break; - double double_value; - if (value.get_double().get(double_value) == simdjson::SUCCESS) { - Double(double_value); - break; + case sj::number_type::floating_point_number: + Double(number.get_double()); + break; } - return Status::Invalid("Failed to convert JSON number"); + break; } case sj::json_type::unknown: { diff --git a/cpp/src/arrow/json/json_writer_internal_test.cc b/cpp/src/arrow/json/json_writer_internal_test.cc index b97d503c0069..c36c4c088fb3 100644 --- a/cpp/src/arrow/json/json_writer_internal_test.cc +++ b/cpp/src/arrow/json/json_writer_internal_test.cc @@ -16,9 +16,9 @@ // under the License. #include -#include "arrow/testing/gtest_util.h" #include "arrow/json/json_writer_internal.h" +#include "arrow/testing/gtest_util.h" namespace sj = simdjson::ondemand; @@ -180,8 +180,11 @@ TEST(JsonWriter, WriteValueSimpleObject) { sj::document doc; ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + sj::value value; + ASSERT_EQ(doc.get_value().get(value), simdjson::SUCCESS); + JsonWriter writer; - ASSERT_OK(writer.WriteValue(doc)); + ASSERT_OK(writer.WriteValue(value)); EXPECT_EQ(writer.GetString(), R"({"a":42,"b":"hello"})"); } @@ -194,8 +197,11 @@ TEST(JsonWriter, WriteValueNestedObject) { sj::document doc; ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + sj::value value; + ASSERT_EQ(doc.get_value().get(value), simdjson::SUCCESS); + JsonWriter writer; - ASSERT_OK(writer.WriteValue(doc)); + ASSERT_OK(writer.WriteValue(value)); EXPECT_EQ(writer.GetString(), R"({"child":{"x":true}})"); } @@ -208,8 +214,11 @@ TEST(JsonWriter, WriteValueObjectWithArray) { sj::document doc; ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + sj::value value; + ASSERT_EQ(doc.get_value().get(value), simdjson::SUCCESS); + JsonWriter writer; - ASSERT_OK(writer.WriteValue(doc)); + ASSERT_OK(writer.WriteValue(value)); EXPECT_EQ(writer.GetString(), R"({"values":[1,2,3]})"); } @@ -223,8 +232,11 @@ TEST(JsonWriter, WriteValueComplexObject) { sj::document doc; ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + sj::value value; + ASSERT_EQ(doc.get_value().get(value), simdjson::SUCCESS); + JsonWriter writer; - ASSERT_OK(writer.WriteValue(doc)); + ASSERT_OK(writer.WriteValue(value)); EXPECT_EQ( writer.GetString(), @@ -239,8 +251,11 @@ TEST(JsonWriter, WriteValueEmptyObject) { sj::document doc; ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + sj::value value; + ASSERT_EQ(doc.get_value().get(value), simdjson::SUCCESS); + JsonWriter writer; - ASSERT_OK(writer.WriteValue(doc)); + ASSERT_OK(writer.WriteValue(value)); EXPECT_EQ(writer.GetString(), "{}"); } From 4b38499a8bf660d80a348440979f1cb9a96cf21b Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 30 Jul 2026 23:24:13 +0530 Subject: [PATCH 3/8] Handle big integers in JsonWriter::WriteValue --- cpp/src/arrow/json/json_writer_internal.cc | 26 ++++++++++++++++++- .../arrow/json/json_writer_internal_test.cc | 24 +++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index 6d11230f93eb..8aaa4ba9357f 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -180,13 +180,33 @@ Status JsonWriter::WriteValue(sj::value value) { } case sj::json_type::number: { + auto number_type_result = value.get_number_type(); + sj::number_type number_type; + if (auto error = std::move(number_type_result).get(number_type); + error != simdjson::SUCCESS) { + return Status::Invalid("Failed to determine JSON number type: ", + simdjson::error_message(error)); + } + + if (number_type == sj::number_type::big_integer) { + auto raw_json_result = simdjson::to_json_string(value); + std::string_view raw_json; + if (auto error = std::move(raw_json_result).get(raw_json); + error != simdjson::SUCCESS) { + return Status::Invalid("Failed to get raw JSON: ", + simdjson::error_message(error)); + } + RawValue(raw_json); + break; + } + sj::number number; if (auto error = value.get_number().get(number); error != simdjson::SUCCESS) { return Status::Invalid("Failed to convert JSON number: ", simdjson::error_message(error)); } - switch (number.get_number_type()) { + switch (number_type) { case sj::number_type::signed_integer: Int64(number.get_int64()); break; @@ -198,6 +218,10 @@ Status JsonWriter::WriteValue(sj::value value) { case sj::number_type::floating_point_number: Double(number.get_double()); break; + + case sj::number_type::big_integer: + // Big integers are handled before calling get_number() + break; } break; diff --git a/cpp/src/arrow/json/json_writer_internal_test.cc b/cpp/src/arrow/json/json_writer_internal_test.cc index c36c4c088fb3..329d0f0d1b09 100644 --- a/cpp/src/arrow/json/json_writer_internal_test.cc +++ b/cpp/src/arrow/json/json_writer_internal_test.cc @@ -260,4 +260,28 @@ TEST(JsonWriter, WriteValueEmptyObject) { EXPECT_EQ(writer.GetString(), "{}"); } +TEST(JsonWriter, WriteValueAllNumberTypes) { + sj::parser parser; + std::string json_str = R"({ + "signed":-42, + "unsigned":18446744073709551615, + "double":2.5, + "big":184467440737095516161234567890 + })"; + simdjson::padded_string json(json_str); + + sj::document doc; + ASSERT_EQ(parser.iterate(json).get(doc), simdjson::SUCCESS); + + sj::value value; + ASSERT_EQ(doc.get_value().get(value), simdjson::SUCCESS); + + JsonWriter writer; + ASSERT_OK(writer.WriteValue(value)); + + EXPECT_EQ( + writer.GetString(), + R"({"signed":-42,"unsigned":18446744073709551615,"double":2.5,"big":184467440737095516161234567890})"); +} + } // namespace arrow::json From ae2437f350ee33974660494819cdbc2d65389b24 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Fri, 31 Jul 2026 18:53:35 +0530 Subject: [PATCH 4/8] Extract simdjson result helper --- cpp/src/arrow/json/from_string.cc | 32 +++----- cpp/src/arrow/json/json_writer_internal.cc | 86 +++++++++------------- cpp/src/arrow/util/simdjson_internal.h | 40 ++++++++++ 3 files changed, 85 insertions(+), 73 deletions(-) create mode 100644 cpp/src/arrow/util/simdjson_internal.h diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index 5694fbde4c53..7073d7d47e9b 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -47,6 +47,7 @@ #include "arrow/util/decimal.h" #include "arrow/util/float16.h" #include "arrow/util/logging_internal.h" +#include "arrow/util/simdjson_internal.h" #include "arrow/util/unreachable.h" #include "arrow/util/value_parsing.h" @@ -155,17 +156,6 @@ Result GetJsonAs(sj::value& value) { return typed_value; } -template -Result GetJsonResult( - simdjson::simdjson_result element, std::string_view error) { - SimdjsonValueType typed_value; - if (auto error_code = std::move(element).get(typed_value); - error_code != simdjson::SUCCESS) { - return Status::Invalid(error, simdjson::error_message(error_code)); - } - return typed_value; -} - // Result because peeking the nonRootScalar can fail (parsed lazily) Result IsJsonNull(sj::value& value) { bool is_null; @@ -215,7 +205,7 @@ class ConcreteConverter : public JSONConverter { int32_t num_elements = 0; for (auto element : json_array) { ARROW_ASSIGN_OR_RAISE(auto value, - GetJsonResult( + internal::GetSimdjsonResult( element, "Could not iterate elements of JSON array: ")); RETURN_NOT_OK(self->AppendValue(value)); num_elements++; @@ -395,9 +385,9 @@ Status ProcessJsonArrayElements( " elements, had ", index); } - ARROW_ASSIGN_OR_RAISE( - sj::value element, - GetJsonResult(*it, "Could not iterate elements of JSON array: ")); + ARROW_ASSIGN_OR_RAISE(sj::value element, + internal::GetSimdjsonResult( + *it, "Could not iterate elements of JSON array: ")); RETURN_NOT_OK(handler(element)); ++it; ++index; @@ -760,8 +750,8 @@ class MapConverter final : public ConcreteConverter { for (auto json_pair_result : array) { ARROW_ASSIGN_OR_RAISE( auto json_pair, - GetJsonResult(json_pair_result, - "Could not iterate elements of JSON array: ")); + internal::GetSimdjsonResult( + json_pair_result, "Could not iterate elements of JSON array: ")); ARROW_ASSIGN_OR_RAISE(auto json_pair_array, GetJsonAs(json_pair)); RETURN_NOT_OK(ProcessJsonArrayElements<2>( @@ -870,7 +860,7 @@ class StructConverter final : public ConcreteConverter { size_t i = 0; for (auto child : array) { ARROW_ASSIGN_OR_RAISE(auto child_value, - GetJsonResult( + internal::GetSimdjsonResult( child, "Could not iterate elements of JSON array: ")); RETURN_NOT_OK(child_converters_[i]->AppendValue(child_value)); ++i; @@ -885,9 +875,9 @@ class StructConverter final : public ConcreteConverter { auto num_fields = type_->num_fields(); std::vector field_seen(num_fields, false); for (auto field_result : object) { - ARROW_ASSIGN_OR_RAISE( - auto field, - GetJsonResult(field_result, "Error getting field of object: ")); + ARROW_ASSIGN_OR_RAISE(auto field, + internal::GetSimdjsonResult( + field_result, "Error getting field of object: ")); std::string_view key; if (field.unescaped_key(/*allow_replacement=*/false).get(key) != simdjson::SUCCESS) { diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index 8aaa4ba9357f..5d7d6f20d9fa 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -16,6 +16,7 @@ // under the License. #include "arrow/json/json_writer_internal.h" +#include "arrow/util/simdjson_internal.h" namespace arrow::json { @@ -99,32 +100,27 @@ 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)); - } + ARROW_ASSIGN_OR_RAISE(auto type, internal::GetSimdjsonResult( + value.type(), "Failed to determine JSON type: ")); switch (type) { case sj::json_type::object: { StartObject(); - sj::object object; - if (auto error = value.get_object().get(object); error != simdjson::SUCCESS) { - return Status::Invalid(simdjson::error_message(error)); - } + ARROW_ASSIGN_OR_RAISE( + auto object, + internal::GetSimdjsonResult(value.get_object(), "Failed to get JSON object: ")); 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)); - } + ARROW_ASSIGN_OR_RAISE(auto key, + internal::GetSimdjsonResult(field.unescaped_key(), + "Failed to get object key: ")); Key(key); - sj::value field_value; - if (auto error = field.value().get(field_value); error != simdjson::SUCCESS) { - return Status::Invalid(simdjson::error_message(error)); - } + ARROW_ASSIGN_OR_RAISE( + auto field_value, + internal::GetSimdjsonResult(field.value(), "Failed to get object value: ")); RETURN_NOT_OK(WriteValue(field_value)); } @@ -136,16 +132,14 @@ Status JsonWriter::WriteValue(sj::value value) { case sj::json_type::array: { StartArray(); - sj::array array; - if (auto error = value.get_array().get(array); error != simdjson::SUCCESS) { - return Status::Invalid(simdjson::error_message(error)); - } + ARROW_ASSIGN_OR_RAISE( + auto array, + internal::GetSimdjsonResult(value.get_array(), "Failed to get 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)); - } + ARROW_ASSIGN_OR_RAISE( + auto element_value, + internal::GetSimdjsonResult(element, "Failed to iterate JSON array: ")); RETURN_NOT_OK(WriteValue(element_value)); } @@ -155,20 +149,18 @@ Status JsonWriter::WriteValue(sj::value value) { } case sj::json_type::string: { - std::string_view string_value; - if (auto error = value.get_string().get(string_value); error != simdjson::SUCCESS) { - return Status::Invalid(simdjson::error_message(error)); - } + ARROW_ASSIGN_OR_RAISE( + auto string_value, + internal::GetSimdjsonResult(value.get_string(), "Failed to get JSON string: ")); String(string_value); break; } case sj::json_type::boolean: { - bool bool_value; - if (auto error = value.get_bool().get(bool_value); error != simdjson::SUCCESS) { - return Status::Invalid(simdjson::error_message(error)); - } + ARROW_ASSIGN_OR_RAISE( + auto bool_value, + internal::GetSimdjsonResult(value.get_bool(), "Failed to get JSON boolean: ")); Bool(bool_value); break; @@ -180,32 +172,22 @@ Status JsonWriter::WriteValue(sj::value value) { } case sj::json_type::number: { - auto number_type_result = value.get_number_type(); - sj::number_type number_type; - if (auto error = std::move(number_type_result).get(number_type); - error != simdjson::SUCCESS) { - return Status::Invalid("Failed to determine JSON number type: ", - simdjson::error_message(error)); - } + ARROW_ASSIGN_OR_RAISE( + auto number_type, + internal::GetSimdjsonResult(value.get_number_type(), + "Failed to determine JSON number type: ")); if (number_type == sj::number_type::big_integer) { - auto raw_json_result = simdjson::to_json_string(value); - std::string_view raw_json; - if (auto error = std::move(raw_json_result).get(raw_json); - error != simdjson::SUCCESS) { - return Status::Invalid("Failed to get raw JSON: ", - simdjson::error_message(error)); - } + ARROW_ASSIGN_OR_RAISE(auto raw_json, + internal::GetSimdjsonResult(simdjson::to_json_string(value), + "Failed to get raw JSON: ")); RawValue(raw_json); break; } - sj::number number; - if (auto error = value.get_number().get(number); error != simdjson::SUCCESS) { - return Status::Invalid("Failed to convert JSON number: ", - simdjson::error_message(error)); - } - + ARROW_ASSIGN_OR_RAISE( + auto number, internal::GetSimdjsonResult(value.get_number(), + "Failed to convert JSON number: ")); switch (number_type) { case sj::number_type::signed_integer: Int64(number.get_int64()); diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h new file mode 100644 index 000000000000..b5dc0a43aca0 --- /dev/null +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -0,0 +1,40 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include + +#include + +#include "arrow/result.h" +#include "arrow/status.h" + +namespace arrow { +namespace internal { + +template +Result GetSimdjsonResult(simdjson::simdjson_result result, std::string_view error) { + T value; + if (auto error_code = std::move(result).get(value); error_code != simdjson::SUCCESS) { + return Status::Invalid(error, simdjson::error_message(error_code)); + } + return value; +} + +} // namespace internal +} // namespace arrow From 00d156bb9d6fd7d372cd1310f32d6dd3b077836d Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Fri, 31 Jul 2026 19:13:05 +0530 Subject: [PATCH 5/8] Add simdjson value visitor helper --- cpp/src/arrow/json/json_writer_internal.cc | 167 +++++++++------------ cpp/src/arrow/util/simdjson_internal.h | 49 ++++++ 2 files changed, 121 insertions(+), 95 deletions(-) diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index 5d7d6f20d9fa..ca4b9cc9896e 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -100,121 +100,98 @@ void JsonWriter::Double(double value) { } Status JsonWriter::WriteValue(sj::value value) { - ARROW_ASSIGN_OR_RAISE(auto type, internal::GetSimdjsonResult( - value.type(), "Failed to determine JSON type: ")); + return internal::VisitJsonValue( + value, - switch (type) { - case sj::json_type::object: { - StartObject(); + [&](sj::object object) -> Status { + StartObject(); - ARROW_ASSIGN_OR_RAISE( - auto object, - internal::GetSimdjsonResult(value.get_object(), "Failed to get JSON object: ")); + for (auto field : object) { + ARROW_ASSIGN_OR_RAISE( + auto key, internal::GetSimdjsonResult(field.unescaped_key(), + "Failed to get object key: ")); - for (auto field : object) { - ARROW_ASSIGN_OR_RAISE(auto key, - internal::GetSimdjsonResult(field.unescaped_key(), - "Failed to get object key: ")); + Key(key); - Key(key); + ARROW_ASSIGN_OR_RAISE( + auto field_value, + internal::GetSimdjsonResult(field.value(), "Failed to get object value: ")); - ARROW_ASSIGN_OR_RAISE( - auto field_value, - internal::GetSimdjsonResult(field.value(), "Failed to get object value: ")); + RETURN_NOT_OK(WriteValue(field_value)); + } - RETURN_NOT_OK(WriteValue(field_value)); - } + EndObject(); + return Status::OK(); + }, - EndObject(); - break; - } + [&](sj::array array) -> Status { + StartArray(); - case sj::json_type::array: { - StartArray(); + for (auto element : array) { + ARROW_ASSIGN_OR_RAISE( + auto element_value, + internal::GetSimdjsonResult(element, "Failed to iterate JSON array: ")); - ARROW_ASSIGN_OR_RAISE( - auto array, - internal::GetSimdjsonResult(value.get_array(), "Failed to get JSON array: ")); + RETURN_NOT_OK(WriteValue(element_value)); + } - for (auto element : array) { - ARROW_ASSIGN_OR_RAISE( - auto element_value, - internal::GetSimdjsonResult(element, "Failed to iterate JSON array: ")); + EndArray(); + return Status::OK(); + }, - RETURN_NOT_OK(WriteValue(element_value)); - } + [&](std::string_view string_value) -> Status { + String(string_value); + return Status::OK(); + }, - EndArray(); - break; - } + [&](bool bool_value) -> Status { + Bool(bool_value); + return Status::OK(); + }, - case sj::json_type::string: { - ARROW_ASSIGN_OR_RAISE( - auto string_value, - internal::GetSimdjsonResult(value.get_string(), "Failed to get JSON string: ")); + [&]() -> Status { + Null(); + return Status::OK(); + }, - String(string_value); - break; - } + [&](sj::value number_value) -> Status { + ARROW_ASSIGN_OR_RAISE( + auto number_type, + internal::GetSimdjsonResult(number_value.get_number_type(), + "Failed to determine JSON number type: ")); + + if (number_type == sj::number_type::big_integer) { + ARROW_ASSIGN_OR_RAISE(auto raw_json, internal::GetSimdjsonResult( + simdjson::to_json_string(number_value), + "Failed to get raw JSON: ")); + RawValue(raw_json); + return Status::OK(); + } - case sj::json_type::boolean: { - ARROW_ASSIGN_OR_RAISE( - auto bool_value, - internal::GetSimdjsonResult(value.get_bool(), "Failed to get JSON boolean: ")); + ARROW_ASSIGN_OR_RAISE( + auto number, internal::GetSimdjsonResult(number_value.get_number(), + "Failed to convert JSON number: ")); - Bool(bool_value); - break; - } + switch (number_type) { + case sj::number_type::signed_integer: + Int64(number.get_int64()); + break; - case sj::json_type::null: { - Null(); - break; - } + case sj::number_type::unsigned_integer: + Uint64(number.get_uint64()); + break; - case sj::json_type::number: { - ARROW_ASSIGN_OR_RAISE( - auto number_type, - internal::GetSimdjsonResult(value.get_number_type(), - "Failed to determine JSON number type: ")); - - if (number_type == sj::number_type::big_integer) { - ARROW_ASSIGN_OR_RAISE(auto raw_json, - internal::GetSimdjsonResult(simdjson::to_json_string(value), - "Failed to get raw JSON: ")); - RawValue(raw_json); - break; - } - - ARROW_ASSIGN_OR_RAISE( - auto number, internal::GetSimdjsonResult(value.get_number(), - "Failed to convert JSON number: ")); - switch (number_type) { - case sj::number_type::signed_integer: - Int64(number.get_int64()); - break; - - case sj::number_type::unsigned_integer: - Uint64(number.get_uint64()); - break; - - case sj::number_type::floating_point_number: - Double(number.get_double()); - break; - - case sj::number_type::big_integer: - // Big integers are handled before calling get_number() - break; - } - - break; - } + case sj::number_type::floating_point_number: + Double(number.get_double()); + break; - case sj::json_type::unknown: { - return Status::Invalid("Unknown JSON type"); - } - } + case sj::number_type::big_integer: + // Big integers are handled above. + break; + } - return Status::OK(); + return Status::OK(); + }); } void JsonWriter::Null() { diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index b5dc0a43aca0..ba469cfeb305 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -36,5 +36,54 @@ Result GetSimdjsonResult(simdjson::simdjson_result result, std::string_vie return value; } +template +Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, + ArrayFn&& array_fn, StringFn&& string_fn, BoolFn&& bool_fn, + NullFn&& null_fn, NumberFn&& number_fn) { + ARROW_ASSIGN_OR_RAISE( + auto type, GetSimdjsonResult(value.type(), "Failed to determine JSON type: ")); + + switch (type) { + case simdjson::ondemand::json_type::object: { + ARROW_ASSIGN_OR_RAISE( + auto object, + GetSimdjsonResult(value.get_object(), "Failed to get JSON object: ")); + return object_fn(object); + } + + case simdjson::ondemand::json_type::array: { + ARROW_ASSIGN_OR_RAISE( + auto array, GetSimdjsonResult(value.get_array(), "Failed to get JSON array: ")); + return array_fn(array); + } + + case simdjson::ondemand::json_type::string: { + ARROW_ASSIGN_OR_RAISE( + auto string, + GetSimdjsonResult(value.get_string(), "Failed to get JSON string: ")); + return string_fn(string); + } + + case simdjson::ondemand::json_type::boolean: { + ARROW_ASSIGN_OR_RAISE( + auto boolean, + GetSimdjsonResult(value.get_bool(), "Failed to get JSON boolean: ")); + return bool_fn(boolean); + } + + case simdjson::ondemand::json_type::null: + return null_fn(); + + case simdjson::ondemand::json_type::number: + return number_fn(value); + + case simdjson::ondemand::json_type::unknown: + return Status::Invalid("Unknown JSON type"); + } + + return Status::Invalid("Unreachable"); +} + } // namespace internal } // namespace arrow From c37f6cc2f9f77f5149cf93ad0f1de0667e130d18 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Fri, 31 Jul 2026 21:36:46 +0530 Subject: [PATCH 6/8] Move simdjson helper functions --- cpp/src/arrow/json/from_string.cc | 171 +++++---------------- cpp/src/arrow/json/json_writer_internal.cc | 50 ++---- cpp/src/arrow/util/simdjson_internal.h | 150 +++++++++++++++++- 3 files changed, 200 insertions(+), 171 deletions(-) diff --git a/cpp/src/arrow/json/from_string.cc b/cpp/src/arrow/json/from_string.cc index 7073d7d47e9b..c9d910667133 100644 --- a/cpp/src/arrow/json/from_string.cc +++ b/cpp/src/arrow/json/from_string.cc @@ -67,105 +67,6 @@ using ::arrow::internal::checked_pointer_cast; namespace { -const char* JsonTypeName(sj::json_type type) { - switch (type) { - case sj::json_type::array: - return "array"; - case sj::json_type::object: - return "object"; - case sj::json_type::number: - return "number"; - case sj::json_type::string: - return "string"; - case sj::json_type::boolean: - return "boolean"; - case sj::json_type::null: - return "null"; - default: - return "unknown"; - } -} - -// Empty struct to represent the type of a simdjson null value -struct SimdjsonNull {}; - -template -struct JsonTypeNameOf; - -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "array"; -}; -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "object"; -}; -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "string"; -}; -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "boolean"; -}; -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "null"; -}; -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "number"; -}; -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "number"; -}; -template <> -struct JsonTypeNameOf { - static constexpr const char* kValue = "number"; -}; - -template -constexpr const char* JsonTypeName() { - return JsonTypeNameOf::kValue; -} - -template -Result GetJsonAs(sj::value& value) { - SimdjsonValueType typed_value{}; - simdjson::error_code error_code; - if constexpr (std::is_same_v) { - // simdjson has no get<>() for null; probe it explicitly - bool is_null; - error_code = value.is_null().get(is_null); - if (error_code == simdjson::SUCCESS && !is_null) { - error_code = simdjson::INCORRECT_TYPE; - } - } else { - error_code = value.get(typed_value); - } - if (error_code != simdjson::SUCCESS) { - sj::json_type json_type; - if (value.type().get(json_type) != simdjson::SUCCESS) { - return Status::Invalid("Expected ", JsonTypeName(), - " or null, got malformed JSON value"); - } - return Status::Invalid("Expected ", JsonTypeName(), - " or null, got JSON type ", JsonTypeName(json_type)); - } - return typed_value; -} - -// Result because peeking the nonRootScalar can fail (parsed lazily) -Result IsJsonNull(sj::value& value) { - bool is_null; - if (auto error_code = value.is_null().get(is_null); error_code != simdjson::SUCCESS) { - return Status::Invalid("Error checking for JSON null: ", - simdjson::error_message(error_code)); - } - return is_null; -} - class JSONConverter { public: virtual ~JSONConverter() = default; @@ -241,7 +142,7 @@ class NullConverter final : public ConcreteConverter { } Status AppendValue(sj::value& json_obj) override { - ARROW_RETURN_NOT_OK(GetJsonAs(json_obj)); + ARROW_RETURN_NOT_OK(internal::GetJsonAs(json_obj)); return AppendNull(); } @@ -262,7 +163,7 @@ class BooleanConverter final : public ConcreteConverter { } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return AppendNull(); } @@ -270,7 +171,7 @@ class BooleanConverter final : public ConcreteConverter { if (json_obj.get(int_value) == simdjson::SUCCESS) { return builder_->Append(int_value != 0); } - ARROW_ASSIGN_OR_RAISE(bool bool_value, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool bool_value, internal::GetJsonAs(json_obj)); return builder_->Append(bool_value); } @@ -289,7 +190,7 @@ enable_if_physical_signed_integer ConvertNumber(sj::value& json_obj, const DataType& type, typename T::c_type* out) { *out = static_cast(0); - ARROW_ASSIGN_OR_RAISE(int64_t v64, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(int64_t v64, internal::GetJsonAs(json_obj)); *out = static_cast(v64); if (*out == v64) { return Status::OK(); @@ -304,7 +205,7 @@ enable_if_unsigned_integer ConvertNumber(sj::value& json_obj, const DataType& type, typename T::c_type* out) { *out = static_cast(0); - ARROW_ASSIGN_OR_RAISE(uint64_t v64, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(uint64_t v64, internal::GetJsonAs(json_obj)); *out = static_cast(v64); if (*out == v64) { return Status::OK(); @@ -346,7 +247,7 @@ enable_if_half_float ConvertNumber(sj::value& json_obj, const DataTyp *out = Float16(f64.value()).bits(); return Status::OK(); } - ARROW_ASSIGN_OR_RAISE(auto f64, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto f64, internal::GetJsonAs(json_obj)); *out = Float16(f64).bits(); return arrow::Status::OK(); } @@ -361,7 +262,7 @@ enable_if_physical_floating_point ConvertNumber(sj::value& json_obj, *out = static_cast(f64.value()); return Status::OK(); } - ARROW_ASSIGN_OR_RAISE(auto f64, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto f64, internal::GetJsonAs(json_obj)); *out = static_cast(f64); return arrow::Status::OK(); } @@ -416,7 +317,7 @@ class IntegerConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } @@ -444,7 +345,7 @@ class FloatConverter final : public ConcreteConverterMakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } @@ -475,11 +376,12 @@ class DecimalConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } - ARROW_ASSIGN_OR_RAISE(auto string_value, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto string_value, + internal::GetJsonAs(json_obj)); int32_t precision, scale; DecimalValue d; RETURN_NOT_OK(DecimalValue::FromString(string_value, &d, &precision, &scale)); @@ -518,7 +420,7 @@ class TimestampConverter final : public ConcreteConverter { } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } @@ -553,12 +455,12 @@ class DayTimeIntervalConverter final } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto array, internal::GetJsonAs(json_obj)); DayTimeIntervalType::DayMilliseconds value; RETURN_NOT_OK(ProcessJsonArrayElements<2>( @@ -587,12 +489,12 @@ class MonthDayNanoIntervalConverter final } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto array, internal::GetJsonAs(json_obj)); MonthDayNanoIntervalType::MonthDayNanos value; RETURN_NOT_OK(ProcessJsonArrayElements<3>( @@ -627,12 +529,12 @@ class StringConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } - ARROW_ASSIGN_OR_RAISE(auto view, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto view, internal::GetJsonAs(json_obj)); return builder_->Append(view); } @@ -656,11 +558,11 @@ class FixedSizeBinaryConverter final Status Init() override { return this->MakeConcreteBuilder(&builder_); } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } - ARROW_ASSIGN_OR_RAISE(auto view, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto view, internal::GetJsonAs(json_obj)); if (view.length() != static_cast(builder_->byte_width())) { std::stringstream ss; ss << "Invalid string length " << view.length() << " in JSON input for " @@ -700,11 +602,11 @@ class VarLengthListLikeConverter final } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto array, internal::GetJsonAs(json_obj)); size_t num_elements; if (array.count_elements().get(num_elements) != simdjson::SUCCESS) { return Status::Invalid("Malformed JSON array for type ", this->type_->ToString()); @@ -740,24 +642,25 @@ class MapConverter final : public ConcreteConverter { } Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } RETURN_NOT_OK(builder_->Append()); - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto array, internal::GetJsonAs(json_obj)); for (auto json_pair_result : array) { ARROW_ASSIGN_OR_RAISE( auto json_pair, internal::GetSimdjsonResult( json_pair_result, "Could not iterate elements of JSON array: ")); - ARROW_ASSIGN_OR_RAISE(auto json_pair_array, GetJsonAs(json_pair)); + ARROW_ASSIGN_OR_RAISE(auto json_pair_array, + internal::GetJsonAs(json_pair)); RETURN_NOT_OK(ProcessJsonArrayElements<2>( json_pair_array, "key-item pair", {[this](sj::value& key) { - ARROW_ASSIGN_OR_RAISE(bool key_is_null, IsJsonNull(key)); + ARROW_ASSIGN_OR_RAISE(bool key_is_null, internal::IsJsonNull(key)); if (key_is_null) { return Status::Invalid("null key is invalid"); } @@ -793,13 +696,13 @@ class FixedSizeListConverter final : public ConcreteConverterAppendNull(); } RETURN_NOT_OK(builder_->Append()); // Extend the child converter with this JSON array - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto array, internal::GetJsonAs(json_obj)); ARROW_ASSIGN_OR_RAISE(int32_t size, child_converter_->AppendValues(array)); if (size != list_size_) { return Status::Invalid("incorrect list size ", size); @@ -842,7 +745,7 @@ class StructConverter final : public ConcreteConverter { // or an object mapping struct names to values (omitted struct members // are mapped to null). Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } @@ -867,7 +770,7 @@ class StructConverter final : public ConcreteConverter { } return builder_->Append(); } - ARROW_ASSIGN_OR_RAISE(auto object, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto object, internal::GetJsonAs(json_obj)); // Iterate the object fields in JSON order (the on-demand API is // forward-only, so per-field lookups would be quadratic and would also // compare against raw, still-escaped keys). Fields absent from the JSON @@ -951,12 +854,12 @@ class UnionConverter final : public ConcreteConverter { // Append a JSON value that must be a 2-long array, containing the type_id // and value of the UnionArray's slot. Status AppendValue(sj::value& json_obj) override { - ARROW_ASSIGN_OR_RAISE(bool is_null, IsJsonNull(json_obj)); + ARROW_ASSIGN_OR_RAISE(bool is_null, internal::IsJsonNull(json_obj)); if (is_null) { return this->AppendNull(); } - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto array, internal::GetJsonAs(json_obj)); int8_t id = 0; std::shared_ptr child_converter; @@ -964,7 +867,7 @@ class UnionConverter final : public ConcreteConverter { RETURN_NOT_OK(ProcessJsonArrayElements<2>( array, "[type_id, value] pair", {[this, &id, &child_converter](sj::value& id_elem) { - ARROW_ASSIGN_OR_RAISE(auto id_value, GetJsonAs(id_elem)); + ARROW_ASSIGN_OR_RAISE(auto id_value, internal::GetJsonAs(id_elem)); id = static_cast(id_value); auto child_num = type_id_to_child_num_[id]; if (child_num == -1) { @@ -1146,7 +1049,7 @@ Result> ArrayFromJSONString(const std::shared_ptr(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto array, internal::GetJsonAs(json_obj)); // The JSON document should be an array, append it RETURN_NOT_OK(converter->AppendValues(array)); @@ -1216,7 +1119,7 @@ Result> ScalarFromJSONString( error_code != simdjson::SUCCESS) { return Status::Invalid("JSON parse error: ", simdjson::error_message(error_code)); } - ARROW_ASSIGN_OR_RAISE(auto singleton_array, GetJsonAs(json_obj)); + ARROW_ASSIGN_OR_RAISE(auto singleton_array, internal::GetJsonAs(json_obj)); ARROW_ASSIGN_OR_RAISE(int32_t num_elements, converter->AppendValues(singleton_array)); if (num_elements != 1) { diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/json/json_writer_internal.cc index ca4b9cc9896e..567694902497 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/json/json_writer_internal.cc @@ -154,42 +154,26 @@ Status JsonWriter::WriteValue(sj::value value) { return Status::OK(); }, - [&](sj::value number_value) -> Status { - ARROW_ASSIGN_OR_RAISE( - auto number_type, - internal::GetSimdjsonResult(number_value.get_number_type(), - "Failed to determine JSON number type: ")); - - if (number_type == sj::number_type::big_integer) { - ARROW_ASSIGN_OR_RAISE(auto raw_json, internal::GetSimdjsonResult( - simdjson::to_json_string(number_value), - "Failed to get raw JSON: ")); - RawValue(raw_json); - return Status::OK(); - } - - ARROW_ASSIGN_OR_RAISE( - auto number, internal::GetSimdjsonResult(number_value.get_number(), - "Failed to convert JSON number: ")); - - switch (number_type) { - case sj::number_type::signed_integer: - Int64(number.get_int64()); - break; - - case sj::number_type::unsigned_integer: - Uint64(number.get_uint64()); - break; + [&](int64_t value) -> Status { + Int64(value); + return Status::OK(); + }, - case sj::number_type::floating_point_number: - Double(number.get_double()); - break; + [&](uint64_t value) -> Status { + Uint64(value); + return Status::OK(); + }, - case sj::number_type::big_integer: - // Big integers are handled above. - break; - } + [&](double value) -> Status { + Double(value); + return Status::OK(); + }, + [&](sj::value value) -> Status { + ARROW_ASSIGN_OR_RAISE(auto raw_json, + internal::GetSimdjsonResult(simdjson::to_json_string(value), + "Failed to get raw JSON: ")); + RawValue(raw_json); return Status::OK(); }); } diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index ba469cfeb305..8299eeed6b99 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -27,6 +27,57 @@ namespace arrow { namespace internal { +// Empty struct to represent the type of a simdjson null value +struct SimdjsonNull {}; + +template +struct JsonTypeNameOf; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "array"; +}; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "object"; +}; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "string"; +}; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "boolean"; +}; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "null"; +}; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "number"; +}; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "number"; +}; + +template <> +struct JsonTypeNameOf { + static constexpr const char* kValue = "number"; +}; + +template +constexpr const char* JsonTypeName() { + return JsonTypeNameOf::kValue; +} + template Result GetSimdjsonResult(simdjson::simdjson_result result, std::string_view error) { T value; @@ -37,10 +88,12 @@ Result GetSimdjsonResult(simdjson::simdjson_result result, std::string_vie } template + typename NullFn, typename Int64Fn, typename Uint64Fn, typename DoubleFn, + typename BigIntegerFn> Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, ArrayFn&& array_fn, StringFn&& string_fn, BoolFn&& bool_fn, - NullFn&& null_fn, NumberFn&& number_fn) { + NullFn&& null_fn, Int64Fn&& int64_fn, Uint64Fn&& uint64_fn, + DoubleFn&& double_fn, BigIntegerFn&& big_integer_fn) { ARROW_ASSIGN_OR_RAISE( auto type, GetSimdjsonResult(value.type(), "Failed to determine JSON type: ")); @@ -75,8 +128,39 @@ Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, case simdjson::ondemand::json_type::null: return null_fn(); - case simdjson::ondemand::json_type::number: - return number_fn(value); + case simdjson::ondemand::json_type::number: { + ARROW_ASSIGN_OR_RAISE(auto number_type, + GetSimdjsonResult(value.get_number_type(), + "Failed to determine JSON number type: ")); + + switch (number_type) { + case simdjson::ondemand::number_type::signed_integer: { + ARROW_ASSIGN_OR_RAISE( + auto number, + GetSimdjsonResult(value.get_int64(), "Failed to get signed integer")); + return int64_fn(number); + } + + case simdjson::ondemand::number_type::unsigned_integer: { + ARROW_ASSIGN_OR_RAISE( + auto number, + GetSimdjsonResult(value.get_uint64(), "Failed to get unsigned integer")); + return uint64_fn(number); + } + + case simdjson::ondemand::number_type::floating_point_number: { + ARROW_ASSIGN_OR_RAISE(auto number, + GetSimdjsonResult(value.get_double(), + "Failed to get floating-point number")); + return double_fn(number); + } + + case simdjson::ondemand::number_type::big_integer: + return big_integer_fn(value); + } + + return Status::Invalid("Unknown JSON number type"); + } case simdjson::ondemand::json_type::unknown: return Status::Invalid("Unknown JSON type"); @@ -85,5 +169,63 @@ Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, return Status::Invalid("Unreachable"); } +inline const char* JsonTypeName(simdjson::ondemand::json_type type) { + switch (type) { + case simdjson::ondemand::json_type::array: + return "array"; + case simdjson::ondemand::json_type::object: + return "object"; + case simdjson::ondemand::json_type::number: + return "number"; + case simdjson::ondemand::json_type::string: + return "string"; + case simdjson::ondemand::json_type::boolean: + return "boolean"; + case simdjson::ondemand::json_type::null: + return "null"; + default: + return "unknown"; + } +} + +// Result because peeking the nonRootScalar can fail (parsed lazily) +inline Result IsJsonNull(simdjson::ondemand::value& value) { + bool is_null; + if (auto error_code = value.is_null().get(is_null); error_code != simdjson::SUCCESS) { + return Status::Invalid("Error checking for JSON null: ", + simdjson::error_message(error_code)); + } + return is_null; +} + +template +Result GetJsonAs(simdjson::ondemand::value& value) { + SimdjsonValueType typed_value{}; + simdjson::error_code error_code; + + if constexpr (std::is_same_v) { + // simdjson has no get<>() for null; probe it explicitly + bool is_null; + error_code = value.is_null().get(is_null); + if (error_code == simdjson::SUCCESS && !is_null) { + error_code = simdjson::INCORRECT_TYPE; + } + } else { + error_code = value.get(typed_value); + } + + if (error_code != simdjson::SUCCESS) { + simdjson::ondemand::json_type json_type; + if (value.type().get(json_type) != simdjson::SUCCESS) { + return Status::Invalid("Expected ", JsonTypeName(), + " or null, got malformed JSON value"); + } + return Status::Invalid("Expected ", JsonTypeName(), + " or null, got JSON type ", JsonTypeName(json_type)); + } + + return typed_value; +} + } // namespace internal } // namespace arrow From 6cefe0383e62531f681ad50ee3554941043c5e45 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Sat, 1 Aug 2026 10:46:11 +0530 Subject: [PATCH 7/8] Address Copilot feedack --- .../arrow/json/json_writer_internal_test.cc | 18 ++++++++---- cpp/src/arrow/util/simdjson_internal.h | 28 +++++++++++++------ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/cpp/src/arrow/json/json_writer_internal_test.cc b/cpp/src/arrow/json/json_writer_internal_test.cc index 329d0f0d1b09..7fc12e8adbe6 100644 --- a/cpp/src/arrow/json/json_writer_internal_test.cc +++ b/cpp/src/arrow/json/json_writer_internal_test.cc @@ -186,7 +186,8 @@ TEST(JsonWriter, WriteValueSimpleObject) { JsonWriter writer; ASSERT_OK(writer.WriteValue(value)); - EXPECT_EQ(writer.GetString(), R"({"a":42,"b":"hello"})"); + ASSERT_OK_AND_ASSIGN(std::string_view out, writer.GetString()); + EXPECT_EQ(out, R"({"a":42,"b":"hello"})"); } TEST(JsonWriter, WriteValueNestedObject) { @@ -203,7 +204,8 @@ TEST(JsonWriter, WriteValueNestedObject) { JsonWriter writer; ASSERT_OK(writer.WriteValue(value)); - EXPECT_EQ(writer.GetString(), R"({"child":{"x":true}})"); + ASSERT_OK_AND_ASSIGN(std::string_view out, writer.GetString()); + EXPECT_EQ(out, R"({"child":{"x":true}})"); } TEST(JsonWriter, WriteValueObjectWithArray) { @@ -220,7 +222,8 @@ TEST(JsonWriter, WriteValueObjectWithArray) { JsonWriter writer; ASSERT_OK(writer.WriteValue(value)); - EXPECT_EQ(writer.GetString(), R"({"values":[1,2,3]})"); + ASSERT_OK_AND_ASSIGN(std::string_view out, writer.GetString()); + EXPECT_EQ(out, R"({"values":[1,2,3]})"); } TEST(JsonWriter, WriteValueComplexObject) { @@ -238,8 +241,9 @@ TEST(JsonWriter, WriteValueComplexObject) { JsonWriter writer; ASSERT_OK(writer.WriteValue(value)); + ASSERT_OK_AND_ASSIGN(std::string_view out, writer.GetString()); EXPECT_EQ( - writer.GetString(), + out, R"({"name":"arrow","version":1,"enabled":true,"values":[1,2.5,null,{"nested":[false,{"x":10}]}]})"); } @@ -257,7 +261,8 @@ TEST(JsonWriter, WriteValueEmptyObject) { JsonWriter writer; ASSERT_OK(writer.WriteValue(value)); - EXPECT_EQ(writer.GetString(), "{}"); + ASSERT_OK_AND_ASSIGN(std::string_view out, writer.GetString()); + EXPECT_EQ(out, "{}"); } TEST(JsonWriter, WriteValueAllNumberTypes) { @@ -279,8 +284,9 @@ TEST(JsonWriter, WriteValueAllNumberTypes) { JsonWriter writer; ASSERT_OK(writer.WriteValue(value)); + ASSERT_OK_AND_ASSIGN(std::string_view out, writer.GetString()); EXPECT_EQ( - writer.GetString(), + out, R"({"signed":-42,"unsigned":18446744073709551615,"double":2.5,"big":184467440737095516161234567890})"); } diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index 8299eeed6b99..0f3eff6eaaa1 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -17,7 +17,10 @@ #pragma once +#include #include +#include +#include #include @@ -137,21 +140,21 @@ Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, case simdjson::ondemand::number_type::signed_integer: { ARROW_ASSIGN_OR_RAISE( auto number, - GetSimdjsonResult(value.get_int64(), "Failed to get signed integer")); + GetSimdjsonResult(value.get_int64(), "Failed to get signed integer: ")); return int64_fn(number); } case simdjson::ondemand::number_type::unsigned_integer: { ARROW_ASSIGN_OR_RAISE( auto number, - GetSimdjsonResult(value.get_uint64(), "Failed to get unsigned integer")); + GetSimdjsonResult(value.get_uint64(), "Failed to get unsigned integer: ")); return uint64_fn(number); } case simdjson::ondemand::number_type::floating_point_number: { - ARROW_ASSIGN_OR_RAISE(auto number, - GetSimdjsonResult(value.get_double(), - "Failed to get floating-point number")); + ARROW_ASSIGN_OR_RAISE( + auto number, GetSimdjsonResult(value.get_double(), + "Failed to get floating-point number: ")); return double_fn(number); } @@ -217,11 +220,20 @@ Result GetJsonAs(simdjson::ondemand::value& value) { if (error_code != simdjson::SUCCESS) { simdjson::ondemand::json_type json_type; if (value.type().get(json_type) != simdjson::SUCCESS) { + if constexpr (std::is_same_v) { + return Status::Invalid("Expected null, got malformed JSON value"); + } else { + return Status::Invalid("Expected ", JsonTypeName(), + ", got malformed JSON value"); + } + } + + if constexpr (std::is_same_v) { + return Status::Invalid("Expected null, got JSON type ", JsonTypeName(json_type)); + } else { return Status::Invalid("Expected ", JsonTypeName(), - " or null, got malformed JSON value"); + ", got JSON type ", JsonTypeName(json_type)); } - return Status::Invalid("Expected ", JsonTypeName(), - " or null, got JSON type ", JsonTypeName(json_type)); } return typed_value; From 92e1a8a19a814214a9db54063394f7463b2dfc0f Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Sat, 1 Aug 2026 20:17:37 +0530 Subject: [PATCH 8/8] Address Feedback --- cpp/src/arrow/util/simdjson_internal.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index 0f3eff6eaaa1..8ffb741da42e 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -17,9 +17,9 @@ #pragma once +#include #include #include -#include #include #include @@ -194,7 +194,8 @@ inline const char* JsonTypeName(simdjson::ondemand::json_type type) { // Result because peeking the nonRootScalar can fail (parsed lazily) inline Result IsJsonNull(simdjson::ondemand::value& value) { bool is_null; - if (auto error_code = value.is_null().get(is_null); error_code != simdjson::SUCCESS) { + auto error_code = value.is_null().get(is_null); + if (error_code != simdjson::SUCCESS) { return Status::Invalid("Error checking for JSON null: ", simdjson::error_message(error_code)); } @@ -206,7 +207,7 @@ Result GetJsonAs(simdjson::ondemand::value& value) { SimdjsonValueType typed_value{}; simdjson::error_code error_code; - if constexpr (std::is_same_v) { + if constexpr (std::same_as) { // simdjson has no get<>() for null; probe it explicitly bool is_null; error_code = value.is_null().get(is_null); @@ -220,7 +221,7 @@ Result GetJsonAs(simdjson::ondemand::value& value) { if (error_code != simdjson::SUCCESS) { simdjson::ondemand::json_type json_type; if (value.type().get(json_type) != simdjson::SUCCESS) { - if constexpr (std::is_same_v) { + if constexpr (std::same_as) { return Status::Invalid("Expected null, got malformed JSON value"); } else { return Status::Invalid("Expected ", JsonTypeName(), @@ -228,7 +229,7 @@ Result GetJsonAs(simdjson::ondemand::value& value) { } } - if constexpr (std::is_same_v) { + if constexpr (std::same_as) { return Status::Invalid("Expected null, got JSON type ", JsonTypeName(json_type)); } else { return Status::Invalid("Expected ", JsonTypeName(),