From 5a1f424fd724e277255168e76fcc2d0622c4055a Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 27 Aug 2026 16:47:26 +0200 Subject: [PATCH 1/7] GH-50859: [C++][Parquet] Move JsonWriter to simdjson utilities Parquet uses JsonWriter independently of the Arrow JSON module. Move the writer into the simdjson utilities so it is available whenever simdjson is enabled, including ARROW_JSON=OFF builds. Preserve the writer files as renames and update CMake, Meson, callers, and tests. --- cpp/src/arrow/CMakeLists.txt | 10 +- cpp/src/arrow/dataset/file_json_test.cc | 2 +- cpp/src/arrow/extension/fixed_shape_tensor.cc | 2 +- cpp/src/arrow/extension/opaque.cc | 2 +- .../arrow/extension/variable_shape_tensor.cc | 2 +- .../sql/odbc/odbc_impl/json_converter.cc | 2 +- cpp/src/arrow/integration/json_integration.cc | 2 +- .../integration/json_integration_test.cc | 2 +- cpp/src/arrow/integration/json_internal.cc | 2 +- cpp/src/arrow/json/CMakeLists.txt | 1 - cpp/src/arrow/json/meson.build | 1 - cpp/src/arrow/meson.build | 41 ++-- cpp/src/arrow/util/CMakeLists.txt | 8 + .../{json => util}/json_writer_internal.cc | 2 +- .../{json => util}/json_writer_internal.h | 0 .../json_writer_internal_test.cc | 2 +- cpp/src/arrow/util/meson.build | 8 +- cpp/src/arrow/util/simdjson_internal.cc | 222 ++++++++++++++++++ cpp/src/arrow/util/simdjson_internal.h | 219 ++--------------- .../file_system_key_material_store.cc | 2 +- cpp/src/parquet/encryption/key_material.cc | 2 +- cpp/src/parquet/encryption/key_metadata.cc | 2 +- .../encryption/local_wrap_kms_client.cc | 2 +- .../parquet/geospatial/util_json_internal.cc | 2 +- cpp/src/parquet/printer.cc | 2 +- cpp/src/parquet/types.cc | 2 +- 26 files changed, 313 insertions(+), 231 deletions(-) rename cpp/src/arrow/{json => util}/json_writer_internal.cc (99%) rename cpp/src/arrow/{json => util}/json_writer_internal.h (100%) rename cpp/src/arrow/{json => util}/json_writer_internal_test.cc (99%) create mode 100644 cpp/src/arrow/util/simdjson_internal.cc diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index b9ff2ffa2c2d..ed8174f7176a 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -644,6 +644,9 @@ endif() if(ARROW_WITH_OPENTELEMETRY) list(APPEND ARROW_UTIL_SRCS util/tracing_internal.cc) endif() +if(ARROW_WITH_SIMDJSON) + list(APPEND ARROW_UTIL_SRCS util/json_writer_internal.cc util/simdjson_internal.cc) +endif() if(ARROW_WITH_SNAPPY) list(APPEND ARROW_UTIL_SRCS util/compression_snappy.cc) endif() @@ -664,6 +667,12 @@ foreach(ARROW_UTIL_TARGET ${ARROW_UTIL_TARGETS}) target_link_libraries(${ARROW_UTIL_TARGET} PRIVATE ${ARROW_XSIMD}) endforeach() +if(ARROW_WITH_SIMDJSON) + foreach(ARROW_UTIL_TARGET ${ARROW_UTIL_TARGETS}) + target_link_libraries(${ARROW_UTIL_TARGET} PRIVATE arrow::simdjson) + endforeach() +endif() + if(ARROW_USE_BOOST) foreach(ARROW_UTIL_TARGET ${ARROW_UTIL_TARGETS}) target_link_libraries(${ARROW_UTIL_TARGET} PRIVATE Boost::headers) @@ -1088,7 +1097,6 @@ if(ARROW_JSON) json/chunker.cc json/converter.cc json/from_string.cc - json/json_writer_internal.cc json/object_parser.cc json/parser.cc json/reader.cc) diff --git a/cpp/src/arrow/dataset/file_json_test.cc b/cpp/src/arrow/dataset/file_json_test.cc index b96d4ffed99c..e3d0c15776ca 100644 --- a/cpp/src/arrow/dataset/file_json_test.cc +++ b/cpp/src/arrow/dataset/file_json_test.cc @@ -20,10 +20,10 @@ #include "arrow/dataset/plan.h" #include "arrow/dataset/test_util_internal.h" #include "arrow/filesystem/mockfs.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/json/parser.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/util.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" namespace arrow { diff --git a/cpp/src/arrow/extension/fixed_shape_tensor.cc b/cpp/src/arrow/extension/fixed_shape_tensor.cc index 6a86d6a7a66f..48799899c7e2 100644 --- a/cpp/src/arrow/extension/fixed_shape_tensor.cc +++ b/cpp/src/arrow/extension/fixed_shape_tensor.cc @@ -27,8 +27,8 @@ #include "arrow/array/array_nested.h" #include "arrow/array/array_primitive.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/tensor.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/print_internal.h" #include "arrow/util/simdjson_internal.h" diff --git a/cpp/src/arrow/extension/opaque.cc b/cpp/src/arrow/extension/opaque.cc index c6068babdb00..6e5a07405f5a 100644 --- a/cpp/src/arrow/extension/opaque.cc +++ b/cpp/src/arrow/extension/opaque.cc @@ -19,7 +19,7 @@ #include -#include "arrow/json/json_writer_internal.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/simdjson_internal.h" diff --git a/cpp/src/arrow/extension/variable_shape_tensor.cc b/cpp/src/arrow/extension/variable_shape_tensor.cc index 784cd334b1d6..0ecd9f18c80b 100644 --- a/cpp/src/arrow/extension/variable_shape_tensor.cc +++ b/cpp/src/arrow/extension/variable_shape_tensor.cc @@ -23,9 +23,9 @@ #include "arrow/extension/variable_shape_tensor.h" #include "arrow/array/array_primitive.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/scalar.h" #include "arrow/tensor.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/print_internal.h" #include "arrow/util/simdjson_internal.h" diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc index 64e91ecd9bff..5ced882545db 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc @@ -20,8 +20,8 @@ #include #include "arrow/builder.h" #include "arrow/flight/sql/odbc/odbc_impl/util.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/scalar.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/visitor.h" using boost::beast::detail::base64::encode; diff --git a/cpp/src/arrow/integration/json_integration.cc b/cpp/src/arrow/integration/json_integration.cc index f978e1da5418..5a1def6d0b40 100644 --- a/cpp/src/arrow/integration/json_integration.cc +++ b/cpp/src/arrow/integration/json_integration.cc @@ -29,11 +29,11 @@ #include "arrow/integration/json_internal.h" #include "arrow/io/file.h" #include "arrow/ipc/dictionary.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/record_batch.h" #include "arrow/result.h" #include "arrow/status.h" #include "arrow/type.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/simdjson_internal.h" diff --git a/cpp/src/arrow/integration/json_integration_test.cc b/cpp/src/arrow/integration/json_integration_test.cc index 700551c23b79..6687f26224c6 100644 --- a/cpp/src/arrow/integration/json_integration_test.cc +++ b/cpp/src/arrow/integration/json_integration_test.cc @@ -38,7 +38,6 @@ #include "arrow/ipc/reader.h" #include "arrow/ipc/test_common.h" #include "arrow/ipc/writer.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/pretty_print.h" #include "arrow/status.h" #include "arrow/testing/builder.h" @@ -49,6 +48,7 @@ #include "arrow/type.h" #include "arrow/type_fwd.h" #include "arrow/util/io_util.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/simdjson_internal.h" DEFINE_string(arrow, "", "Arrow file name"); diff --git a/cpp/src/arrow/integration/json_internal.cc b/cpp/src/arrow/integration/json_internal.cc index abf48b9df767..4ed852b121bf 100644 --- a/cpp/src/arrow/integration/json_internal.cc +++ b/cpp/src/arrow/integration/json_internal.cc @@ -36,7 +36,6 @@ #include "arrow/array/builder_time.h" #include "arrow/extension_type.h" #include "arrow/ipc/dictionary.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/record_batch.h" #include "arrow/result.h" #include "arrow/scalar.h" @@ -48,6 +47,7 @@ #include "arrow/util/checked_cast.h" #include "arrow/util/decimal.h" #include "arrow/util/formatting.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/key_value_metadata.h" #include "arrow/util/logging_internal.h" #include "arrow/util/range.h" diff --git a/cpp/src/arrow/json/CMakeLists.txt b/cpp/src/arrow/json/CMakeLists.txt index e7719c771c3d..cad6f0fbdf3f 100644 --- a/cpp/src/arrow/json/CMakeLists.txt +++ b/cpp/src/arrow/json/CMakeLists.txt @@ -21,7 +21,6 @@ add_arrow_test(test chunker_test.cc converter_test.cc from_string_test.cc - json_writer_internal_test.cc object_parser_test.cc parser_test.cc reader_test.cc diff --git a/cpp/src/arrow/json/meson.build b/cpp/src/arrow/json/meson.build index ee2a26b2ccde..8c5813feec41 100644 --- a/cpp/src/arrow/json/meson.build +++ b/cpp/src/arrow/json/meson.build @@ -22,7 +22,6 @@ exc = executable( 'chunker_test.cc', 'converter_test.cc', 'from_string_test.cc', - 'json_writer_internal_test.cc', 'parser_test.cc', 'reader_test.cc', ], diff --git a/cpp/src/arrow/meson.build b/cpp/src/arrow/meson.build index b5443c28cf84..298c7a5e02c1 100644 --- a/cpp/src/arrow/meson.build +++ b/cpp/src/arrow/meson.build @@ -18,6 +18,22 @@ dl_dep = dependency('dl') threads_dep = dependency('threads') +if needs_json or needs_integration or needs_parquet + simdjson_dep = dependency('simdjson', allow_fallback: false, required: false) + + if not simdjson_dep.found() + cmake = import('cmake') + simdjson_opts = cmake.subproject_options() + + simdjson_opts.add_cmake_defines({'SIMDJSON_EXCEPTIONS': 'OFF'}) + + simdjson_proj = cmake.subproject('simdjson', options: simdjson_opts) + simdjson_dep = simdjson_proj.dependency('simdjson') + endif +else + simdjson_dep = disabler() +endif + arrow_components = { 'arrow_array': { 'sources': [ @@ -224,6 +240,14 @@ arrow_util_srcs = [ arrow_util_deps = [threads_dep] +if needs_json or needs_integration or needs_parquet + arrow_util_srcs += [ + 'util/json_writer_internal.cc', + 'util/simdjson_internal.cc', + ] + arrow_util_deps += [simdjson_dep] +endif + if needs_brotli arrow_util_srcs += ['util/compression_brotli.cc'] arrow_util_deps += [dependency('libbrotlidec'), dependency('libbrotlienc')] @@ -318,22 +342,6 @@ else rapidjson_dep = disabler() endif -if needs_json or needs_integration - simdjson_dep = dependency('simdjson', allow_fallback: false, required: false) - - if not simdjson_dep.found() - cmake = import('cmake') - simdjson_opts = cmake.subproject_options() - - simdjson_opts.add_cmake_defines({'SIMDJSON_EXCEPTIONS': 'OFF'}) - - simdjson_proj = cmake.subproject('simdjson', options: simdjson_opts) - simdjson_dep = simdjson_proj.dependency('simdjson') - endif -else - simdjson_dep = disabler() -endif - azure_dep = disabler() gcs_dep = disabler() s3_dep = disabler() @@ -530,7 +538,6 @@ if needs_json 'json/chunker.cc', 'json/converter.cc', 'json/from_string.cc', - 'json/json_writer_internal.cc', 'json/object_parser.cc', 'json/options.cc', 'json/parser.cc', diff --git a/cpp/src/arrow/util/CMakeLists.txt b/cpp/src/arrow/util/CMakeLists.txt index 628e9a4d1c7e..f77c8faa0caf 100644 --- a/cpp/src/arrow/util/CMakeLists.txt +++ b/cpp/src/arrow/util/CMakeLists.txt @@ -87,6 +87,14 @@ add_arrow_test(utility-test EXTRA_LINK_LIBS ${ARROW_UTILITY_TEST_LINK_LIBS}) +if(ARROW_WITH_SIMDJSON) + add_arrow_test(json-writer-internal-test + SOURCES + json_writer_internal_test.cc + EXTRA_LINK_LIBS + simdjson::simdjson) +endif() + add_arrow_test(async-utility-test SOURCES async_generator_test.cc diff --git a/cpp/src/arrow/json/json_writer_internal.cc b/cpp/src/arrow/util/json_writer_internal.cc similarity index 99% rename from cpp/src/arrow/json/json_writer_internal.cc rename to cpp/src/arrow/util/json_writer_internal.cc index e49841fda658..15175f0d313f 100644 --- a/cpp/src/arrow/json/json_writer_internal.cc +++ b/cpp/src/arrow/util/json_writer_internal.cc @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/json/json_writer_internal.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/simdjson_internal.h" namespace arrow::json { diff --git a/cpp/src/arrow/json/json_writer_internal.h b/cpp/src/arrow/util/json_writer_internal.h similarity index 100% rename from cpp/src/arrow/json/json_writer_internal.h rename to cpp/src/arrow/util/json_writer_internal.h diff --git a/cpp/src/arrow/json/json_writer_internal_test.cc b/cpp/src/arrow/util/json_writer_internal_test.cc similarity index 99% rename from cpp/src/arrow/json/json_writer_internal_test.cc rename to cpp/src/arrow/util/json_writer_internal_test.cc index a0123e808362..8d8e64528225 100644 --- a/cpp/src/arrow/json/json_writer_internal_test.cc +++ b/cpp/src/arrow/util/json_writer_internal_test.cc @@ -17,8 +17,8 @@ #include -#include "arrow/json/json_writer_internal.h" #include "arrow/testing/gtest_util.h" +#include "arrow/util/json_writer_internal.h" namespace sj = simdjson::ondemand; diff --git a/cpp/src/arrow/util/meson.build b/cpp/src/arrow/util/meson.build index c39e09c2d030..48a39072e3a6 100644 --- a/cpp/src/arrow/util/meson.build +++ b/cpp/src/arrow/util/meson.build @@ -222,6 +222,12 @@ utility_test_srcs = [ 'value_parsing_test.cc', ] +utility_test_deps = [arrow_test_dep_no_main] +if needs_json or needs_integration or needs_parquet + utility_test_srcs += ['json_writer_internal_test.cc'] + utility_test_deps += [simdjson_dep] +endif + if host_machine.system() == 'windows' # This manifest enables long file paths on Windows 10+ # See https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#enable-long-paths-in-windows-10-version-1607-and-later @@ -235,7 +241,7 @@ endif exc = executable( 'arrow-utility-test', sources: utility_test_srcs, - dependencies: arrow_test_dep_no_main, + dependencies: utility_test_deps, implicit_include_directories: false, ) test('arrow-utility-test', exc) diff --git a/cpp/src/arrow/util/simdjson_internal.cc b/cpp/src/arrow/util/simdjson_internal.cc new file mode 100644 index 000000000000..165b617768f3 --- /dev/null +++ b/cpp/src/arrow/util/simdjson_internal.cc @@ -0,0 +1,222 @@ +// 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. + +#include "arrow/util/simdjson_internal.h" + +namespace arrow::internal { + +const char* JsonTypeName(simdjson::dom::element_type type) { + switch (type) { + case simdjson::dom::element_type::ARRAY: + return "array"; + case simdjson::dom::element_type::OBJECT: + return "object"; + case simdjson::dom::element_type::INT64: + case simdjson::dom::element_type::UINT64: + case simdjson::dom::element_type::DOUBLE: + return "number"; + case simdjson::dom::element_type::STRING: + return "string"; + case simdjson::dom::element_type::BOOL: + return "boolean"; + case simdjson::dom::element_type::NULL_VALUE: + return "null"; + default: + return "unknown"; + } +} + +Result GetJsonArray(simdjson::dom::element value, + std::string_view name) { + if (!value.is_array()) { + return Status::Invalid(name, " must be an array, got ", JsonTypeName(value.type())); + } + return ResolveSimdjsonResult(value.get_array(), "Failed to get JSON array"); +} + +Result GetJsonInt(simdjson::dom::element value, std::string_view name, + std::string_view expected) { + if (!value.is_int64()) { + return Status::Invalid(name, " must contain ", expected, ", got ", + JsonTypeName(value.type())); + } + return ResolveSimdjsonResult(value.get_int64(), "Failed to get JSON integer"); +} + +Result ParseJsonObject(simdjson::dom::parser& parser, + const std::string& json) { + return ResolveSimdjsonResult(parser.parse(json).get_object(), + "Invalid serialized JSON data"); +} + +Result> GetOptionalJsonField( + const simdjson::dom::object& object, std::string_view key) { + auto field = object.at_key(key); + if (field.error() == simdjson::NO_SUCH_FIELD) { + return std::nullopt; + } + + ARROW_ASSIGN_OR_RAISE( + auto value, + ResolveSimdjsonResult(std::move(field), "Failed to get JSON object field")); + + return std::optional(std::move(value)); +} + +Result> GetJsonIntArray(simdjson::dom::element value, + std::string_view name) { + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonArray(value, name)); + + std::vector result; + result.reserve(array.size()); + + for (auto element : array) { + ARROW_ASSIGN_OR_RAISE(auto number, GetJsonInt(element, name, "integers")); + result.push_back(number); + } + + return result; +} + +Result>> GetJsonNullableIntArray( + simdjson::dom::element value, std::string_view name) { + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonArray(value, name)); + + std::vector> result; + result.reserve(array.size()); + + for (auto element : array) { + if (element.is_null()) { + result.emplace_back(std::nullopt); + } else { + ARROW_ASSIGN_OR_RAISE(auto number, GetJsonInt(element, name, "integers or nulls")); + result.emplace_back(number); + } + } + + return result; +} + +Result> GetJsonStringArray(simdjson::dom::element value, + std::string_view name) { + ARROW_ASSIGN_OR_RAISE(auto array, GetJsonArray(value, name)); + + std::vector result; + result.reserve(array.size()); + + for (auto element : array) { + if (!element.is_string()) { + return Status::Invalid(name, " must contain strings, got ", + JsonTypeName(element.type())); + } + + ARROW_ASSIGN_OR_RAISE( + auto string, + ResolveSimdjsonResult(element.get_string(), "Failed to get JSON string")); + result.emplace_back(string); + } + + return result; +} + +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 IsJsonNull(simdjson::ondemand::value& value) { + bool is_null; + 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)); + } + return is_null; +} + +Result MinifyJson(std::string_view json) { + std::string minified(json.size(), '\0'); + size_t minified_len = 0; + + if (auto error = + simdjson::minify(json.data(), json.size(), minified.data(), minified_len); + error != simdjson::SUCCESS) { + return Status::Invalid("Failed to minify JSON: ", simdjson::error_message(error)); + } + + minified.resize(minified_len); + return minified; +} + +Status ConsumeJsonValue(simdjson::ondemand::value value) { + return VisitJsonValue( + value, ValidateJsonObject, ValidateJsonArray, + [](std::string_view) { return Status::OK(); }, [](bool) { return Status::OK(); }, + []() { return Status::OK(); }, [](int64_t) { return Status::OK(); }, + [](uint64_t) { return Status::OK(); }, [](double) { return Status::OK(); }, + [](simdjson::ondemand::value) { return Status::OK(); }); +} + +Status ValidateJsonObject(simdjson::ondemand::object object) { + for (auto field_result : object) { + ARROW_ASSIGN_OR_RAISE( + auto field, ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); + + RETURN_NOT_OK(ConsumeJsonValue(field.value())); + } + + return Status::OK(); +} + +Status ValidateJsonArray(simdjson::ondemand::array array) { + for (auto element_result : array) { + ARROW_ASSIGN_OR_RAISE( + auto value, + ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); + + RETURN_NOT_OK(ConsumeJsonValue(value)); + } + + return Status::OK(); +} + +Status ValidateJsonDocument(simdjson::ondemand::parser& parser, + simdjson::padded_string& json) { + ARROW_ASSIGN_OR_RAISE( + auto document, ResolveSimdjsonResult(parser.iterate(json), "Failed to parse JSON")); + + ARROW_ASSIGN_OR_RAISE(auto value, ResolveSimdjsonResult(document.get_value(), + "Failed to get JSON value")); + + return ConsumeJsonValue(value); +} + +} // namespace arrow::internal diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index 799ebfc5ea62..c507cd4be789 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -29,9 +29,9 @@ #include "arrow/result.h" #include "arrow/status.h" +#include "arrow/util/visibility.h" -namespace arrow { -namespace internal { +namespace arrow::internal { // Empty struct to represent the type of a simdjson null value struct SimdjsonNull {}; @@ -94,121 +94,30 @@ Result ResolveSimdjsonResult(simdjson::simdjson_result result, return value; } -inline const char* JsonTypeName(simdjson::dom::element_type type) { - switch (type) { - case simdjson::dom::element_type::ARRAY: - return "array"; - case simdjson::dom::element_type::OBJECT: - return "object"; - case simdjson::dom::element_type::INT64: - case simdjson::dom::element_type::UINT64: - case simdjson::dom::element_type::DOUBLE: - return "number"; - case simdjson::dom::element_type::STRING: - return "string"; - case simdjson::dom::element_type::BOOL: - return "boolean"; - case simdjson::dom::element_type::NULL_VALUE: - return "null"; - default: - return "unknown"; - } -} +ARROW_EXPORT const char* JsonTypeName(simdjson::dom::element_type type); -inline Result GetJsonArray(simdjson::dom::element value, - std::string_view name) { - if (!value.is_array()) { - return Status::Invalid(name, " must be an array, got ", JsonTypeName(value.type())); - } - return ResolveSimdjsonResult(value.get_array(), "Failed to get JSON array"); -} +ARROW_EXPORT Result GetJsonArray(simdjson::dom::element value, + std::string_view name); -inline Result GetJsonInt(simdjson::dom::element value, std::string_view name, - std::string_view expected) { - if (!value.is_int64()) { - return Status::Invalid(name, " must contain ", expected, ", got ", - JsonTypeName(value.type())); - } - return ResolveSimdjsonResult(value.get_int64(), "Failed to get JSON integer"); -} +ARROW_EXPORT Result GetJsonInt(simdjson::dom::element value, + std::string_view name, std::string_view expected); -inline Result ParseJsonObject(simdjson::dom::parser& parser, - const std::string& json) { - return ResolveSimdjsonResult(parser.parse(json).get_object(), - "Invalid serialized JSON data"); -} +ARROW_EXPORT Result ParseJsonObject(simdjson::dom::parser& parser, + const std::string& json); // object.at_key() performs a linear search. This is acceptable here // since these objects are expected to contain only a small number of fields. -inline Result> GetOptionalJsonField( - const simdjson::dom::object& object, std::string_view key) { - auto field = object.at_key(key); - if (field.error() == simdjson::NO_SUCH_FIELD) { - return std::nullopt; - } - - ARROW_ASSIGN_OR_RAISE( - auto value, - ResolveSimdjsonResult(std::move(field), "Failed to get JSON object field")); - - return std::optional(std::move(value)); -} - -inline Result> GetJsonIntArray(simdjson::dom::element value, - std::string_view name) { - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonArray(value, name)); - - std::vector result; - result.reserve(array.size()); - - for (auto element : array) { - ARROW_ASSIGN_OR_RAISE(auto number, GetJsonInt(element, name, "integers")); - result.push_back(number); - } - - return result; -} - -inline Result>> GetJsonNullableIntArray( - simdjson::dom::element value, std::string_view name) { - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonArray(value, name)); - - std::vector> result; - result.reserve(array.size()); - - for (auto element : array) { - if (element.is_null()) { - result.emplace_back(std::nullopt); - } else { - ARROW_ASSIGN_OR_RAISE(auto number, GetJsonInt(element, name, "integers or nulls")); - result.emplace_back(number); - } - } - - return result; -} - -inline Result> GetJsonStringArray(simdjson::dom::element value, - std::string_view name) { - ARROW_ASSIGN_OR_RAISE(auto array, GetJsonArray(value, name)); +ARROW_EXPORT Result> GetOptionalJsonField( + const simdjson::dom::object& object, std::string_view key); - std::vector result; - result.reserve(array.size()); +ARROW_EXPORT Result> GetJsonIntArray(simdjson::dom::element value, + std::string_view name); - for (auto element : array) { - if (!element.is_string()) { - return Status::Invalid(name, " must contain strings, got ", - JsonTypeName(element.type())); - } +ARROW_EXPORT Result>> GetJsonNullableIntArray( + simdjson::dom::element value, std::string_view name); - ARROW_ASSIGN_OR_RAISE( - auto string, - ResolveSimdjsonResult(element.get_string(), "Failed to get JSON string")); - result.emplace_back(string); - } - - return result; -} +ARROW_EXPORT Result> GetJsonStringArray( + simdjson::dom::element value, std::string_view name); template because peeking the nonRootScalar can fail (parsed lazily) -inline Result IsJsonNull(simdjson::ondemand::value& value) { - bool is_null; - 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)); - } - return is_null; -} +ARROW_EXPORT Result IsJsonNull(simdjson::ondemand::value& value); template Result GetJsonAs(simdjson::ondemand::value& value) { @@ -386,66 +270,15 @@ Result GetJsonField(simdjson::ondemand::object& object, std::string_view key) return Status::KeyError("Missing JSON field: ", key); } -inline Result MinifyJson(std::string_view json) { - std::string minified(json.size(), '\0'); - size_t minified_len = 0; +ARROW_EXPORT Result MinifyJson(std::string_view json); - if (auto error = - simdjson::minify(json.data(), json.size(), minified.data(), minified_len); - error != simdjson::SUCCESS) { - return Status::Invalid("Failed to minify JSON: ", simdjson::error_message(error)); - } - - minified.resize(minified_len); - return minified; -} - -inline Status ValidateJsonObject(simdjson::ondemand::object object); +ARROW_EXPORT Status ValidateJsonObject(simdjson::ondemand::object object); -inline Status ValidateJsonArray(simdjson::ondemand::array array); +ARROW_EXPORT Status ValidateJsonArray(simdjson::ondemand::array array); -inline Status ConsumeJsonValue(simdjson::ondemand::value value) { - return VisitJsonValue( - value, ValidateJsonObject, ValidateJsonArray, - [](std::string_view) { return Status::OK(); }, [](bool) { return Status::OK(); }, - []() { return Status::OK(); }, [](int64_t) { return Status::OK(); }, - [](uint64_t) { return Status::OK(); }, [](double) { return Status::OK(); }, - [](simdjson::ondemand::value) { return Status::OK(); }); -} +ARROW_EXPORT Status ConsumeJsonValue(simdjson::ondemand::value value); -inline Status ValidateJsonObject(simdjson::ondemand::object object) { - for (auto field_result : object) { - ARROW_ASSIGN_OR_RAISE( - auto field, ResolveSimdjsonResult(field_result, "Failed to iterate JSON object")); - - RETURN_NOT_OK(ConsumeJsonValue(field.value())); - } - - return Status::OK(); -} - -inline Status ValidateJsonArray(simdjson::ondemand::array array) { - for (auto element_result : array) { - ARROW_ASSIGN_OR_RAISE( - auto value, - ResolveSimdjsonResult(element_result, "Failed to iterate JSON array")); - - RETURN_NOT_OK(ConsumeJsonValue(value)); - } - - return Status::OK(); -} - -inline Status ValidateJsonDocument(simdjson::ondemand::parser& parser, - simdjson::padded_string& json) { - ARROW_ASSIGN_OR_RAISE( - auto document, ResolveSimdjsonResult(parser.iterate(json), "Failed to parse JSON")); - - ARROW_ASSIGN_OR_RAISE(auto value, ResolveSimdjsonResult(document.get_value(), - "Failed to get JSON value")); - - return ConsumeJsonValue(value); -} +ARROW_EXPORT Status ValidateJsonDocument(simdjson::ondemand::parser& parser, + simdjson::padded_string& json); -} // namespace internal -} // namespace arrow +} // namespace arrow::internal diff --git a/cpp/src/parquet/encryption/file_system_key_material_store.cc b/cpp/src/parquet/encryption/file_system_key_material_store.cc index ece14b9664dc..4fce13f0d8c6 100644 --- a/cpp/src/parquet/encryption/file_system_key_material_store.cc +++ b/cpp/src/parquet/encryption/file_system_key_material_store.cc @@ -20,9 +20,9 @@ #include "arrow/buffer.h" #include "arrow/filesystem/filesystem.h" #include "arrow/filesystem/path_util.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/json/object_parser.h" #include "arrow/result.h" +#include "arrow/util/json_writer_internal.h" #include "parquet/encryption/file_system_key_material_store.h" #include "parquet/encryption/key_material.h" diff --git a/cpp/src/parquet/encryption/key_material.cc b/cpp/src/parquet/encryption/key_material.cc index 305ad5969852..827e1e445025 100644 --- a/cpp/src/parquet/encryption/key_material.cc +++ b/cpp/src/parquet/encryption/key_material.cc @@ -17,8 +17,8 @@ #include -#include "arrow/json/json_writer_internal.h" #include "arrow/json/object_parser.h" +#include "arrow/util/json_writer_internal.h" #include "parquet/encryption/key_material.h" #include "parquet/encryption/key_metadata.h" diff --git a/cpp/src/parquet/encryption/key_metadata.cc b/cpp/src/parquet/encryption/key_metadata.cc index 94253c87e3e8..fd213169ff80 100644 --- a/cpp/src/parquet/encryption/key_metadata.cc +++ b/cpp/src/parquet/encryption/key_metadata.cc @@ -17,8 +17,8 @@ #include -#include "arrow/json/json_writer_internal.h" #include "arrow/json/object_parser.h" +#include "arrow/util/json_writer_internal.h" #include "parquet/encryption/key_metadata.h" #include "parquet/exception.h" diff --git a/cpp/src/parquet/encryption/local_wrap_kms_client.cc b/cpp/src/parquet/encryption/local_wrap_kms_client.cc index dcb6c49836e0..c2d336de6815 100644 --- a/cpp/src/parquet/encryption/local_wrap_kms_client.cc +++ b/cpp/src/parquet/encryption/local_wrap_kms_client.cc @@ -17,8 +17,8 @@ #include -#include "arrow/json/json_writer_internal.h" #include "arrow/json/object_parser.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/secure_string.h" #include "parquet/encryption/key_toolkit_internal.h" diff --git a/cpp/src/parquet/geospatial/util_json_internal.cc b/cpp/src/parquet/geospatial/util_json_internal.cc index 44efcfc8d764..9bdc82fe1d8e 100644 --- a/cpp/src/parquet/geospatial/util_json_internal.cc +++ b/cpp/src/parquet/geospatial/util_json_internal.cc @@ -20,8 +20,8 @@ #include #include "arrow/extension_type.h" -#include "arrow/json/json_writer_internal.h" #include "arrow/result.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/simdjson_internal.h" #include "arrow/util/string.h" diff --git a/cpp/src/parquet/printer.cc b/cpp/src/parquet/printer.cc index dd6a21913bf6..053183c41b42 100644 --- a/cpp/src/parquet/printer.cc +++ b/cpp/src/parquet/printer.cc @@ -25,7 +25,7 @@ #include #include -#include "arrow/json/json_writer_internal.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/key_value_metadata.h" #include "arrow/util/string.h" diff --git a/cpp/src/parquet/types.cc b/cpp/src/parquet/types.cc index 534c79201c0f..ba80a27137a7 100644 --- a/cpp/src/parquet/types.cc +++ b/cpp/src/parquet/types.cc @@ -24,11 +24,11 @@ #include #include -#include "arrow/json/json_writer_internal.h" #include "arrow/util/checked_cast.h" #include "arrow/util/compression.h" #include "arrow/util/decimal.h" #include "arrow/util/float16.h" +#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "parquet/exception.h" From 86e85bd69b09e387b282bb1d51387856479cd381 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 27 Aug 2026 17:28:37 +0200 Subject: [PATCH 2/7] simplify meson change --- cpp/meson.build | 1 + cpp/src/arrow/meson.build | 4 ++-- cpp/src/arrow/util/meson.build | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cpp/meson.build b/cpp/meson.build index 023877e19d11..5532d866db33 100644 --- a/cpp/meson.build +++ b/cpp/meson.build @@ -104,6 +104,7 @@ needs_testing = (get_option('testing').enabled() or needs_integration ) needs_json = get_option('json').enabled() or needs_testing +needs_simdjson = needs_json or needs_parquet needs_brotli = get_option('brotli').enabled() or needs_fuzzing needs_bz2 = get_option('bz2').enabled() needs_lz4 = get_option('lz4').enabled() diff --git a/cpp/src/arrow/meson.build b/cpp/src/arrow/meson.build index 298c7a5e02c1..5c475902d076 100644 --- a/cpp/src/arrow/meson.build +++ b/cpp/src/arrow/meson.build @@ -18,7 +18,7 @@ dl_dep = dependency('dl') threads_dep = dependency('threads') -if needs_json or needs_integration or needs_parquet +if needs_simdjson simdjson_dep = dependency('simdjson', allow_fallback: false, required: false) if not simdjson_dep.found() @@ -240,7 +240,7 @@ arrow_util_srcs = [ arrow_util_deps = [threads_dep] -if needs_json or needs_integration or needs_parquet +if needs_simdjson arrow_util_srcs += [ 'util/json_writer_internal.cc', 'util/simdjson_internal.cc', diff --git a/cpp/src/arrow/util/meson.build b/cpp/src/arrow/util/meson.build index 48a39072e3a6..456b8c61c7a3 100644 --- a/cpp/src/arrow/util/meson.build +++ b/cpp/src/arrow/util/meson.build @@ -223,7 +223,7 @@ utility_test_srcs = [ ] utility_test_deps = [arrow_test_dep_no_main] -if needs_json or needs_integration or needs_parquet +if needs_simdjson utility_test_srcs += ['json_writer_internal_test.cc'] utility_test_deps += [simdjson_dep] endif From 4d93dff9b3396f3484749fb1e4c6f8937c16ac1f Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Thu, 27 Aug 2026 19:13:49 +0200 Subject: [PATCH 3/7] set ARROW_JSON for odbc build --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 8888f52f721b..e65f6d3f174f 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -411,11 +411,11 @@ if(ARROW_AZURE) set(ARROW_WITH_AZURE_SDK ON) endif() -if(ARROW_JSON OR ARROW_FLIGHT_SQL_ODBC) +if(ARROW_JSON) set(ARROW_WITH_RAPIDJSON ON) endif() -if(ARROW_JSON) +if(ARROW_JSON OR ARROW_FLIGHT_SQL_ODBC) set(ARROW_WITH_SIMDJSON ON) endif() From ab2c41a1cea6ad48205f727c8602aaaa5ecda805 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Fri, 28 Aug 2026 12:49:16 +0200 Subject: [PATCH 4/7] GH-50859: [C++][Parquet] Build ObjectParser with simdjson utilities Fully qualify simdjson utility calls so CMake unity builds do not resolve internal to arrow::json::internal. --- cpp/cmake_modules/DefineOptions.cmake | 3 +-- cpp/cmake_modules/ThirdpartyToolchain.cmake | 1 - cpp/src/arrow/CMakeLists.txt | 7 +++++-- cpp/src/arrow/json/object_parser.h | 2 +- cpp/src/arrow/meson.build | 2 +- cpp/src/arrow/util/CMakeLists.txt | 2 +- cpp/src/arrow/util/json_writer_internal.cc | 18 +++++++++--------- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/cpp/cmake_modules/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake index bfe4485aa912..1d12edd0613e 100644 --- a/cpp/cmake_modules/DefineOptions.cmake +++ b/cpp/cmake_modules/DefineOptions.cmake @@ -599,8 +599,7 @@ takes precedence over ccache if a storage backend is configured" ON) "Build support for encryption. Fail if OpenSSL is not found" OFF DEPENDS - ARROW_FILESYSTEM - ARROW_JSON) + ARROW_FILESYSTEM) #---------------------------------------------------------------------- set_option_category("Gandiva") diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index e65f6d3f174f..0fc8f234355a 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -383,7 +383,6 @@ if(ARROW_WITH_OPENTELEMETRY) endif() if(ARROW_PARQUET) - set(ARROW_WITH_RAPIDJSON ON) set(ARROW_WITH_SIMDJSON ON) set(ARROW_WITH_THRIFT ON) endif() diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index ed8174f7176a..122012efbeda 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -645,7 +645,11 @@ if(ARROW_WITH_OPENTELEMETRY) list(APPEND ARROW_UTIL_SRCS util/tracing_internal.cc) endif() if(ARROW_WITH_SIMDJSON) - list(APPEND ARROW_UTIL_SRCS util/json_writer_internal.cc util/simdjson_internal.cc) + list(APPEND + ARROW_UTIL_SRCS + json/object_parser.cc + util/json_writer_internal.cc + util/simdjson_internal.cc) endif() if(ARROW_WITH_SNAPPY) list(APPEND ARROW_UTIL_SRCS util/compression_snappy.cc) @@ -1097,7 +1101,6 @@ if(ARROW_JSON) json/chunker.cc json/converter.cc json/from_string.cc - json/object_parser.cc json/parser.cc json/reader.cc) foreach(ARROW_JSON_TARGET ${ARROW_JSON_TARGETS}) diff --git a/cpp/src/arrow/json/object_parser.h b/cpp/src/arrow/json/object_parser.h index 8035695e537c..bd04525efa50 100644 --- a/cpp/src/arrow/json/object_parser.h +++ b/cpp/src/arrow/json/object_parser.h @@ -29,7 +29,7 @@ namespace json { namespace internal { /// This class is a helper to parse a json object from a string. -/// It uses rapidjson::Document in implementation. +/// It uses simdjson in the implementation. class ARROW_EXPORT ObjectParser { public: ObjectParser(); diff --git a/cpp/src/arrow/meson.build b/cpp/src/arrow/meson.build index 5c475902d076..3315faccd860 100644 --- a/cpp/src/arrow/meson.build +++ b/cpp/src/arrow/meson.build @@ -242,6 +242,7 @@ arrow_util_deps = [threads_dep] if needs_simdjson arrow_util_srcs += [ + 'json/object_parser.cc', 'util/json_writer_internal.cc', 'util/simdjson_internal.cc', ] @@ -538,7 +539,6 @@ if needs_json 'json/chunker.cc', 'json/converter.cc', 'json/from_string.cc', - 'json/object_parser.cc', 'json/options.cc', 'json/parser.cc', 'json/reader.cc', diff --git a/cpp/src/arrow/util/CMakeLists.txt b/cpp/src/arrow/util/CMakeLists.txt index f77c8faa0caf..2744737eb8f2 100644 --- a/cpp/src/arrow/util/CMakeLists.txt +++ b/cpp/src/arrow/util/CMakeLists.txt @@ -92,7 +92,7 @@ if(ARROW_WITH_SIMDJSON) SOURCES json_writer_internal_test.cc EXTRA_LINK_LIBS - simdjson::simdjson) + arrow::simdjson) endif() add_arrow_test(async-utility-test diff --git a/cpp/src/arrow/util/json_writer_internal.cc b/cpp/src/arrow/util/json_writer_internal.cc index 15175f0d313f..26d58d9b20f2 100644 --- a/cpp/src/arrow/util/json_writer_internal.cc +++ b/cpp/src/arrow/util/json_writer_internal.cc @@ -100,21 +100,21 @@ void JsonWriter::Double(double value) { } Status JsonWriter::WriteValue(sj::value value) { - return internal::VisitJsonValue( + return ::arrow::internal::VisitJsonValue( value, [&](sj::object object) -> Status { StartObject(); for (auto field : object) { - ARROW_ASSIGN_OR_RAISE( - auto key, internal::ResolveSimdjsonResult(field.unescaped_key(), - "Failed to get object key")); + ARROW_ASSIGN_OR_RAISE(auto key, + ::arrow::internal::ResolveSimdjsonResult( + field.unescaped_key(), "Failed to get object key")); Key(key); ARROW_ASSIGN_OR_RAISE(auto field_value, - internal::ResolveSimdjsonResult( + ::arrow::internal::ResolveSimdjsonResult( field.value(), "Failed to get object value")); RETURN_NOT_OK(WriteValue(field_value)); @@ -128,9 +128,9 @@ Status JsonWriter::WriteValue(sj::value value) { StartArray(); for (auto element : array) { - ARROW_ASSIGN_OR_RAISE( - auto element_value, - internal::ResolveSimdjsonResult(element, "Failed to iterate JSON array")); + ARROW_ASSIGN_OR_RAISE(auto element_value, + ::arrow::internal::ResolveSimdjsonResult( + element, "Failed to iterate JSON array")); RETURN_NOT_OK(WriteValue(element_value)); } @@ -170,7 +170,7 @@ Status JsonWriter::WriteValue(sj::value value) { }, [&](sj::value value) -> Status { - ARROW_ASSIGN_OR_RAISE(auto raw_json, internal::ResolveSimdjsonResult( + ARROW_ASSIGN_OR_RAISE(auto raw_json, ::arrow::internal::ResolveSimdjsonResult( simdjson::to_json_string(value), "Failed to get raw JSON")); RawValue(raw_json); From ea1bc356aeb340f0f24aab603f4b48bac8b1f8f4 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Mon, 31 Aug 2026 13:57:16 +0200 Subject: [PATCH 5/7] move everything into simdjson_internal --- cpp/src/arrow/CMakeLists.txt | 6 +- cpp/src/arrow/dataset/file_json_test.cc | 8 +- cpp/src/arrow/extension/fixed_shape_tensor.cc | 3 +- cpp/src/arrow/extension/opaque.cc | 3 +- .../arrow/extension/variable_shape_tensor.cc | 3 +- .../sql/odbc/odbc_impl/json_converter.cc | 4 +- cpp/src/arrow/integration/json_integration.cc | 3 +- .../integration/json_integration_test.cc | 5 +- cpp/src/arrow/integration/json_internal.cc | 3 +- cpp/src/arrow/integration/json_internal.h | 13 +- cpp/src/arrow/json/CMakeLists.txt | 1 - cpp/src/arrow/json/meson.build | 1 - cpp/src/arrow/json/object_parser.cc | 178 --------- cpp/src/arrow/json/object_parser.h | 54 --- cpp/src/arrow/json/object_parser_test.cc | 113 ------ cpp/src/arrow/meson.build | 6 +- cpp/src/arrow/util/CMakeLists.txt | 4 +- cpp/src/arrow/util/json_writer_internal.cc | 232 ----------- cpp/src/arrow/util/json_writer_internal.h | 77 ---- cpp/src/arrow/util/meson.build | 2 +- cpp/src/arrow/util/simdjson_internal.cc | 359 ++++++++++++++++++ cpp/src/arrow/util/simdjson_internal.h | 69 ++++ ...rnal_test.cc => simdjson_internal_test.cc} | 89 ++++- .../file_system_key_material_store.cc | 7 +- cpp/src/parquet/encryption/key_material.cc | 7 +- cpp/src/parquet/encryption/key_material.h | 11 +- cpp/src/parquet/encryption/key_metadata.cc | 7 +- .../encryption/local_wrap_kms_client.cc | 7 +- .../parquet/geospatial/util_json_internal.cc | 3 +- cpp/src/parquet/printer.cc | 4 +- cpp/src/parquet/types.cc | 18 +- 31 files changed, 565 insertions(+), 735 deletions(-) delete mode 100644 cpp/src/arrow/json/object_parser.cc delete mode 100644 cpp/src/arrow/json/object_parser.h delete mode 100644 cpp/src/arrow/json/object_parser_test.cc delete mode 100644 cpp/src/arrow/util/json_writer_internal.cc delete mode 100644 cpp/src/arrow/util/json_writer_internal.h rename cpp/src/arrow/util/{json_writer_internal_test.cc => simdjson_internal_test.cc} (81%) diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 122012efbeda..d7086773a1fd 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -645,11 +645,7 @@ if(ARROW_WITH_OPENTELEMETRY) list(APPEND ARROW_UTIL_SRCS util/tracing_internal.cc) endif() if(ARROW_WITH_SIMDJSON) - list(APPEND - ARROW_UTIL_SRCS - json/object_parser.cc - util/json_writer_internal.cc - util/simdjson_internal.cc) + list(APPEND ARROW_UTIL_SRCS util/simdjson_internal.cc) endif() if(ARROW_WITH_SNAPPY) list(APPEND ARROW_UTIL_SRCS util/compression_snappy.cc) diff --git a/cpp/src/arrow/dataset/file_json_test.cc b/cpp/src/arrow/dataset/file_json_test.cc index e3d0c15776ca..aa060e73bb22 100644 --- a/cpp/src/arrow/dataset/file_json_test.cc +++ b/cpp/src/arrow/dataset/file_json_test.cc @@ -23,8 +23,8 @@ #include "arrow/json/parser.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/util.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" +#include "arrow/util/simdjson_internal.h" namespace arrow { @@ -104,11 +104,11 @@ struct WriteVisitor { return Status::OK(); } - json::JsonWriter& writer_; + ::arrow::internal::JsonWriter& writer_; const Scalar& scalar_; }; -Status WriteJson(const StructScalar& scalar, json::JsonWriter* writer) { +Status WriteJson(const StructScalar& scalar, ::arrow::internal::JsonWriter* writer) { WriteVisitor visitor{*writer, scalar}; return VisitWriteableTypeId(Type::STRUCT, &visitor); } @@ -122,7 +122,7 @@ class JsonFormatHelper { std::stringstream ss; for (const auto& scalar : scalars) { - json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; RETURN_NOT_OK(WriteJson(*scalar, &writer)); ARROW_ASSIGN_OR_RAISE(auto json, writer.GetString()); diff --git a/cpp/src/arrow/extension/fixed_shape_tensor.cc b/cpp/src/arrow/extension/fixed_shape_tensor.cc index 48799899c7e2..69eed2eed00b 100644 --- a/cpp/src/arrow/extension/fixed_shape_tensor.cc +++ b/cpp/src/arrow/extension/fixed_shape_tensor.cc @@ -28,14 +28,13 @@ #include "arrow/array/array_nested.h" #include "arrow/array/array_primitive.h" #include "arrow/tensor.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/print_internal.h" #include "arrow/util/simdjson_internal.h" #include "arrow/util/sort_internal.h" #include "arrow/util/string.h" -using ::arrow::json::JsonWriter; +using ::arrow::internal::JsonWriter; namespace arrow::extension { diff --git a/cpp/src/arrow/extension/opaque.cc b/cpp/src/arrow/extension/opaque.cc index 6e5a07405f5a..2dae9ef56788 100644 --- a/cpp/src/arrow/extension/opaque.cc +++ b/cpp/src/arrow/extension/opaque.cc @@ -19,13 +19,12 @@ #include -#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/simdjson_internal.h" #include -using ::arrow::json::JsonWriter; +using ::arrow::internal::JsonWriter; namespace arrow::extension { diff --git a/cpp/src/arrow/extension/variable_shape_tensor.cc b/cpp/src/arrow/extension/variable_shape_tensor.cc index 0ecd9f18c80b..0e4f5f54ca7b 100644 --- a/cpp/src/arrow/extension/variable_shape_tensor.cc +++ b/cpp/src/arrow/extension/variable_shape_tensor.cc @@ -25,14 +25,13 @@ #include "arrow/array/array_primitive.h" #include "arrow/scalar.h" #include "arrow/tensor.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/print_internal.h" #include "arrow/util/simdjson_internal.h" #include "arrow/util/sort_internal.h" #include "arrow/util/string.h" -using ::arrow::json::JsonWriter; +using ::arrow::internal::JsonWriter; namespace arrow::extension { diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc b/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc index 5ced882545db..ea2fad42570d 100644 --- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc +++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc @@ -21,7 +21,7 @@ #include "arrow/builder.h" #include "arrow/flight/sql/odbc/odbc_impl/util.h" #include "arrow/scalar.h" -#include "arrow/util/json_writer_internal.h" +#include "arrow/util/simdjson_internal.h" #include "arrow/visitor.h" using boost::beast::detail::base64::encode; @@ -30,7 +30,7 @@ namespace base64 = boost::beast::detail::base64; namespace arrow::flight::sql::odbc { -using ::arrow::json::JsonWriter; +using ::arrow::internal::JsonWriter; using util::ThrowIfNotOK; namespace { diff --git a/cpp/src/arrow/integration/json_integration.cc b/cpp/src/arrow/integration/json_integration.cc index 5a1def6d0b40..98bb58f28657 100644 --- a/cpp/src/arrow/integration/json_integration.cc +++ b/cpp/src/arrow/integration/json_integration.cc @@ -33,14 +33,13 @@ #include "arrow/result.h" #include "arrow/status.h" #include "arrow/type.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" #include "arrow/util/simdjson_internal.h" using arrow::ipc::DictionaryFieldMapper; using arrow::ipc::DictionaryMemo; -using JsonWriter = arrow::json::JsonWriter; +using JsonWriter = arrow::internal::JsonWriter; namespace arrow::internal::integration { diff --git a/cpp/src/arrow/integration/json_integration_test.cc b/cpp/src/arrow/integration/json_integration_test.cc index 6687f26224c6..98bbd3f56ba3 100644 --- a/cpp/src/arrow/integration/json_integration_test.cc +++ b/cpp/src/arrow/integration/json_integration_test.cc @@ -48,7 +48,6 @@ #include "arrow/type.h" #include "arrow/type_fwd.h" #include "arrow/util/io_util.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/simdjson_internal.h" DEFINE_string(arrow, "", "Arrow file name"); @@ -725,7 +724,7 @@ static const char* json_example6 = R"example( )example"; void TestSchemaRoundTrip(const std::shared_ptr& schema) { - arrow::json::JsonWriter writer; + arrow::internal::JsonWriter writer; DictionaryFieldMapper mapper(*schema); @@ -749,7 +748,7 @@ void TestSchemaRoundTrip(const std::shared_ptr& schema) { void TestArrayRoundTrip(const Array& array) { static std::string name = "dummy"; - arrow::json::JsonWriter writer; + arrow::internal::JsonWriter writer; ASSERT_OK(json::WriteArray(name, array, &writer)); diff --git a/cpp/src/arrow/integration/json_internal.cc b/cpp/src/arrow/integration/json_internal.cc index 4ed852b121bf..0f72207bd4d8 100644 --- a/cpp/src/arrow/integration/json_internal.cc +++ b/cpp/src/arrow/integration/json_internal.cc @@ -47,7 +47,6 @@ #include "arrow/util/checked_cast.h" #include "arrow/util/decimal.h" #include "arrow/util/formatting.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/key_value_metadata.h" #include "arrow/util/logging_internal.h" #include "arrow/util/range.h" @@ -66,7 +65,7 @@ using arrow::ipc::DictionaryFieldMapper; using arrow::ipc::DictionaryMemo; using arrow::ipc::internal::FieldPosition; -using JsonWriter = arrow::json::JsonWriter; +using JsonWriter = arrow::internal::JsonWriter; namespace arrow::internal::integration::json { diff --git a/cpp/src/arrow/integration/json_internal.h b/cpp/src/arrow/integration/json_internal.h index b61d5b574606..c2d6fd3d45ce 100644 --- a/cpp/src/arrow/integration/json_internal.h +++ b/cpp/src/arrow/integration/json_internal.h @@ -32,26 +32,25 @@ using JsonValue = simdjson::dom::element; using JsonObject = simdjson::dom::object; using JsonArray = simdjson::dom::array; -namespace arrow::json { +namespace arrow::internal { class JsonWriter; -} // namespace arrow::json +} // namespace arrow::internal namespace arrow::internal::integration::json { /// \brief Append integration test Schema format to JSON writer ARROW_EXPORT Status WriteSchema(const Schema& schema, const ipc::DictionaryFieldMapper& mapper, - arrow::json::JsonWriter*); + JsonWriter*); ARROW_EXPORT -Status WriteDictionary(int64_t id, const std::shared_ptr& dictionary, - arrow::json::JsonWriter*); +Status WriteDictionary(int64_t id, const std::shared_ptr& dictionary, JsonWriter*); ARROW_EXPORT -Status WriteRecordBatch(const RecordBatch& batch, arrow::json::JsonWriter*); +Status WriteRecordBatch(const RecordBatch& batch, JsonWriter*); ARROW_EXPORT -Status WriteArray(const std::string& name, const Array& array, arrow::json::JsonWriter*); +Status WriteArray(const std::string& name, const Array& array, JsonWriter*); ARROW_EXPORT Result> ReadSchema(const JsonValue& json_obj, MemoryPool* pool, diff --git a/cpp/src/arrow/json/CMakeLists.txt b/cpp/src/arrow/json/CMakeLists.txt index cad6f0fbdf3f..b930034537d0 100644 --- a/cpp/src/arrow/json/CMakeLists.txt +++ b/cpp/src/arrow/json/CMakeLists.txt @@ -21,7 +21,6 @@ add_arrow_test(test chunker_test.cc converter_test.cc from_string_test.cc - object_parser_test.cc parser_test.cc reader_test.cc PREFIX diff --git a/cpp/src/arrow/json/meson.build b/cpp/src/arrow/json/meson.build index 8c5813feec41..e5383b90b919 100644 --- a/cpp/src/arrow/json/meson.build +++ b/cpp/src/arrow/json/meson.build @@ -43,7 +43,6 @@ install_headers( 'chunker.h', 'converter.h', 'from_string.h', - 'object_parser.h', 'options.h', 'parser.h', 'rapidjson_defs.h', diff --git a/cpp/src/arrow/json/object_parser.cc b/cpp/src/arrow/json/object_parser.cc deleted file mode 100644 index 4aad2aca679c..000000000000 --- a/cpp/src/arrow/json/object_parser.cc +++ /dev/null @@ -1,178 +0,0 @@ -// 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. - -#include "arrow/json/object_parser.h" - -#include - -namespace arrow { -namespace json { -namespace internal { - -class ObjectParser::Impl { - public: - Status Parse(std::string_view json) { - // Copy into padded buffer - padded_json_ = simdjson::padded_string(json); - - // Store parsed document - if (auto error = parser_.iterate(padded_json_).get(document_)) { - return Status::Invalid("JSON parse error: ", simdjson::error_message(error)); - } - - // Validate root is an object - auto object = document_.get_object(); - if (object.error()) { - if (object.error() == simdjson::INCORRECT_TYPE) { - return Status::TypeError("Not a JSON object"); - } - return Status::Invalid("JSON parse error: ", - simdjson::error_message(object.error())); - } - - return Status::OK(); - } - - Result GetString(const char* key) { - document_.rewind(); - - auto object = document_.get_object(); - - auto field = object.find_field(key); - - if (field.error() == simdjson::NO_SUCH_FIELD) { - return Status::KeyError("Key '", key, "' does not exist"); - } - if (field.error()) { - return Status::Invalid("Error accessing key '", key, - "': ", simdjson::error_message(field.error())); - } - - auto str_result = field.get_string(); - if (str_result.error() == simdjson::INCORRECT_TYPE) { - return Status::TypeError("Key '", key, "' is not a string"); - } - if (str_result.error()) { - return Status::Invalid("Error getting string for key '", key, - "': ", simdjson::error_message(str_result.error())); - } - - std::string_view str; - if (auto error = std::move(str_result).get(str)) { - return Status::Invalid("Error getting string for key '", key, - "': ", simdjson::error_message(error)); - } - return std::string(str); - } - - Result> GetStringMap() { - std::unordered_map map; - - document_.rewind(); - - auto object = document_.get_object(); - - for (auto field : object) { - std::string_view key; - if (auto error = field.unescaped_key().get(key)) { - return Status::Invalid("Error getting object key: ", - simdjson::error_message(error)); - } - - auto value = field.value(); - - auto str_result = value.get_string(); - - if (str_result.error() == simdjson::INCORRECT_TYPE) { - return Status::TypeError("Key '", std::string(key), - "' does not have a string value"); - } - if (str_result.error()) { - return Status::Invalid("Error getting value for key '", std::string(key), - "': (code=", static_cast(str_result.error()), ")"); - } - - std::string_view str; - if (auto error = std::move(str_result).get(str)) { - return Status::Invalid("Error getting value for key '", std::string(key), - "': ", simdjson::error_message(error)); - } - - map.emplace(std::string(key), std::string(str)); - } - - return map; - } - - Result GetBool(const char* key) { - document_.rewind(); - - auto object = document_.get_object(); - - auto field = object.find_field(key); - - if (field.error() == simdjson::NO_SUCH_FIELD) { - return Status::KeyError("Key '", key, "' does not exist"); - } - if (field.error()) { - return Status::Invalid("Error accessing key '", key, - "': ", simdjson::error_message(field.error())); - } - - auto bool_result = field.get_bool(); - if (bool_result.error() == simdjson::INCORRECT_TYPE) { - return Status::TypeError("Key '", key, "' is not a boolean"); - } - if (bool_result.error()) { - return Status::Invalid("Error getting bool for key '", key, - "': ", simdjson::error_message(bool_result.error())); - } - - bool value; - if (auto error = std::move(bool_result).get(value)) { - return Status::Invalid("Error getting bool for key '", key, - "': ", simdjson::error_message(error)); - } - - return value; - } - - private: - simdjson::ondemand::parser parser_; - simdjson::padded_string padded_json_; - simdjson::ondemand::document document_; -}; - -ObjectParser::ObjectParser() : impl_(new ObjectParser::Impl()) {} - -ObjectParser::~ObjectParser() = default; - -Status ObjectParser::Parse(std::string_view json) { return impl_->Parse(json); } - -Result ObjectParser::GetString(const char* key) const { - return impl_->GetString(key); -} - -Result ObjectParser::GetBool(const char* key) const { return impl_->GetBool(key); } - -Result> ObjectParser::GetStringMap() const { - return impl_->GetStringMap(); -} - -} // namespace internal -} // namespace json -} // namespace arrow diff --git a/cpp/src/arrow/json/object_parser.h b/cpp/src/arrow/json/object_parser.h deleted file mode 100644 index bd04525efa50..000000000000 --- a/cpp/src/arrow/json/object_parser.h +++ /dev/null @@ -1,54 +0,0 @@ -// 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 - -#include "arrow/result.h" -#include "arrow/util/visibility.h" - -namespace arrow { -namespace json { -namespace internal { - -/// This class is a helper to parse a json object from a string. -/// It uses simdjson in the implementation. -class ARROW_EXPORT ObjectParser { - public: - ObjectParser(); - ~ObjectParser(); - - Status Parse(std::string_view json); - - Result GetString(const char* key) const; - - Result GetBool(const char* key) const; - - // Get all members of the object as a map from string keys to string values - Result> GetStringMap() const; - - private: - class Impl; - std::unique_ptr impl_; -}; - -} // namespace internal -} // namespace json -} // namespace arrow diff --git a/cpp/src/arrow/json/object_parser_test.cc b/cpp/src/arrow/json/object_parser_test.cc deleted file mode 100644 index b9465aee4cf0..000000000000 --- a/cpp/src/arrow/json/object_parser_test.cc +++ /dev/null @@ -1,113 +0,0 @@ -// 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. - -#include "arrow/json/object_parser.h" - -#include -#include - -#include -#include - -#include "arrow/testing/gtest_util.h" - -namespace arrow { -namespace json { -namespace internal { - -TEST(ObjectParser, GetString) { - ObjectParser parser; - - ASSERT_OK(parser.Parse(R"({"name":"arrow"})")); - - ASSERT_OK_AND_ASSIGN(auto value, parser.GetString("name")); - EXPECT_EQ(value, "arrow"); -} - -TEST(ObjectParser, GetBool) { - ObjectParser parser; - - ASSERT_OK(parser.Parse(R"({"enabled":true})")); - - ASSERT_OK_AND_ASSIGN(auto value, parser.GetBool("enabled")); - EXPECT_TRUE(value); -} - -TEST(ObjectParser, InvalidJson) { - ObjectParser parser; - - EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, ::testing::HasSubstr("JSON parse error"), - parser.Parse(R"({"name":)")); -} - -TEST(ObjectParser, GetStringMap) { - ObjectParser parser; - - ASSERT_OK(parser.Parse(R"({ - "k1": "v1", - "k2": "v2" - })")); - - ASSERT_OK_AND_ASSIGN(auto map, parser.GetStringMap()); - - ASSERT_EQ(map.size(), 2U); - EXPECT_EQ(map["k1"], "v1"); - EXPECT_EQ(map["k2"], "v2"); -} - -TEST(ObjectParser, MissingKey) { - ObjectParser parser; - - ASSERT_OK(parser.Parse(R"({ - "name": "arrow" - })")); - - ASSERT_RAISES(KeyError, parser.GetString("missing")); - ASSERT_RAISES(KeyError, parser.GetBool("missing")); -} - -TEST(ObjectParser, WrongType) { - ObjectParser parser; - - ASSERT_OK(parser.Parse(R"({ - "flag": true, - "name": "arrow" - })")); - - ASSERT_RAISES(TypeError, parser.GetString("flag")); - ASSERT_RAISES(TypeError, parser.GetBool("name")); -} - -TEST(ObjectParser, NonObjectRoot) { - ObjectParser parser; - - ASSERT_RAISES(TypeError, parser.Parse(R"(["a", "b"])")); -} - -TEST(ObjectParser, EmptyObject) { - ObjectParser parser; - - ASSERT_OK(parser.Parse(R"({})")); - - ASSERT_OK_AND_ASSIGN(auto map, parser.GetStringMap()); - - EXPECT_TRUE(map.empty()); -} - -} // namespace internal -} // namespace json -} // namespace arrow diff --git a/cpp/src/arrow/meson.build b/cpp/src/arrow/meson.build index 3315faccd860..f181321aaab7 100644 --- a/cpp/src/arrow/meson.build +++ b/cpp/src/arrow/meson.build @@ -241,11 +241,7 @@ arrow_util_srcs = [ arrow_util_deps = [threads_dep] if needs_simdjson - arrow_util_srcs += [ - 'json/object_parser.cc', - 'util/json_writer_internal.cc', - 'util/simdjson_internal.cc', - ] + arrow_util_srcs += ['util/simdjson_internal.cc'] arrow_util_deps += [simdjson_dep] endif diff --git a/cpp/src/arrow/util/CMakeLists.txt b/cpp/src/arrow/util/CMakeLists.txt index 2744737eb8f2..c67abf55a251 100644 --- a/cpp/src/arrow/util/CMakeLists.txt +++ b/cpp/src/arrow/util/CMakeLists.txt @@ -88,9 +88,9 @@ add_arrow_test(utility-test ${ARROW_UTILITY_TEST_LINK_LIBS}) if(ARROW_WITH_SIMDJSON) - add_arrow_test(json-writer-internal-test + add_arrow_test(simdjson-internal-test SOURCES - json_writer_internal_test.cc + simdjson_internal_test.cc EXTRA_LINK_LIBS arrow::simdjson) endif() diff --git a/cpp/src/arrow/util/json_writer_internal.cc b/cpp/src/arrow/util/json_writer_internal.cc deleted file mode 100644 index 26d58d9b20f2..000000000000 --- a/cpp/src/arrow/util/json_writer_internal.cc +++ /dev/null @@ -1,232 +0,0 @@ -// 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. - -#include "arrow/util/json_writer_internal.h" -#include "arrow/util/simdjson_internal.h" - -namespace arrow::json { - -namespace sj = simdjson::ondemand; - -void JsonWriter::StartObject() { - MaybeComma(); - builder_.start_object(); - needs_comma_ = false; -} - -void JsonWriter::EndObject() { - builder_.end_object(); - needs_comma_ = true; -} - -void JsonWriter::StartArray() { - MaybeComma(); - builder_.start_array(); - needs_comma_ = false; -} - -void JsonWriter::EndArray() { - builder_.end_array(); - needs_comma_ = true; -} - -void JsonWriter::Key(std::string_view key) { - MaybeComma(); - builder_.escape_and_append_with_quotes(key); - builder_.append_colon(); - needs_comma_ = false; -} - -void JsonWriter::String(std::string_view value) { - MaybeComma(); - builder_.escape_and_append_with_quotes(value); - needs_comma_ = true; -} - -void JsonWriter::RawValue(std::string_view value) { - MaybeComma(); - builder_.append_raw(value); - needs_comma_ = true; -} - -void JsonWriter::Bool(bool value) { - MaybeComma(); - builder_.append(value); - needs_comma_ = true; -} - -void JsonWriter::Int(int32_t value) { - MaybeComma(); - builder_.append(value); - needs_comma_ = true; -} - -void JsonWriter::Int64(int64_t value) { - MaybeComma(); - builder_.append(value); - needs_comma_ = true; -} - -void JsonWriter::Uint(uint32_t value) { - MaybeComma(); - builder_.append(value); - needs_comma_ = true; -} - -void JsonWriter::Uint64(uint64_t value) { - MaybeComma(); - builder_.append(value); - needs_comma_ = true; -} - -void JsonWriter::Double(double value) { - MaybeComma(); - builder_.append(value); - needs_comma_ = true; -} - -Status JsonWriter::WriteValue(sj::value value) { - return ::arrow::internal::VisitJsonValue( - value, - - [&](sj::object object) -> Status { - StartObject(); - - for (auto field : object) { - ARROW_ASSIGN_OR_RAISE(auto key, - ::arrow::internal::ResolveSimdjsonResult( - field.unescaped_key(), "Failed to get object key")); - - Key(key); - - ARROW_ASSIGN_OR_RAISE(auto field_value, - ::arrow::internal::ResolveSimdjsonResult( - field.value(), "Failed to get object value")); - - RETURN_NOT_OK(WriteValue(field_value)); - } - - EndObject(); - return Status::OK(); - }, - - [&](sj::array array) -> Status { - StartArray(); - - for (auto element : array) { - ARROW_ASSIGN_OR_RAISE(auto element_value, - ::arrow::internal::ResolveSimdjsonResult( - element, "Failed to iterate JSON array")); - - RETURN_NOT_OK(WriteValue(element_value)); - } - - EndArray(); - return Status::OK(); - }, - - [&](std::string_view string_value) -> Status { - String(string_value); - return Status::OK(); - }, - - [&](bool bool_value) -> Status { - Bool(bool_value); - return Status::OK(); - }, - - [&]() -> Status { - Null(); - return Status::OK(); - }, - - [&](int64_t value) -> Status { - Int64(value); - return Status::OK(); - }, - - [&](uint64_t value) -> Status { - Uint64(value); - return Status::OK(); - }, - - [&](double value) -> Status { - Double(value); - return Status::OK(); - }, - - [&](sj::value value) -> Status { - ARROW_ASSIGN_OR_RAISE(auto raw_json, ::arrow::internal::ResolveSimdjsonResult( - simdjson::to_json_string(value), - "Failed to get raw JSON")); - RawValue(raw_json); - return Status::OK(); - }); -} - -void JsonWriter::Null() { - MaybeComma(); - builder_.append_null(); - needs_comma_ = true; -} - -Result JsonWriter::GetString() const { - std::string_view view; - if (auto error = builder_.view().get(view); error != simdjson::SUCCESS) { - if (error == simdjson::OUT_OF_CAPACITY) { - return Status::OutOfMemory( - "OutOfMemory when allocating buffer to serialize json to string"); - } - return Status::Invalid("Failed to retrieve json from string builder: ", - simdjson::error_message(error)); - } - return view; -} - -Result JsonWriter::GetPrettyString( - const simdjson::fractured_json_options& options) const { - ARROW_ASSIGN_OR_RAISE(std::string_view json, GetString()); - return simdjson::fractured_json_string(json, options); -} - -void JsonWriter::Clear() { - builder_.clear(); - needs_comma_ = false; -} - -void JsonWriter::MaybeComma() { - if (needs_comma_) { - builder_.append_comma(); - } -} - -void JsonWriter::StringField(std::string_view key, std::string_view value) { - Key(key); - String(value); -} - -void JsonWriter::BoolField(std::string_view key, bool value) { - Key(key); - Bool(value); -} - -void JsonWriter::IntField(std::string_view key, int32_t value) { - Key(key); - Int(value); -} - -} // namespace arrow::json diff --git a/cpp/src/arrow/util/json_writer_internal.h b/cpp/src/arrow/util/json_writer_internal.h deleted file mode 100644 index e407695daa92..000000000000 --- a/cpp/src/arrow/util/json_writer_internal.h +++ /dev/null @@ -1,77 +0,0 @@ -// 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 - -#include "arrow/result.h" -#include "arrow/status.h" -#include "arrow/util/visibility.h" - -namespace arrow::json { - -class ARROW_EXPORT JsonWriter { - public: - JsonWriter() = default; - - void StartObject(); - void EndObject(); - - void StartArray(); - void EndArray(); - - void Key(std::string_view key); - - void String(std::string_view value); - void RawValue(std::string_view value); - void Bool(bool value); - - void Int(int32_t value); - void Int64(int64_t value); - - void Uint(uint32_t value); - void Uint64(uint64_t value); - - void Double(double value); - - Status WriteValue(simdjson::ondemand::value value); - - void Null(); - - void StringField(std::string_view key, std::string_view value); - void BoolField(std::string_view key, bool value); - void IntField(std::string_view key, int32_t value); - - Result GetString() const; - - Result GetPrettyString( - const simdjson::fractured_json_options& options = {}) const; - - void Clear(); - - private: - void MaybeComma(); - - simdjson::builder::string_builder builder_; - bool needs_comma_ = false; -}; - -} // namespace arrow::json diff --git a/cpp/src/arrow/util/meson.build b/cpp/src/arrow/util/meson.build index 456b8c61c7a3..729cfba47222 100644 --- a/cpp/src/arrow/util/meson.build +++ b/cpp/src/arrow/util/meson.build @@ -224,7 +224,7 @@ utility_test_srcs = [ utility_test_deps = [arrow_test_dep_no_main] if needs_simdjson - utility_test_srcs += ['json_writer_internal_test.cc'] + utility_test_srcs += ['simdjson_internal_test.cc'] utility_test_deps += [simdjson_dep] endif diff --git a/cpp/src/arrow/util/simdjson_internal.cc b/cpp/src/arrow/util/simdjson_internal.cc index 165b617768f3..a966a4ba1740 100644 --- a/cpp/src/arrow/util/simdjson_internal.cc +++ b/cpp/src/arrow/util/simdjson_internal.cc @@ -19,6 +19,365 @@ namespace arrow::internal { +class ObjectParser::Impl { + public: + Status Parse(std::string_view json) { + // Copy into padded buffer + padded_json_ = simdjson::padded_string(json); + + // Store parsed document + if (auto error = parser_.iterate(padded_json_).get(document_)) { + return Status::Invalid("JSON parse error: ", simdjson::error_message(error)); + } + + // Validate root is an object + auto object = document_.get_object(); + if (object.error()) { + if (object.error() == simdjson::INCORRECT_TYPE) { + return Status::TypeError("Not a JSON object"); + } + return Status::Invalid("JSON parse error: ", + simdjson::error_message(object.error())); + } + + return Status::OK(); + } + + Result GetString(const char* key) { + document_.rewind(); + + auto object = document_.get_object(); + + auto field = object.find_field(key); + + if (field.error() == simdjson::NO_SUCH_FIELD) { + return Status::KeyError("Key '", key, "' does not exist"); + } + if (field.error()) { + return Status::Invalid("Error accessing key '", key, + "': ", simdjson::error_message(field.error())); + } + + auto str_result = field.get_string(); + if (str_result.error() == simdjson::INCORRECT_TYPE) { + return Status::TypeError("Key '", key, "' is not a string"); + } + if (str_result.error()) { + return Status::Invalid("Error getting string for key '", key, + "': ", simdjson::error_message(str_result.error())); + } + + std::string_view str; + if (auto error = std::move(str_result).get(str)) { + return Status::Invalid("Error getting string for key '", key, + "': ", simdjson::error_message(error)); + } + return std::string(str); + } + + Result> GetStringMap() { + std::unordered_map map; + + document_.rewind(); + + auto object = document_.get_object(); + + for (auto field : object) { + std::string_view key; + if (auto error = field.unescaped_key().get(key)) { + return Status::Invalid("Error getting object key: ", + simdjson::error_message(error)); + } + + auto value = field.value(); + + auto str_result = value.get_string(); + + if (str_result.error() == simdjson::INCORRECT_TYPE) { + return Status::TypeError("Key '", std::string(key), + "' does not have a string value"); + } + if (str_result.error()) { + return Status::Invalid("Error getting value for key '", std::string(key), + "': (code=", static_cast(str_result.error()), ")"); + } + + std::string_view str; + if (auto error = std::move(str_result).get(str)) { + return Status::Invalid("Error getting value for key '", std::string(key), + "': ", simdjson::error_message(error)); + } + + map.emplace(std::string(key), std::string(str)); + } + + return map; + } + + Result GetBool(const char* key) { + document_.rewind(); + + auto object = document_.get_object(); + + auto field = object.find_field(key); + + if (field.error() == simdjson::NO_SUCH_FIELD) { + return Status::KeyError("Key '", key, "' does not exist"); + } + if (field.error()) { + return Status::Invalid("Error accessing key '", key, + "': ", simdjson::error_message(field.error())); + } + + auto bool_result = field.get_bool(); + if (bool_result.error() == simdjson::INCORRECT_TYPE) { + return Status::TypeError("Key '", key, "' is not a boolean"); + } + if (bool_result.error()) { + return Status::Invalid("Error getting bool for key '", key, + "': ", simdjson::error_message(bool_result.error())); + } + + bool value; + if (auto error = std::move(bool_result).get(value)) { + return Status::Invalid("Error getting bool for key '", key, + "': ", simdjson::error_message(error)); + } + + return value; + } + + private: + simdjson::ondemand::parser parser_; + simdjson::padded_string padded_json_; + simdjson::ondemand::document document_; +}; + +ObjectParser::ObjectParser() : impl_(new ObjectParser::Impl()) {} + +ObjectParser::~ObjectParser() = default; + +Status ObjectParser::Parse(std::string_view json) { return impl_->Parse(json); } + +Result ObjectParser::GetString(const char* key) const { + return impl_->GetString(key); +} + +Result ObjectParser::GetBool(const char* key) const { return impl_->GetBool(key); } + +Result> ObjectParser::GetStringMap() const { + return impl_->GetStringMap(); +} + +namespace sj = simdjson::ondemand; + +void JsonWriter::StartObject() { + MaybeComma(); + builder_.start_object(); + needs_comma_ = false; +} + +void JsonWriter::EndObject() { + builder_.end_object(); + needs_comma_ = true; +} + +void JsonWriter::StartArray() { + MaybeComma(); + builder_.start_array(); + needs_comma_ = false; +} + +void JsonWriter::EndArray() { + builder_.end_array(); + needs_comma_ = true; +} + +void JsonWriter::Key(std::string_view key) { + MaybeComma(); + builder_.escape_and_append_with_quotes(key); + builder_.append_colon(); + needs_comma_ = false; +} + +void JsonWriter::String(std::string_view value) { + MaybeComma(); + builder_.escape_and_append_with_quotes(value); + needs_comma_ = true; +} + +void JsonWriter::RawValue(std::string_view value) { + MaybeComma(); + builder_.append_raw(value); + needs_comma_ = true; +} + +void JsonWriter::Bool(bool value) { + MaybeComma(); + builder_.append(value); + needs_comma_ = true; +} + +void JsonWriter::Int(int32_t value) { + MaybeComma(); + builder_.append(value); + needs_comma_ = true; +} + +void JsonWriter::Int64(int64_t value) { + MaybeComma(); + builder_.append(value); + needs_comma_ = true; +} + +void JsonWriter::Uint(uint32_t value) { + MaybeComma(); + builder_.append(value); + needs_comma_ = true; +} + +void JsonWriter::Uint64(uint64_t value) { + MaybeComma(); + builder_.append(value); + needs_comma_ = true; +} + +void JsonWriter::Double(double value) { + MaybeComma(); + builder_.append(value); + needs_comma_ = true; +} + +Status JsonWriter::WriteValue(sj::value value) { + return VisitJsonValue( + value, + + [&](sj::object object) -> Status { + StartObject(); + + for (auto field : object) { + ARROW_ASSIGN_OR_RAISE( + auto key, + ResolveSimdjsonResult(field.unescaped_key(), "Failed to get object key")); + + Key(key); + + ARROW_ASSIGN_OR_RAISE( + auto field_value, + ResolveSimdjsonResult(field.value(), "Failed to get object value")); + + RETURN_NOT_OK(WriteValue(field_value)); + } + + EndObject(); + return Status::OK(); + }, + + [&](sj::array array) -> Status { + StartArray(); + + for (auto element : array) { + ARROW_ASSIGN_OR_RAISE( + auto element_value, + ResolveSimdjsonResult(element, "Failed to iterate JSON array")); + + RETURN_NOT_OK(WriteValue(element_value)); + } + + EndArray(); + return Status::OK(); + }, + + [&](std::string_view string_value) -> Status { + String(string_value); + return Status::OK(); + }, + + [&](bool bool_value) -> Status { + Bool(bool_value); + return Status::OK(); + }, + + [&]() -> Status { + Null(); + return Status::OK(); + }, + + [&](int64_t value) -> Status { + Int64(value); + return Status::OK(); + }, + + [&](uint64_t value) -> Status { + Uint64(value); + return Status::OK(); + }, + + [&](double value) -> Status { + Double(value); + return Status::OK(); + }, + + [&](sj::value value) -> Status { + ARROW_ASSIGN_OR_RAISE(auto raw_json, + ResolveSimdjsonResult(simdjson::to_json_string(value), + "Failed to get raw JSON")); + RawValue(raw_json); + return Status::OK(); + }); +} + +void JsonWriter::Null() { + MaybeComma(); + builder_.append_null(); + needs_comma_ = true; +} + +Result JsonWriter::GetString() const { + std::string_view view; + if (auto error = builder_.view().get(view); error != simdjson::SUCCESS) { + if (error == simdjson::OUT_OF_CAPACITY) { + return Status::OutOfMemory( + "OutOfMemory when allocating buffer to serialize json to string"); + } + return Status::Invalid("Failed to retrieve json from string builder: ", + simdjson::error_message(error)); + } + return view; +} + +Result JsonWriter::GetPrettyString( + const simdjson::fractured_json_options& options) const { + ARROW_ASSIGN_OR_RAISE(std::string_view json, GetString()); + return simdjson::fractured_json_string(json, options); +} + +void JsonWriter::Clear() { + builder_.clear(); + needs_comma_ = false; +} + +void JsonWriter::MaybeComma() { + if (needs_comma_) { + builder_.append_comma(); + } +} + +void JsonWriter::StringField(std::string_view key, std::string_view value) { + Key(key); + String(value); +} + +void JsonWriter::BoolField(std::string_view key, bool value) { + Key(key); + Bool(value); +} + +void JsonWriter::IntField(std::string_view key, int32_t value) { + Key(key); + Int(value); +} + const char* JsonTypeName(simdjson::dom::element_type type) { switch (type) { case simdjson::dom::element_type::ARRAY: diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index c507cd4be789..585557cb1fed 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -19,9 +19,11 @@ #include #include +#include #include #include #include +#include #include #include @@ -33,6 +35,73 @@ namespace arrow::internal { +/// This class is a helper to parse a JSON object from a string. +/// It uses simdjson in the implementation. +class ARROW_EXPORT ObjectParser { + public: + ObjectParser(); + ~ObjectParser(); + + Status Parse(std::string_view json); + + Result GetString(const char* key) const; + + Result GetBool(const char* key) const; + + // Get all members of the object as a map from string keys to string values + Result> GetStringMap() const; + + private: + class Impl; + std::unique_ptr impl_; +}; + +class ARROW_EXPORT JsonWriter { + public: + JsonWriter() = default; + + void StartObject(); + void EndObject(); + + void StartArray(); + void EndArray(); + + void Key(std::string_view key); + + void String(std::string_view value); + void RawValue(std::string_view value); + void Bool(bool value); + + void Int(int32_t value); + void Int64(int64_t value); + + void Uint(uint32_t value); + void Uint64(uint64_t value); + + void Double(double value); + + Status WriteValue(simdjson::ondemand::value value); + + void Null(); + + void StringField(std::string_view key, std::string_view value); + void BoolField(std::string_view key, bool value); + void IntField(std::string_view key, int32_t value); + + Result GetString() const; + + Result GetPrettyString( + const simdjson::fractured_json_options& options = {}) const; + + void Clear(); + + private: + void MaybeComma(); + + simdjson::builder::string_builder builder_; + bool needs_comma_ = false; +}; + // Empty struct to represent the type of a simdjson null value struct SimdjsonNull {}; diff --git a/cpp/src/arrow/util/json_writer_internal_test.cc b/cpp/src/arrow/util/simdjson_internal_test.cc similarity index 81% rename from cpp/src/arrow/util/json_writer_internal_test.cc rename to cpp/src/arrow/util/simdjson_internal_test.cc index 8d8e64528225..5891084db77c 100644 --- a/cpp/src/arrow/util/json_writer_internal_test.cc +++ b/cpp/src/arrow/util/simdjson_internal_test.cc @@ -15,14 +15,18 @@ // specific language governing permissions and limitations // under the License. +#include #include +#include +#include + #include "arrow/testing/gtest_util.h" -#include "arrow/util/json_writer_internal.h" +#include "arrow/util/simdjson_internal.h" namespace sj = simdjson::ondemand; -namespace arrow::json { +namespace arrow::internal { TEST(JsonWriter, SimpleObject) { JsonWriter writer; @@ -333,4 +337,83 @@ TEST(JsonWriter, GetPrettyString) { EXPECT_EQ(b_value, "hello"); } -} // namespace arrow::json +TEST(ObjectParser, GetString) { + ObjectParser parser; + + ASSERT_OK(parser.Parse(R"({"name":"arrow"})")); + + ASSERT_OK_AND_ASSIGN(auto value, parser.GetString("name")); + EXPECT_EQ(value, "arrow"); +} + +TEST(ObjectParser, GetBool) { + ObjectParser parser; + + ASSERT_OK(parser.Parse(R"({"enabled":true})")); + + ASSERT_OK_AND_ASSIGN(auto value, parser.GetBool("enabled")); + EXPECT_TRUE(value); +} + +TEST(ObjectParser, InvalidJson) { + ObjectParser parser; + + EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, ::testing::HasSubstr("JSON parse error"), + parser.Parse(R"({"name":)")); +} + +TEST(ObjectParser, GetStringMap) { + ObjectParser parser; + + ASSERT_OK(parser.Parse(R"({ + "k1": "v1", + "k2": "v2" + })")); + + ASSERT_OK_AND_ASSIGN(auto map, parser.GetStringMap()); + + ASSERT_EQ(map.size(), 2U); + EXPECT_EQ(map["k1"], "v1"); + EXPECT_EQ(map["k2"], "v2"); +} + +TEST(ObjectParser, MissingKey) { + ObjectParser parser; + + ASSERT_OK(parser.Parse(R"({ + "name": "arrow" + })")); + + ASSERT_RAISES(KeyError, parser.GetString("missing")); + ASSERT_RAISES(KeyError, parser.GetBool("missing")); +} + +TEST(ObjectParser, WrongType) { + ObjectParser parser; + + ASSERT_OK(parser.Parse(R"({ + "flag": true, + "name": "arrow" + })")); + + ASSERT_RAISES(TypeError, parser.GetString("flag")); + ASSERT_RAISES(TypeError, parser.GetBool("name")); +} + +TEST(ObjectParser, NonObjectRoot) { + ObjectParser parser; + + ASSERT_RAISES(TypeError, parser.Parse(R"(["a", "b"])")); +} + +TEST(ObjectParser, EmptyObject) { + ObjectParser parser; + + ASSERT_OK(parser.Parse(R"({})")); + + ASSERT_OK_AND_ASSIGN(auto map, parser.GetStringMap()); + + EXPECT_TRUE(map.empty()); +} + +} // namespace arrow::internal diff --git a/cpp/src/parquet/encryption/file_system_key_material_store.cc b/cpp/src/parquet/encryption/file_system_key_material_store.cc index 4fce13f0d8c6..62757bf67634 100644 --- a/cpp/src/parquet/encryption/file_system_key_material_store.cc +++ b/cpp/src/parquet/encryption/file_system_key_material_store.cc @@ -20,9 +20,8 @@ #include "arrow/buffer.h" #include "arrow/filesystem/filesystem.h" #include "arrow/filesystem/path_util.h" -#include "arrow/json/object_parser.h" #include "arrow/result.h" -#include "arrow/util/json_writer_internal.h" +#include "arrow/util/simdjson_internal.h" #include "parquet/encryption/file_system_key_material_store.h" #include "parquet/encryption/key_material.h" @@ -74,14 +73,14 @@ void FileSystemKeyMaterialStore::LoadKeyMaterialMap() { PARQUET_ASSIGN_OR_THROW(input_size, input->GetSize()); PARQUET_ASSIGN_OR_THROW(buff, input->ReadAt(0, input_size)); std::string buff_str = buff->ToString(); - ::arrow::json::internal::ObjectParser parser; + ::arrow::internal::ObjectParser parser; auto status = parser.Parse(buff_str); PARQUET_THROW_NOT_OK(status); PARQUET_ASSIGN_OR_THROW(key_material_map_, parser.GetStringMap()); } std::string FileSystemKeyMaterialStore::BuildKeyMaterialMapJson() { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); for (const auto& it : key_material_map_) { writer.StringField(it.first, it.second); diff --git a/cpp/src/parquet/encryption/key_material.cc b/cpp/src/parquet/encryption/key_material.cc index 827e1e445025..ae469a30e27b 100644 --- a/cpp/src/parquet/encryption/key_material.cc +++ b/cpp/src/parquet/encryption/key_material.cc @@ -17,15 +17,14 @@ #include -#include "arrow/json/object_parser.h" -#include "arrow/util/json_writer_internal.h" +#include "arrow/util/simdjson_internal.h" #include "parquet/encryption/key_material.h" #include "parquet/encryption/key_metadata.h" #include "parquet/exception.h" -using ::arrow::json::JsonWriter; -using ::arrow::json::internal::ObjectParser; +using ::arrow::internal::JsonWriter; +using ::arrow::internal::ObjectParser; namespace parquet::encryption { diff --git a/cpp/src/parquet/encryption/key_material.h b/cpp/src/parquet/encryption/key_material.h index 3e7e862c996d..b1154bbad25c 100644 --- a/cpp/src/parquet/encryption/key_material.h +++ b/cpp/src/parquet/encryption/key_material.h @@ -21,13 +21,9 @@ #include "parquet/platform.h" -namespace arrow { -namespace json { -namespace internal { +namespace arrow::internal { class ObjectParser; -} // namespace internal -} // namespace json -} // namespace arrow +} // namespace arrow::internal namespace parquet::encryption { @@ -86,8 +82,7 @@ class PARQUET_EXPORT KeyMaterial { static KeyMaterial Parse(const std::string& key_material_string); - static KeyMaterial Parse( - const ::arrow::json::internal::ObjectParser* key_material_json); + static KeyMaterial Parse(const ::arrow::internal::ObjectParser* key_material_json); /// This method returns a json string that will be stored either inside a parquet file /// or in a key material store outside the parquet file. diff --git a/cpp/src/parquet/encryption/key_metadata.cc b/cpp/src/parquet/encryption/key_metadata.cc index fd213169ff80..99e00c30d4ea 100644 --- a/cpp/src/parquet/encryption/key_metadata.cc +++ b/cpp/src/parquet/encryption/key_metadata.cc @@ -17,14 +17,13 @@ #include -#include "arrow/json/object_parser.h" -#include "arrow/util/json_writer_internal.h" +#include "arrow/util/simdjson_internal.h" #include "parquet/encryption/key_metadata.h" #include "parquet/exception.h" -using ::arrow::json::JsonWriter; -using ::arrow::json::internal::ObjectParser; +using ::arrow::internal::JsonWriter; +using ::arrow::internal::ObjectParser; namespace parquet::encryption { diff --git a/cpp/src/parquet/encryption/local_wrap_kms_client.cc b/cpp/src/parquet/encryption/local_wrap_kms_client.cc index c2d336de6815..66d1013bf10a 100644 --- a/cpp/src/parquet/encryption/local_wrap_kms_client.cc +++ b/cpp/src/parquet/encryption/local_wrap_kms_client.cc @@ -17,16 +17,15 @@ #include -#include "arrow/json/object_parser.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/secure_string.h" +#include "arrow/util/simdjson_internal.h" #include "parquet/encryption/key_toolkit_internal.h" #include "parquet/encryption/local_wrap_kms_client.h" #include "parquet/exception.h" -using ::arrow::json::JsonWriter; -using ::arrow::json::internal::ObjectParser; +using ::arrow::internal::JsonWriter; +using ::arrow::internal::ObjectParser; using ::arrow::util::SecureString; namespace parquet::encryption { diff --git a/cpp/src/parquet/geospatial/util_json_internal.cc b/cpp/src/parquet/geospatial/util_json_internal.cc index 9bdc82fe1d8e..6933d692795f 100644 --- a/cpp/src/parquet/geospatial/util_json_internal.cc +++ b/cpp/src/parquet/geospatial/util_json_internal.cc @@ -21,7 +21,6 @@ #include "arrow/extension_type.h" #include "arrow/result.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/simdjson_internal.h" #include "arrow/util/string.h" @@ -185,7 +184,7 @@ ::arrow::Result MakeGeoArrowCrsMetadata( } ::arrow::Result EscapeJsonString(std::string_view value) { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.String(value); ARROW_ASSIGN_OR_RAISE(auto escaped, writer.GetString()); diff --git a/cpp/src/parquet/printer.cc b/cpp/src/parquet/printer.cc index 053183c41b42..cf4decb32fd2 100644 --- a/cpp/src/parquet/printer.cc +++ b/cpp/src/parquet/printer.cc @@ -25,8 +25,8 @@ #include #include -#include "arrow/util/json_writer_internal.h" #include "arrow/util/key_value_metadata.h" +#include "arrow/util/simdjson_internal.h" #include "arrow/util/string.h" #include "parquet/column_scanner.h" @@ -256,7 +256,7 @@ void ParquetFilePrinter::DebugPrint(std::ostream& stream, std::list selecte void ParquetFilePrinter::JSONPrint(std::ostream& stream, std::list selected_columns, const char* filename) { const FileMetaData* file_metadata = fileReader->metadata().get(); - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("FileName", filename); writer.StringField("Version", ParquetVersionToString(file_metadata->version())); diff --git a/cpp/src/parquet/types.cc b/cpp/src/parquet/types.cc index ba80a27137a7..fda5e319e05b 100644 --- a/cpp/src/parquet/types.cc +++ b/cpp/src/parquet/types.cc @@ -28,8 +28,8 @@ #include "arrow/util/compression.h" #include "arrow/util/decimal.h" #include "arrow/util/float16.h" -#include "arrow/util/json_writer_internal.h" #include "arrow/util/logging_internal.h" +#include "arrow/util/simdjson_internal.h" #include "parquet/exception.h" #include "parquet/thrift_internal.h" @@ -717,7 +717,7 @@ class LogicalType::Impl { } virtual std::string ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", ToString()); writer.EndObject(); @@ -1174,7 +1174,7 @@ std::string LogicalType::Impl::Decimal::ToString() const { } std::string LogicalType::Impl::Decimal::ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", "Decimal"); writer.IntField("precision", precision_); @@ -1323,7 +1323,7 @@ std::string LogicalType::Impl::Time::ToString() const { } std::string LogicalType::Impl::Time::ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", "Time"); writer.BoolField("isAdjustedToUTC", adjusted_); @@ -1474,7 +1474,7 @@ std::string LogicalType::Impl::Timestamp::ToString() const { } std::string LogicalType::Impl::Timestamp::ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", "Timestamp"); writer.BoolField("isAdjustedToUTC", adjusted_); @@ -1668,7 +1668,7 @@ std::string LogicalType::Impl::Int::ToString() const { } std::string LogicalType::Impl::Int::ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", "Int"); writer.IntField("bitWidth", width_); @@ -1825,7 +1825,7 @@ std::string LogicalType::Impl::Geometry::ToString() const { } std::string LogicalType::Impl::Geometry::ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", "Geometry"); if (!crs_.empty()) { @@ -1916,7 +1916,7 @@ std::string LogicalType::Impl::Geography::ToString() const { } std::string LogicalType::Impl::Geography::ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", "Geography"); if (!crs_.empty()) { @@ -2008,7 +2008,7 @@ std::string LogicalType::Impl::Variant::ToString() const { } std::string LogicalType::Impl::Variant::ToJSON() const { - ::arrow::json::JsonWriter writer; + ::arrow::internal::JsonWriter writer; writer.StartObject(); writer.StringField("Type", "Variant"); writer.IntField("SpecVersion", static_cast(spec_version_)); From f46f189c7ffcd4b2fbf92ae491553f7d94b2af1c Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Mon, 31 Aug 2026 15:02:30 +0200 Subject: [PATCH 6/7] Rename ObjectParser to JsonObjectParser --- cpp/src/arrow/util/simdjson_internal.cc | 17 ++++++---- cpp/src/arrow/util/simdjson_internal.h | 6 ++-- cpp/src/arrow/util/simdjson_internal_test.cc | 32 +++++++++---------- .../file_system_key_material_store.cc | 2 +- cpp/src/parquet/encryption/key_material.cc | 6 ++-- cpp/src/parquet/encryption/key_material.h | 4 +-- cpp/src/parquet/encryption/key_metadata.cc | 4 +-- .../encryption/local_wrap_kms_client.cc | 4 +-- 8 files changed, 39 insertions(+), 36 deletions(-) diff --git a/cpp/src/arrow/util/simdjson_internal.cc b/cpp/src/arrow/util/simdjson_internal.cc index a966a4ba1740..146b48d9eb7f 100644 --- a/cpp/src/arrow/util/simdjson_internal.cc +++ b/cpp/src/arrow/util/simdjson_internal.cc @@ -19,7 +19,7 @@ namespace arrow::internal { -class ObjectParser::Impl { +class JsonObjectParser::Impl { public: Status Parse(std::string_view json) { // Copy into padded buffer @@ -153,19 +153,22 @@ class ObjectParser::Impl { simdjson::ondemand::document document_; }; -ObjectParser::ObjectParser() : impl_(new ObjectParser::Impl()) {} +JsonObjectParser::JsonObjectParser() : impl_(new JsonObjectParser::Impl()) {} -ObjectParser::~ObjectParser() = default; +JsonObjectParser::~JsonObjectParser() = default; -Status ObjectParser::Parse(std::string_view json) { return impl_->Parse(json); } +Status JsonObjectParser::Parse(std::string_view json) { return impl_->Parse(json); } -Result ObjectParser::GetString(const char* key) const { +Result JsonObjectParser::GetString(const char* key) const { return impl_->GetString(key); } -Result ObjectParser::GetBool(const char* key) const { return impl_->GetBool(key); } +Result JsonObjectParser::GetBool(const char* key) const { + return impl_->GetBool(key); +} -Result> ObjectParser::GetStringMap() const { +Result> JsonObjectParser::GetStringMap() + const { return impl_->GetStringMap(); } diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h index 585557cb1fed..5c52f7648c6a 100644 --- a/cpp/src/arrow/util/simdjson_internal.h +++ b/cpp/src/arrow/util/simdjson_internal.h @@ -37,10 +37,10 @@ namespace arrow::internal { /// This class is a helper to parse a JSON object from a string. /// It uses simdjson in the implementation. -class ARROW_EXPORT ObjectParser { +class ARROW_EXPORT JsonObjectParser { public: - ObjectParser(); - ~ObjectParser(); + JsonObjectParser(); + ~JsonObjectParser(); Status Parse(std::string_view json); diff --git a/cpp/src/arrow/util/simdjson_internal_test.cc b/cpp/src/arrow/util/simdjson_internal_test.cc index 5891084db77c..d376ee8ed68c 100644 --- a/cpp/src/arrow/util/simdjson_internal_test.cc +++ b/cpp/src/arrow/util/simdjson_internal_test.cc @@ -337,8 +337,8 @@ TEST(JsonWriter, GetPrettyString) { EXPECT_EQ(b_value, "hello"); } -TEST(ObjectParser, GetString) { - ObjectParser parser; +TEST(JsonObjectParser, GetString) { + JsonObjectParser parser; ASSERT_OK(parser.Parse(R"({"name":"arrow"})")); @@ -346,8 +346,8 @@ TEST(ObjectParser, GetString) { EXPECT_EQ(value, "arrow"); } -TEST(ObjectParser, GetBool) { - ObjectParser parser; +TEST(JsonObjectParser, GetBool) { + JsonObjectParser parser; ASSERT_OK(parser.Parse(R"({"enabled":true})")); @@ -355,15 +355,15 @@ TEST(ObjectParser, GetBool) { EXPECT_TRUE(value); } -TEST(ObjectParser, InvalidJson) { - ObjectParser parser; +TEST(JsonObjectParser, InvalidJson) { + JsonObjectParser parser; EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, ::testing::HasSubstr("JSON parse error"), parser.Parse(R"({"name":)")); } -TEST(ObjectParser, GetStringMap) { - ObjectParser parser; +TEST(JsonObjectParser, GetStringMap) { + JsonObjectParser parser; ASSERT_OK(parser.Parse(R"({ "k1": "v1", @@ -377,8 +377,8 @@ TEST(ObjectParser, GetStringMap) { EXPECT_EQ(map["k2"], "v2"); } -TEST(ObjectParser, MissingKey) { - ObjectParser parser; +TEST(JsonObjectParser, MissingKey) { + JsonObjectParser parser; ASSERT_OK(parser.Parse(R"({ "name": "arrow" @@ -388,8 +388,8 @@ TEST(ObjectParser, MissingKey) { ASSERT_RAISES(KeyError, parser.GetBool("missing")); } -TEST(ObjectParser, WrongType) { - ObjectParser parser; +TEST(JsonObjectParser, WrongType) { + JsonObjectParser parser; ASSERT_OK(parser.Parse(R"({ "flag": true, @@ -400,14 +400,14 @@ TEST(ObjectParser, WrongType) { ASSERT_RAISES(TypeError, parser.GetBool("name")); } -TEST(ObjectParser, NonObjectRoot) { - ObjectParser parser; +TEST(JsonObjectParser, NonObjectRoot) { + JsonObjectParser parser; ASSERT_RAISES(TypeError, parser.Parse(R"(["a", "b"])")); } -TEST(ObjectParser, EmptyObject) { - ObjectParser parser; +TEST(JsonObjectParser, EmptyObject) { + JsonObjectParser parser; ASSERT_OK(parser.Parse(R"({})")); diff --git a/cpp/src/parquet/encryption/file_system_key_material_store.cc b/cpp/src/parquet/encryption/file_system_key_material_store.cc index 62757bf67634..b0ba9c9e72fe 100644 --- a/cpp/src/parquet/encryption/file_system_key_material_store.cc +++ b/cpp/src/parquet/encryption/file_system_key_material_store.cc @@ -73,7 +73,7 @@ void FileSystemKeyMaterialStore::LoadKeyMaterialMap() { PARQUET_ASSIGN_OR_THROW(input_size, input->GetSize()); PARQUET_ASSIGN_OR_THROW(buff, input->ReadAt(0, input_size)); std::string buff_str = buff->ToString(); - ::arrow::internal::ObjectParser parser; + ::arrow::internal::JsonObjectParser parser; auto status = parser.Parse(buff_str); PARQUET_THROW_NOT_OK(status); PARQUET_ASSIGN_OR_THROW(key_material_map_, parser.GetStringMap()); diff --git a/cpp/src/parquet/encryption/key_material.cc b/cpp/src/parquet/encryption/key_material.cc index ae469a30e27b..1d29cc87fb8d 100644 --- a/cpp/src/parquet/encryption/key_material.cc +++ b/cpp/src/parquet/encryption/key_material.cc @@ -23,8 +23,8 @@ #include "parquet/encryption/key_metadata.h" #include "parquet/exception.h" +using ::arrow::internal::JsonObjectParser; using ::arrow::internal::JsonWriter; -using ::arrow::internal::ObjectParser; namespace parquet::encryption { @@ -59,7 +59,7 @@ KeyMaterial::KeyMaterial(bool is_footer_key, const std::string& kms_instance_id, encoded_wrapped_dek_(encoded_wrapped_dek) {} KeyMaterial KeyMaterial::Parse(const std::string& key_material_string) { - ObjectParser json_parser; + JsonObjectParser json_parser; ::arrow::Status status = json_parser.Parse(key_material_string); if (!status.ok()) { throw ParquetException("Failed to parse key material " + key_material_string); @@ -77,7 +77,7 @@ KeyMaterial KeyMaterial::Parse(const std::string& key_material_string) { return Parse(&json_parser); } -KeyMaterial KeyMaterial::Parse(const ObjectParser* key_material_json) { +KeyMaterial KeyMaterial::Parse(const JsonObjectParser* key_material_json) { // 2. Check if "key material" belongs to file footer key bool is_footer_key; PARQUET_ASSIGN_OR_THROW(is_footer_key, key_material_json->GetBool(kIsFooterKeyField)); diff --git a/cpp/src/parquet/encryption/key_material.h b/cpp/src/parquet/encryption/key_material.h index b1154bbad25c..8475c88b7510 100644 --- a/cpp/src/parquet/encryption/key_material.h +++ b/cpp/src/parquet/encryption/key_material.h @@ -22,7 +22,7 @@ #include "parquet/platform.h" namespace arrow::internal { -class ObjectParser; +class JsonObjectParser; } // namespace arrow::internal namespace parquet::encryption { @@ -82,7 +82,7 @@ class PARQUET_EXPORT KeyMaterial { static KeyMaterial Parse(const std::string& key_material_string); - static KeyMaterial Parse(const ::arrow::internal::ObjectParser* key_material_json); + static KeyMaterial Parse(const ::arrow::internal::JsonObjectParser* key_material_json); /// This method returns a json string that will be stored either inside a parquet file /// or in a key material store outside the parquet file. diff --git a/cpp/src/parquet/encryption/key_metadata.cc b/cpp/src/parquet/encryption/key_metadata.cc index 99e00c30d4ea..8fd097270996 100644 --- a/cpp/src/parquet/encryption/key_metadata.cc +++ b/cpp/src/parquet/encryption/key_metadata.cc @@ -22,8 +22,8 @@ #include "parquet/encryption/key_metadata.h" #include "parquet/exception.h" +using ::arrow::internal::JsonObjectParser; using ::arrow::internal::JsonWriter; -using ::arrow::internal::ObjectParser; namespace parquet::encryption { @@ -37,7 +37,7 @@ KeyMetadata::KeyMetadata(const KeyMaterial& key_material) : is_internal_storage_(true), key_material_or_reference_(key_material) {} KeyMetadata KeyMetadata::Parse(const std::string& key_metadata) { - ObjectParser json_parser; + JsonObjectParser json_parser; ::arrow::Status status = json_parser.Parse(key_metadata); if (!status.ok()) { throw ParquetException("Failed to parse key metadata " + key_metadata); diff --git a/cpp/src/parquet/encryption/local_wrap_kms_client.cc b/cpp/src/parquet/encryption/local_wrap_kms_client.cc index 66d1013bf10a..2628d9aa32ea 100644 --- a/cpp/src/parquet/encryption/local_wrap_kms_client.cc +++ b/cpp/src/parquet/encryption/local_wrap_kms_client.cc @@ -24,8 +24,8 @@ #include "parquet/encryption/local_wrap_kms_client.h" #include "parquet/exception.h" +using ::arrow::internal::JsonObjectParser; using ::arrow::internal::JsonWriter; -using ::arrow::internal::ObjectParser; using ::arrow::util::SecureString; namespace parquet::encryption { @@ -57,7 +57,7 @@ std::string LocalWrapKmsClient::LocalKeyWrap::CreateSerialized( LocalWrapKmsClient::LocalKeyWrap LocalWrapKmsClient::LocalKeyWrap::Parse( const std::string& wrapped_key) { - ObjectParser json_parser; + JsonObjectParser json_parser; auto status = json_parser.Parse(wrapped_key); if (!status.ok()) { throw ParquetException("Failed to parse local key wrap json " + wrapped_key); From d15007a673289f9ef2c37414ff9d15c8bc5b83c3 Mon Sep 17 00:00:00 2001 From: Rok Mihevc Date: Mon, 31 Aug 2026 15:41:14 +0200 Subject: [PATCH 7/7] Remove overload --- cpp/src/parquet/encryption/key_material.cc | 23 ++++++++-------------- cpp/src/parquet/encryption/key_material.h | 6 ------ cpp/src/parquet/encryption/key_metadata.cc | 3 +-- 3 files changed, 9 insertions(+), 23 deletions(-) diff --git a/cpp/src/parquet/encryption/key_material.cc b/cpp/src/parquet/encryption/key_material.cc index 1d29cc87fb8d..2aba3cd3e4e4 100644 --- a/cpp/src/parquet/encryption/key_material.cc +++ b/cpp/src/parquet/encryption/key_material.cc @@ -73,44 +73,37 @@ KeyMaterial KeyMaterial::Parse(const std::string& key_material_string) { throw ParquetException("Wrong key material type: " + key_material_type + " vs " + kKeyMaterialType1); } - // Parse other fields (common to internal and external key material) - return Parse(&json_parser); -} -KeyMaterial KeyMaterial::Parse(const JsonObjectParser* key_material_json) { // 2. Check if "key material" belongs to file footer key bool is_footer_key; - PARQUET_ASSIGN_OR_THROW(is_footer_key, key_material_json->GetBool(kIsFooterKeyField)); + PARQUET_ASSIGN_OR_THROW(is_footer_key, json_parser.GetBool(kIsFooterKeyField)); std::string kms_instance_id; std::string kms_instance_url; if (is_footer_key) { // 3. For footer key, extract KMS Instance ID - PARQUET_ASSIGN_OR_THROW(kms_instance_id, - key_material_json->GetString(kKmsInstanceIdField)); + PARQUET_ASSIGN_OR_THROW(kms_instance_id, json_parser.GetString(kKmsInstanceIdField)); // 4. For footer key, extract KMS Instance URL PARQUET_ASSIGN_OR_THROW(kms_instance_url, - key_material_json->GetString(kKmsInstanceUrlField)); + json_parser.GetString(kKmsInstanceUrlField)); } // 5. Extract master key ID std::string master_key_id; - PARQUET_ASSIGN_OR_THROW(master_key_id, key_material_json->GetString(kMasterKeyIdField)); + PARQUET_ASSIGN_OR_THROW(master_key_id, json_parser.GetString(kMasterKeyIdField)); // 6. Extract wrapped DEK std::string encoded_wrapped_dek; PARQUET_ASSIGN_OR_THROW(encoded_wrapped_dek, - key_material_json->GetString(kWrappedDataEncryptionKeyField)); + json_parser.GetString(kWrappedDataEncryptionKeyField)); std::string kek_id; std::string encoded_wrapped_kek; // 7. Check if "key material" was generated in double wrapping mode bool is_double_wrapped; - PARQUET_ASSIGN_OR_THROW(is_double_wrapped, - key_material_json->GetBool(kDoubleWrappingField)); + PARQUET_ASSIGN_OR_THROW(is_double_wrapped, json_parser.GetBool(kDoubleWrappingField)); if (is_double_wrapped) { // 8. In double wrapping mode, extract KEK ID - PARQUET_ASSIGN_OR_THROW(kek_id, - key_material_json->GetString(kKeyEncryptionKeyIdField)); + PARQUET_ASSIGN_OR_THROW(kek_id, json_parser.GetString(kKeyEncryptionKeyIdField)); // 9. In double wrapping mode, extract wrapped KEK PARQUET_ASSIGN_OR_THROW(encoded_wrapped_kek, - key_material_json->GetString(kWrappedKeyEncryptionKeyField)); + json_parser.GetString(kWrappedKeyEncryptionKeyField)); } return KeyMaterial(is_footer_key, kms_instance_id, kms_instance_url, master_key_id, diff --git a/cpp/src/parquet/encryption/key_material.h b/cpp/src/parquet/encryption/key_material.h index 8475c88b7510..2ce5ec857824 100644 --- a/cpp/src/parquet/encryption/key_material.h +++ b/cpp/src/parquet/encryption/key_material.h @@ -21,10 +21,6 @@ #include "parquet/platform.h" -namespace arrow::internal { -class JsonObjectParser; -} // namespace arrow::internal - namespace parquet::encryption { // KeyMaterial class represents the "key material", keeping the information that allows @@ -82,8 +78,6 @@ class PARQUET_EXPORT KeyMaterial { static KeyMaterial Parse(const std::string& key_material_string); - static KeyMaterial Parse(const ::arrow::internal::JsonObjectParser* key_material_json); - /// This method returns a json string that will be stored either inside a parquet file /// or in a key material store outside the parquet file. static std::string SerializeToJson(bool is_footer_key, diff --git a/cpp/src/parquet/encryption/key_metadata.cc b/cpp/src/parquet/encryption/key_metadata.cc index 8fd097270996..65b67bc33b8c 100644 --- a/cpp/src/parquet/encryption/key_metadata.cc +++ b/cpp/src/parquet/encryption/key_metadata.cc @@ -60,8 +60,7 @@ KeyMetadata KeyMetadata::Parse(const std::string& key_metadata) { if (is_internal_storage) { // 3.1 "key material" is stored internally, inside "key metadata" - parse it - KeyMaterial key_material = KeyMaterial::Parse(&json_parser); - return KeyMetadata(key_material); + return KeyMetadata(KeyMaterial::Parse(key_metadata)); } else { // 3.2 "key material" is stored externally. "key metadata" keeps a reference to it std::string key_reference;