From 4338ca9ae8ae9ff92f37481a0bb794cdcd9a00bf Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 27 Aug 2026 03:14:17 +0530 Subject: [PATCH 01/10] Replace in gtest_util --- cpp/src/arrow/CMakeLists.txt | 4 +-- cpp/src/arrow/testing/gtest_util.cc | 42 +++++++++++------------- cpp/src/arrow/testing/gtest_util_test.cc | 17 ++++++++++ 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index d7086773a1fd..f3c5fce74a24 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -741,8 +741,8 @@ else() endif() set(ARROW_TESTING_SHARED_LINK_LIBS arrow_shared ${ARROW_GTEST_GTEST}) -set(ARROW_TESTING_SHARED_PRIVATE_LINK_LIBS arrow::flatbuffers RapidJSON) -set(ARROW_TESTING_STATIC_LINK_LIBS arrow::flatbuffers RapidJSON arrow_static +set(ARROW_TESTING_SHARED_PRIVATE_LINK_LIBS arrow::flatbuffers RapidJSON arrow::simdjson) +set(ARROW_TESTING_STATIC_LINK_LIBS arrow::flatbuffers RapidJSON arrow::simdjson arrow_static ${ARROW_GTEST_GTEST}) if(ARROW_ENABLE_THREADING) list(APPEND ARROW_TESTING_SHARED_PRIVATE_LINK_LIBS arrow::Boost::process) diff --git a/cpp/src/arrow/testing/gtest_util.cc b/cpp/src/arrow/testing/gtest_util.cc index b7d2a963d0de..bbbb1f880af4 100644 --- a/cpp/src/arrow/testing/gtest_util.cc +++ b/cpp/src/arrow/testing/gtest_util.cc @@ -53,7 +53,6 @@ #include "arrow/ipc/reader.h" #include "arrow/ipc/writer.h" #include "arrow/json/from_string.h" -#include "arrow/json/rapidjson_defs.h" // IWYU pragma: keep #include "arrow/pretty_print.h" #include "arrow/record_batch.h" #include "arrow/status.h" @@ -65,13 +64,10 @@ #include "arrow/util/future.h" #include "arrow/util/io_util.h" #include "arrow/util/logging_internal.h" +#include "arrow/util/simdjson_internal.h" #include "arrow/util/thread_pool.h" #include "arrow/util/windows_compatibility.h" -#include - -namespace rj = arrow::rapidjson; - namespace arrow { using internal::checked_cast; @@ -445,24 +441,24 @@ std::shared_ptr TensorFromJSON(const std::shared_ptr& type, std::string_view dim_names) { std::shared_ptr array = arrow::ArrayFromJSON(type, data); - rj::Document json_shape; - json_shape.Parse(shape.data(), shape.length()); - std::vector shape_vector; - for (auto& x : json_shape.GetArray()) { - shape_vector.emplace_back(x.GetInt64()); - } - rj::Document json_strides; - json_strides.Parse(strides.data(), strides.length()); - std::vector strides_vector; - for (auto& x : json_strides.GetArray()) { - strides_vector.emplace_back(x.GetInt64()); - } - rj::Document json_dim_names; - json_dim_names.Parse(dim_names.data(), dim_names.length()); - std::vector dim_names_vector; - for (auto& x : json_dim_names.GetArray()) { - dim_names_vector.emplace_back(x.GetString()); - } + simdjson::dom::parser parser; + + auto json_shape = + internal::ResolveSimdjsonResult(parser.parse(shape), "Failed to parse shape") + .ValueOrDie(); + auto shape_vector = internal::GetJsonIntArray(json_shape, "shape").ValueOrDie(); + + auto json_strides = + internal::ResolveSimdjsonResult(parser.parse(strides), "Failed to parse strides") + .ValueOrDie(); + auto strides_vector = internal::GetJsonIntArray(json_strides, "strides").ValueOrDie(); + + auto json_dim_names = internal::ResolveSimdjsonResult(parser.parse(dim_names), + "Failed to parse dimension names") + .ValueOrDie(); + auto dim_names_vector = + internal::GetJsonStringArray(json_dim_names, "dimension names").ValueOrDie(); + return *Tensor::Make(type, array->data()->buffers[1], shape_vector, strides_vector, dim_names_vector); } diff --git a/cpp/src/arrow/testing/gtest_util_test.cc b/cpp/src/arrow/testing/gtest_util_test.cc index 31b5b9e66285..f8bf694f5c36 100644 --- a/cpp/src/arrow/testing/gtest_util_test.cc +++ b/cpp/src/arrow/testing/gtest_util_test.cc @@ -179,6 +179,23 @@ TEST_F(TestTensorFromJSON, FromJSON) { EXPECT_TRUE(tensor_expected->Equals(*result)); } +TEST_F(TestTensorFromJSON, FromJSONWithStridesAndDimNames) { + std::vector shape = {2, 3}; + std::vector strides = {sizeof(int64_t) * 3, sizeof(int64_t)}; + std::vector dim_names = {"row", "column"}; + std::vector values = {1, 2, 3, 4, 5, 6}; + auto data = Buffer::Wrap(values); + + std::shared_ptr tensor_expected; + ASSERT_OK_AND_ASSIGN(tensor_expected, + Tensor::Make(int64(), data, shape, strides, dim_names)); + + std::shared_ptr result = TensorFromJSON(int64(), "[1, 2, 3, 4, 5, 6]", "[2, 3]", + "[24, 8]", R"(["row", "column"])"); + + EXPECT_TRUE(tensor_expected->Equals(*result)); +} + TEST(AssertTestWithinUlp, Basics) { AssertWithinUlp(123.4567, 123.45670000000015, 11); AssertWithinUlp(123.456f, 123.456085f, 11); From 15e5cb7cb4037cce7e7755c267fcc6a4195d48e4 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 27 Aug 2026 04:21:10 +0530 Subject: [PATCH 02/10] Replace in test_commons --- cpp/src/arrow/CMakeLists.txt | 4 +- cpp/src/arrow/json/parser_test.cc | 12 +++ cpp/src/arrow/json/reader_test.cc | 11 +- cpp/src/arrow/json/test_common.h | 75 ++++++++------ cpp/src/arrow/util/simdjson_internal.h | 134 +++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 35 deletions(-) diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index f3c5fce74a24..eead221dbdae 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -741,8 +741,8 @@ else() endif() set(ARROW_TESTING_SHARED_LINK_LIBS arrow_shared ${ARROW_GTEST_GTEST}) -set(ARROW_TESTING_SHARED_PRIVATE_LINK_LIBS arrow::flatbuffers RapidJSON arrow::simdjson) -set(ARROW_TESTING_STATIC_LINK_LIBS arrow::flatbuffers RapidJSON arrow::simdjson arrow_static +set(ARROW_TESTING_SHARED_PRIVATE_LINK_LIBS arrow::flatbuffers arrow::simdjson) +set(ARROW_TESTING_STATIC_LINK_LIBS arrow::flatbuffers arrow::simdjson arrow_static ${ARROW_GTEST_GTEST}) if(ARROW_ENABLE_THREADING) list(APPEND ARROW_TESTING_SHARED_PRIVATE_LINK_LIBS arrow::Boost::process) diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index 1b107aa020fd..ad7dd01d4ddd 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -321,5 +321,17 @@ TEST(BlockParser, AdHoc) { R"([{"c":true, "d": "1991-02-03"}, {"c":false, "d":"2019-04-01"}])"}); } +TEST(JsonTest, PrettyPrintEscapesObjectKeys) { + const std::string input = R"({"a\"b":1,"a\\b":2})"; + + const std::string expected = + "{\n" + " \"a\\\"b\": 1,\n" + " \"a\\\\b\": 2\n" + "}"; + + EXPECT_EQ(PrettyPrint(input), expected); +} + } // namespace json } // namespace arrow diff --git a/cpp/src/arrow/json/reader_test.cc b/cpp/src/arrow/json/reader_test.cc index 2aca602ae9ed..79140684f076 100644 --- a/cpp/src/arrow/json/reader_test.cc +++ b/cpp/src/arrow/json/reader_test.cc @@ -24,6 +24,7 @@ #include "arrow/io/interfaces.h" #include "arrow/io/slow.h" +#include "arrow/json/json_writer_internal.h" #include "arrow/json/options.h" #include "arrow/json/reader.h" #include "arrow/json/test_common.h" @@ -550,10 +551,14 @@ class StreamingReaderTestBase { auto options = GenerateOptions::Defaults(); options.null_probability = 0; for (int i = 0; i < num_rows; ++i) { - StringBuffer string_buffer; - Writer writer(string_buffer); + Writer writer; ABORT_NOT_OK(Generate(data_fields, engine, &writer, options)); - std::string json = string_buffer.GetString(); + + auto json_result = writer.GetString(); + ABORT_NOT_OK(json_result.status()); + auto json_view = std::move(json_result).ValueOrDie(); + std::string json(json_view); + rows[i] = Join({"{\"i\":", std::to_string(i), ",\"d\":", json, "}\n"}); max_row_size = std::max(max_row_size, rows[i].size()); } diff --git a/cpp/src/arrow/json/test_common.h b/cpp/src/arrow/json/test_common.h index ab2ce9cdc749..96103f580e8c 100644 --- a/cpp/src/arrow/json/test_common.h +++ b/cpp/src/arrow/json/test_common.h @@ -25,35 +25,31 @@ #include #include +#include + #include "arrow/array.h" #include "arrow/array/builder_binary.h" #include "arrow/io/memory.h" #include "arrow/json/converter.h" +#include "arrow/json/json_writer_internal.h" #include "arrow/json/options.h" #include "arrow/json/parser.h" -#include "arrow/json/rapidjson_defs.h" +#include "arrow/result.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/random.h" #include "arrow/type.h" #include "arrow/util/checked_cast.h" +#include "arrow/util/simdjson_internal.h" #include "arrow/visit_type_inline.h" -#include "rapidjson/document.h" -#include "rapidjson/prettywriter.h" -#include "rapidjson/reader.h" -#include "rapidjson/writer.h" - namespace arrow { using internal::checked_cast; namespace json { -namespace rj = arrow::rapidjson; - -using rj::StringBuffer; using std::string_view; -using Writer = rj::Writer; +using Writer = JsonWriter; struct GenerateOptions { // Probability of a field being written @@ -87,35 +83,43 @@ inline static Status Generate( template struct GenerateImpl { - Status Visit(const NullType&) { return OK(writer.Null()); } + Status Visit(const NullType&) { + writer.Null(); + return Status::OK(); + } Status Visit(const BooleanType&) { - return OK(writer.Bool(std::uniform_int_distribution{}(e) & 1)); + writer.Bool(std::uniform_int_distribution{}(e) & 1); + return Status::OK(); } template enable_if_physical_unsigned_integer Visit(const T&) { auto val = std::uniform_int_distribution<>{}(e); - return OK(writer.Uint64(static_cast(val))); + writer.Uint64(static_cast(val)); + return Status::OK(); } template enable_if_physical_signed_integer Visit(const T&) { auto val = std::uniform_int_distribution<>{}(e); - return OK(writer.Int64(static_cast(val))); + writer.Int64(static_cast(val)); + return Status::OK(); } template enable_if_physical_floating_point Visit(const T&) { auto val = std::normal_distribution{0, 1 << 10}(e); - return OK(writer.Double(val)); + writer.Double(val); + return Status::OK(); } Status GenerateUtf8(const DataType&) { auto num_codepoints = std::poisson_distribution<>{4}(e); auto seed = std::uniform_int_distribution{}(e); std::string s = RandomUtf8String(seed, num_codepoints); - return OK(writer.String(s)); + writer.String(s); + return Status::OK(); } template @@ -132,7 +136,8 @@ struct GenerateImpl { for (int i = 0; i < size; ++i) { RETURN_NOT_OK(Generate(t.value_type(), e, &writer, options)); } - return OK(writer.EndArray(size)); + writer.EndArray(); + return Status::OK(); } Status Visit(const ListViewType& t) { return NotImplemented(t); } @@ -162,7 +167,7 @@ struct GenerateImpl { } Engine& e; - rj::Writer& writer; + Writer& writer; const GenerateOptions& options; }; @@ -180,12 +185,9 @@ inline static Status Generate(const std::shared_ptr& type, Engine& e, template inline static Status Generate(const std::vector>& fields, Engine& e, Writer* writer, const GenerateOptions& options) { - RETURN_NOT_OK(OK(writer->StartObject())); - - int num_fields = 0; + writer->StartObject(); auto write_field = [&](const Field& f) { - ++num_fields; - writer->Key(f.name().c_str()); + writer->Key(f.name()); return Generate(f.type(), e, writer, options); }; @@ -210,7 +212,8 @@ inline static Status Generate(const std::vector>& fields, } } - return OK(writer->EndObject(num_fields)); + writer->EndObject(); + return Status::OK(); } inline static Status MakeStream(string_view src_str, @@ -258,14 +261,26 @@ inline static Status ParseFromString(ParseOptions options, string_view src_str, } static inline std::string PrettyPrint(string_view one_line) { - rj::Document document; + simdjson::ondemand::parser parser; // Must pass size to avoid ASAN issues. - document.Parse(one_line.data(), one_line.size()); - rj::StringBuffer sb; - rj::PrettyWriter writer(sb); - document.Accept(writer); - return sb.GetString(); + simdjson::padded_string json(one_line.data(), one_line.size()); + + auto document_result = + internal::ResolveSimdjsonResult(parser.iterate(json), "Failed to parse JSON"); + ABORT_NOT_OK(document_result.status()); + auto document = std::move(document_result).ValueOrDie(); + + auto value_result = + internal::ResolveSimdjsonResult(document.get_value(), "Failed to get JSON value"); + ABORT_NOT_OK(value_result.status()); + auto value = std::move(value_result).ValueOrDie(); + + std::string result; + result.reserve(one_line.size()); + + ABORT_NOT_OK(internal::PrettyPrintJsonValue(value, &result)); + return result; } template diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index 5c52f7648c6a..c708e1a59634 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -29,6 +29,7 @@ #include +#include "arrow/json/json_writer_internal.h" #include "arrow/result.h" #include "arrow/status.h" #include "arrow/util/visibility.h" @@ -272,7 +273,140 @@ Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, return Status::Invalid("Unreachable"); } +<<<<<<< HEAD ARROW_EXPORT const char* JsonTypeName(simdjson::ondemand::json_type type); +======= +inline Status PrettyPrintJsonValue(simdjson::ondemand::value value, std::string* out, + int indent = 0) { + constexpr int kIndentSize = 4; + + auto append_indent = [&](int level) { + out->append(static_cast(level * kIndentSize), ' '); + }; + + ARROW_ASSIGN_OR_RAISE( + auto type, ResolveSimdjsonResult(value.type(), "Failed to determine JSON type")); + + switch (type) { + case simdjson::ondemand::json_type::object: { + ARROW_ASSIGN_OR_RAISE( + auto object, + ResolveSimdjsonResult(value.get_object(), "Failed to get JSON object")); + + out->append("{"); + + bool first = true; + for (auto field_result : object) { + ARROW_ASSIGN_OR_RAISE( + auto field, + ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); + + ARROW_ASSIGN_OR_RAISE(auto key, + ResolveSimdjsonResult(field.unescaped_key(), + "Failed to get JSON object key")); + + auto field_value = field.value(); + + if (first) { + out->append("\n"); + first = false; + } else { + out->append(",\n"); + } + + append_indent(indent + 1); + + json::JsonWriter writer; + writer.String(key); + + ARROW_ASSIGN_OR_RAISE(auto escaped_key, writer.GetString()); + out->append(escaped_key); + out->append(": "); + + RETURN_NOT_OK(PrettyPrintJsonValue(field_value, out, indent + 1)); + } + + if (!first) { + out->append("\n"); + append_indent(indent); + } + + out->append("}"); + return Status::OK(); + } + + case simdjson::ondemand::json_type::array: { + ARROW_ASSIGN_OR_RAISE( + auto array, + ResolveSimdjsonResult(value.get_array(), "Failed to get JSON array")); + + out->append("["); + + bool first = true; + for (auto element_result : array) { + ARROW_ASSIGN_OR_RAISE( + auto element, + ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); + + if (first) { + out->append("\n"); + first = false; + } else { + out->append(",\n"); + } + + append_indent(indent + 1); + + RETURN_NOT_OK(PrettyPrintJsonValue(element, out, indent + 1)); + } + + if (!first) { + out->append("\n"); + append_indent(indent); + } + + out->append("]"); + return Status::OK(); + } + + case simdjson::ondemand::json_type::string: + case simdjson::ondemand::json_type::boolean: + case simdjson::ondemand::json_type::null: + case simdjson::ondemand::json_type::number: { + ARROW_ASSIGN_OR_RAISE(auto serialized, + ResolveSimdjsonResult(simdjson::to_json_string(value), + "Failed to serialize JSON value")); + + out->append(serialized); + return Status::OK(); + } + + case simdjson::ondemand::json_type::unknown: + return Status::Invalid("Unknown JSON type"); + } + + 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"; + } +} +>>>>>>> 8ed25131dc (Replace in test_commons) // Result because peeking the nonRootScalar can fail (parsed lazily) ARROW_EXPORT Result IsJsonNull(simdjson::ondemand::value& value); From f015bc7e627a634323b692878d255b399b1864d2 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 27 Aug 2026 14:06:44 +0530 Subject: [PATCH 03/10] Add meson dep --- cpp/src/arrow/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/src/arrow/meson.build b/cpp/src/arrow/meson.build index f181321aaab7..fea26ef4e452 100644 --- a/cpp/src/arrow/meson.build +++ b/cpp/src/arrow/meson.build @@ -758,6 +758,7 @@ if needs_testing filesystem_dep, gmock_dep, gtest_dep, + simdjson_dep, ], ) From a300b1bf4c9302b6fabf34159d446aa5b3f86e6d Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 27 Aug 2026 15:06:21 +0530 Subject: [PATCH 04/10] Replace with JsonWriter --- cpp/src/arrow/json/parser_benchmark.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/json/parser_benchmark.cc b/cpp/src/arrow/json/parser_benchmark.cc index a5a6eb68e67a..f26d6a144316 100644 --- a/cpp/src/arrow/json/parser_benchmark.cc +++ b/cpp/src/arrow/json/parser_benchmark.cc @@ -38,10 +38,14 @@ std::string GenerateTestData(const Input& input, int num_rows, std::default_random_engine engine(kSeed); std::string json; for (int i = 0; i < num_rows; ++i) { - StringBuffer sb; - Writer writer(sb); + Writer writer; ABORT_NOT_OK(Generate(input, engine, &writer, options)); - json += pretty ? PrettyPrint(sb.GetString()) : sb.GetString(); + + auto json_result = writer.GetString(); + ABORT_NOT_OK(json_result.status()); + auto json_view = std::move(json_result).ValueOrDie(); + + json += pretty ? PrettyPrint(json_view) : json_view; json += "\n"; } return json; From 7d391d2c3a57ba6bb045e0b61071638bbb17f304 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Thu, 27 Aug 2026 17:12:12 +0530 Subject: [PATCH 05/10] add cmake dep --- cpp/src/arrow/json/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/json/CMakeLists.txt b/cpp/src/arrow/json/CMakeLists.txt index b930034537d0..de9115497e2b 100644 --- a/cpp/src/arrow/json/CMakeLists.txt +++ b/cpp/src/arrow/json/CMakeLists.txt @@ -33,7 +33,9 @@ add_arrow_benchmark(parser_benchmark PREFIX "arrow-json" EXTRA_LINK_LIBS - RapidJSON) + RapidJSON + simdjson::simdjson) + arrow_install_all_headers("arrow/json") # pkg-config support From 6e34b702a55b1acec757e8cea7b29e130f61f822 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Mon, 7 Sep 2026 10:03:39 +0530 Subject: [PATCH 06/10] restore simdjson_internal --- cpp/src/arrow/util/simdjson_internal.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index c708e1a59634..edbd0eb0f545 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -273,9 +273,6 @@ Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, return Status::Invalid("Unreachable"); } -<<<<<<< HEAD -ARROW_EXPORT const char* JsonTypeName(simdjson::ondemand::json_type type); -======= inline Status PrettyPrintJsonValue(simdjson::ondemand::value value, std::string* out, int indent = 0) { constexpr int kIndentSize = 4; @@ -406,7 +403,6 @@ inline const char* JsonTypeName(simdjson::ondemand::json_type type) { return "unknown"; } } ->>>>>>> 8ed25131dc (Replace in test_commons) // Result because peeking the nonRootScalar can fail (parsed lazily) ARROW_EXPORT Result IsJsonNull(simdjson::ondemand::value& value); From 4155b6c6ad68f987201ddb5feca7f0f3e550f2dc Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Mon, 7 Sep 2026 10:40:02 +0530 Subject: [PATCH 07/10] WIP: Move PrettyPrintJsonValue to simdjson_internal.cc --- cpp/src/arrow/json/reader_test.cc | 1 - cpp/src/arrow/json/test_common.h | 14 +-- cpp/src/arrow/util/simdjson_internal.cc | 122 +++++++++++++++++++++ cpp/src/arrow/util/simdjson_internal.h | 135 +----------------------- 4 files changed, 131 insertions(+), 141 deletions(-) diff --git a/cpp/src/arrow/json/reader_test.cc b/cpp/src/arrow/json/reader_test.cc index 79140684f076..f9b376224928 100644 --- a/cpp/src/arrow/json/reader_test.cc +++ b/cpp/src/arrow/json/reader_test.cc @@ -24,7 +24,6 @@ #include "arrow/io/interfaces.h" #include "arrow/io/slow.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/json/options.h" #include "arrow/json/reader.h" #include "arrow/json/test_common.h" diff --git a/cpp/src/arrow/json/test_common.h b/cpp/src/arrow/json/test_common.h index 96103f580e8c..8c6f7c52e089 100644 --- a/cpp/src/arrow/json/test_common.h +++ b/cpp/src/arrow/json/test_common.h @@ -31,7 +31,6 @@ #include "arrow/array/builder_binary.h" #include "arrow/io/memory.h" #include "arrow/json/converter.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/json/options.h" #include "arrow/json/parser.h" #include "arrow/result.h" @@ -49,7 +48,7 @@ using internal::checked_cast; namespace json { using std::string_view; -using Writer = JsonWriter; +using Writer = internal::JsonWriter; struct GenerateOptions { // Probability of a field being written @@ -260,9 +259,8 @@ inline static Status ParseFromString(ParseOptions options, string_view src_str, return Status::OK(); } -static inline std::string PrettyPrint(string_view one_line) { +static inline std::string PrettyPrint(std::string_view one_line) { simdjson::ondemand::parser parser; - // Must pass size to avoid ASAN issues. simdjson::padded_string json(one_line.data(), one_line.size()); @@ -276,11 +274,9 @@ static inline std::string PrettyPrint(string_view one_line) { ABORT_NOT_OK(value_result.status()); auto value = std::move(value_result).ValueOrDie(); - std::string result; - result.reserve(one_line.size()); - - ABORT_NOT_OK(internal::PrettyPrintJsonValue(value, &result)); - return result; + auto result = internal::PrettyPrintJsonValue(value); + ABORT_NOT_OK(result.status()); + return std::move(result).ValueOrDie(); } template diff --git a/cpp/src/arrow/util/simdjson_internal.cc b/cpp/src/arrow/util/simdjson_internal.cc index 146b48d9eb7f..b7366765958d 100644 --- a/cpp/src/arrow/util/simdjson_internal.cc +++ b/cpp/src/arrow/util/simdjson_internal.cc @@ -581,4 +581,126 @@ Status ValidateJsonDocument(simdjson::ondemand::parser& parser, return ConsumeJsonValue(value); } +namespace { + +Status PrettyPrintJsonValueImpl(simdjson::ondemand::value value, std::string* out, + int indent) { + constexpr int kIndentSize = 4; + + auto append_indent = [&](int level) { + out->append(static_cast(level * kIndentSize), ' '); + }; + + ARROW_ASSIGN_OR_RAISE( + auto type, ResolveSimdjsonResult(value.type(), "Failed to determine JSON type")); + + switch (type) { + case simdjson::ondemand::json_type::object: { + ARROW_ASSIGN_OR_RAISE( + auto object, + ResolveSimdjsonResult(value.get_object(), "Failed to get JSON object")); + + out->append("{"); + + bool first = true; + for (auto field_result : object) { + ARROW_ASSIGN_OR_RAISE( + auto field, + ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); + + ARROW_ASSIGN_OR_RAISE(auto key, + ResolveSimdjsonResult(field.unescaped_key(), + "Failed to get JSON object key")); + + auto field_value = field.value(); + + if (first) { + out->append("\n"); + first = false; + } else { + out->append(",\n"); + } + + append_indent(indent + 1); + + JsonWriter writer; + writer.String(key); + + ARROW_ASSIGN_OR_RAISE(auto escaped_key, writer.GetString()); + out->append(escaped_key); + out->append(": "); + + RETURN_NOT_OK(PrettyPrintJsonValueImpl(field_value, out, indent + 1)); + } + + if (!first) { + out->append("\n"); + append_indent(indent); + } + + out->append("}"); + return Status::OK(); + } + + case simdjson::ondemand::json_type::array: { + ARROW_ASSIGN_OR_RAISE( + auto array, + ResolveSimdjsonResult(value.get_array(), "Failed to get JSON array")); + + out->append("["); + + bool first = true; + for (auto element_result : array) { + ARROW_ASSIGN_OR_RAISE( + auto element, + ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); + + if (first) { + out->append("\n"); + first = false; + } else { + out->append(",\n"); + } + + append_indent(indent + 1); + + RETURN_NOT_OK(PrettyPrintJsonValueImpl(element, out, indent + 1)); + } + + if (!first) { + out->append("\n"); + append_indent(indent); + } + + out->append("]"); + return Status::OK(); + } + + case simdjson::ondemand::json_type::string: + case simdjson::ondemand::json_type::boolean: + case simdjson::ondemand::json_type::null: + case simdjson::ondemand::json_type::number: { + ARROW_ASSIGN_OR_RAISE(auto serialized, + ResolveSimdjsonResult(simdjson::to_json_string(value), + "Failed to serialize JSON value")); + + out->append(serialized); + return Status::OK(); + } + + case simdjson::ondemand::json_type::unknown: + return Status::Invalid("Unknown JSON type"); + } + + return Status::Invalid("Unreachable"); +} + +} // namespace + +Result PrettyPrintJsonValue(simdjson::ondemand::value value, int indent) { + std::string out; + RETURN_NOT_OK(PrettyPrintJsonValueImpl(value, &out, indent)); + return out; +} + } // namespace arrow::internal diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index edbd0eb0f545..145257ea2827 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -29,7 +29,6 @@ #include -#include "arrow/json/json_writer_internal.h" #include "arrow/result.h" #include "arrow/status.h" #include "arrow/util/visibility.h" @@ -273,136 +272,7 @@ Status VisitJsonValue(simdjson::ondemand::value value, ObjectFn&& object_fn, return Status::Invalid("Unreachable"); } -inline Status PrettyPrintJsonValue(simdjson::ondemand::value value, std::string* out, - int indent = 0) { - constexpr int kIndentSize = 4; - - auto append_indent = [&](int level) { - out->append(static_cast(level * kIndentSize), ' '); - }; - - ARROW_ASSIGN_OR_RAISE( - auto type, ResolveSimdjsonResult(value.type(), "Failed to determine JSON type")); - - switch (type) { - case simdjson::ondemand::json_type::object: { - ARROW_ASSIGN_OR_RAISE( - auto object, - ResolveSimdjsonResult(value.get_object(), "Failed to get JSON object")); - - out->append("{"); - - bool first = true; - for (auto field_result : object) { - ARROW_ASSIGN_OR_RAISE( - auto field, - ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); - - ARROW_ASSIGN_OR_RAISE(auto key, - ResolveSimdjsonResult(field.unescaped_key(), - "Failed to get JSON object key")); - - auto field_value = field.value(); - - if (first) { - out->append("\n"); - first = false; - } else { - out->append(",\n"); - } - - append_indent(indent + 1); - - json::JsonWriter writer; - writer.String(key); - - ARROW_ASSIGN_OR_RAISE(auto escaped_key, writer.GetString()); - out->append(escaped_key); - out->append(": "); - - RETURN_NOT_OK(PrettyPrintJsonValue(field_value, out, indent + 1)); - } - - if (!first) { - out->append("\n"); - append_indent(indent); - } - - out->append("}"); - return Status::OK(); - } - - case simdjson::ondemand::json_type::array: { - ARROW_ASSIGN_OR_RAISE( - auto array, - ResolveSimdjsonResult(value.get_array(), "Failed to get JSON array")); - - out->append("["); - - bool first = true; - for (auto element_result : array) { - ARROW_ASSIGN_OR_RAISE( - auto element, - ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); - - if (first) { - out->append("\n"); - first = false; - } else { - out->append(",\n"); - } - - append_indent(indent + 1); - - RETURN_NOT_OK(PrettyPrintJsonValue(element, out, indent + 1)); - } - - if (!first) { - out->append("\n"); - append_indent(indent); - } - - out->append("]"); - return Status::OK(); - } - - case simdjson::ondemand::json_type::string: - case simdjson::ondemand::json_type::boolean: - case simdjson::ondemand::json_type::null: - case simdjson::ondemand::json_type::number: { - ARROW_ASSIGN_OR_RAISE(auto serialized, - ResolveSimdjsonResult(simdjson::to_json_string(value), - "Failed to serialize JSON value")); - - out->append(serialized); - return Status::OK(); - } - - case simdjson::ondemand::json_type::unknown: - return Status::Invalid("Unknown JSON type"); - } - - 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"; - } -} +ARROW_EXPORT const char* JsonTypeName(simdjson::ondemand::json_type type); // Result because peeking the nonRootScalar can fail (parsed lazily) ARROW_EXPORT Result IsJsonNull(simdjson::ondemand::value& value); @@ -480,4 +350,7 @@ ARROW_EXPORT Status ConsumeJsonValue(simdjson::ondemand::value value); ARROW_EXPORT Status ValidateJsonDocument(simdjson::ondemand::parser& parser, simdjson::padded_string& json); +ARROW_EXPORT Result PrettyPrintJsonValue(simdjson::ondemand::value value, + int indent = 0); + } // namespace arrow::internal From 1c9d6754404a52bb0dda875a8e944efaef9d7794 Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Mon, 7 Sep 2026 19:19:36 +0530 Subject: [PATCH 08/10] Address Feedback --- cpp/src/arrow/json/reader_test.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cpp/src/arrow/json/reader_test.cc b/cpp/src/arrow/json/reader_test.cc index f9b376224928..ac5bafb5293e 100644 --- a/cpp/src/arrow/json/reader_test.cc +++ b/cpp/src/arrow/json/reader_test.cc @@ -553,10 +553,7 @@ class StreamingReaderTestBase { Writer writer; ABORT_NOT_OK(Generate(data_fields, engine, &writer, options)); - auto json_result = writer.GetString(); - ABORT_NOT_OK(json_result.status()); - auto json_view = std::move(json_result).ValueOrDie(); - std::string json(json_view); + std::string json(writer.GetString().ValueOrDie()); rows[i] = Join({"{\"i\":", std::to_string(i), ",\"d\":", json, "}\n"}); max_row_size = std::max(max_row_size, rows[i].size()); From 2cbac54f9a8f128172c72f836e9bdb0da142636d Mon Sep 17 00:00:00 2001 From: Aaditya Srinivasan Date: Tue, 8 Sep 2026 09:17:32 +0530 Subject: [PATCH 09/10] Use fractured_json_string and remove PrettyPrintJsonValue --- cpp/src/arrow/json/parser_test.cc | 7 +- cpp/src/arrow/json/test_common.h | 18 +--- cpp/src/arrow/util/simdjson_internal.cc | 122 ------------------------ cpp/src/arrow/util/simdjson_internal.h | 3 - 4 files changed, 2 insertions(+), 148 deletions(-) diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index ad7dd01d4ddd..d052f6bf56a9 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -323,12 +323,7 @@ TEST(BlockParser, AdHoc) { TEST(JsonTest, PrettyPrintEscapesObjectKeys) { const std::string input = R"({"a\"b":1,"a\\b":2})"; - - const std::string expected = - "{\n" - " \"a\\\"b\": 1,\n" - " \"a\\\\b\": 2\n" - "}"; + const std::string expected = R"({ "a\"b": 1, "a\\b": 2 })"; EXPECT_EQ(PrettyPrint(input), expected); } diff --git a/cpp/src/arrow/json/test_common.h b/cpp/src/arrow/json/test_common.h index 8c6f7c52e089..241584959e74 100644 --- a/cpp/src/arrow/json/test_common.h +++ b/cpp/src/arrow/json/test_common.h @@ -260,23 +260,7 @@ inline static Status ParseFromString(ParseOptions options, string_view src_str, } static inline std::string PrettyPrint(std::string_view one_line) { - simdjson::ondemand::parser parser; - // Must pass size to avoid ASAN issues. - simdjson::padded_string json(one_line.data(), one_line.size()); - - auto document_result = - internal::ResolveSimdjsonResult(parser.iterate(json), "Failed to parse JSON"); - ABORT_NOT_OK(document_result.status()); - auto document = std::move(document_result).ValueOrDie(); - - auto value_result = - internal::ResolveSimdjsonResult(document.get_value(), "Failed to get JSON value"); - ABORT_NOT_OK(value_result.status()); - auto value = std::move(value_result).ValueOrDie(); - - auto result = internal::PrettyPrintJsonValue(value); - ABORT_NOT_OK(result.status()); - return std::move(result).ValueOrDie(); + return simdjson::fractured_json_string(one_line); } template diff --git a/cpp/src/arrow/util/simdjson_internal.cc b/cpp/src/arrow/util/simdjson_internal.cc index b7366765958d..146b48d9eb7f 100644 --- a/cpp/src/arrow/util/simdjson_internal.cc +++ b/cpp/src/arrow/util/simdjson_internal.cc @@ -581,126 +581,4 @@ Status ValidateJsonDocument(simdjson::ondemand::parser& parser, return ConsumeJsonValue(value); } -namespace { - -Status PrettyPrintJsonValueImpl(simdjson::ondemand::value value, std::string* out, - int indent) { - constexpr int kIndentSize = 4; - - auto append_indent = [&](int level) { - out->append(static_cast(level * kIndentSize), ' '); - }; - - ARROW_ASSIGN_OR_RAISE( - auto type, ResolveSimdjsonResult(value.type(), "Failed to determine JSON type")); - - switch (type) { - case simdjson::ondemand::json_type::object: { - ARROW_ASSIGN_OR_RAISE( - auto object, - ResolveSimdjsonResult(value.get_object(), "Failed to get JSON object")); - - out->append("{"); - - bool first = true; - for (auto field_result : object) { - ARROW_ASSIGN_OR_RAISE( - auto field, - ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); - - ARROW_ASSIGN_OR_RAISE(auto key, - ResolveSimdjsonResult(field.unescaped_key(), - "Failed to get JSON object key")); - - auto field_value = field.value(); - - if (first) { - out->append("\n"); - first = false; - } else { - out->append(",\n"); - } - - append_indent(indent + 1); - - JsonWriter writer; - writer.String(key); - - ARROW_ASSIGN_OR_RAISE(auto escaped_key, writer.GetString()); - out->append(escaped_key); - out->append(": "); - - RETURN_NOT_OK(PrettyPrintJsonValueImpl(field_value, out, indent + 1)); - } - - if (!first) { - out->append("\n"); - append_indent(indent); - } - - out->append("}"); - return Status::OK(); - } - - case simdjson::ondemand::json_type::array: { - ARROW_ASSIGN_OR_RAISE( - auto array, - ResolveSimdjsonResult(value.get_array(), "Failed to get JSON array")); - - out->append("["); - - bool first = true; - for (auto element_result : array) { - ARROW_ASSIGN_OR_RAISE( - auto element, - ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); - - if (first) { - out->append("\n"); - first = false; - } else { - out->append(",\n"); - } - - append_indent(indent + 1); - - RETURN_NOT_OK(PrettyPrintJsonValueImpl(element, out, indent + 1)); - } - - if (!first) { - out->append("\n"); - append_indent(indent); - } - - out->append("]"); - return Status::OK(); - } - - case simdjson::ondemand::json_type::string: - case simdjson::ondemand::json_type::boolean: - case simdjson::ondemand::json_type::null: - case simdjson::ondemand::json_type::number: { - ARROW_ASSIGN_OR_RAISE(auto serialized, - ResolveSimdjsonResult(simdjson::to_json_string(value), - "Failed to serialize JSON value")); - - out->append(serialized); - return Status::OK(); - } - - case simdjson::ondemand::json_type::unknown: - return Status::Invalid("Unknown JSON type"); - } - - return Status::Invalid("Unreachable"); -} - -} // namespace - -Result PrettyPrintJsonValue(simdjson::ondemand::value value, int indent) { - std::string out; - RETURN_NOT_OK(PrettyPrintJsonValueImpl(value, &out, indent)); - return out; -} - } // namespace arrow::internal diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index 145257ea2827..5c52f7648c6a 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -350,7 +350,4 @@ ARROW_EXPORT Status ConsumeJsonValue(simdjson::ondemand::value value); ARROW_EXPORT Status ValidateJsonDocument(simdjson::ondemand::parser& parser, simdjson::padded_string& json); -ARROW_EXPORT Result PrettyPrintJsonValue(simdjson::ondemand::value value, - int indent = 0); - } // namespace arrow::internal From 259feafa9bdc843d1c25040e2521486669b06694 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 8 Sep 2026 10:41:54 +0200 Subject: [PATCH 10/10] Remove test that's not really useful --- cpp/src/arrow/json/parser_test.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/cpp/src/arrow/json/parser_test.cc b/cpp/src/arrow/json/parser_test.cc index d052f6bf56a9..1b107aa020fd 100644 --- a/cpp/src/arrow/json/parser_test.cc +++ b/cpp/src/arrow/json/parser_test.cc @@ -321,12 +321,5 @@ TEST(BlockParser, AdHoc) { R"([{"c":true, "d": "1991-02-03"}, {"c":false, "d":"2019-04-01"}])"}); } -TEST(JsonTest, PrettyPrintEscapesObjectKeys) { - const std::string input = R"({"a\"b":1,"a\\b":2})"; - const std::string expected = R"({ "a\"b": 1, "a\\b": 2 })"; - - EXPECT_EQ(PrettyPrint(input), expected); -} - } // namespace json } // namespace arrow