Skip to content

GH-50779: [C++][Parquet] Replace remaining RapidJSON usage with simdjson #50781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Reranko05 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

GH-50779: [C++][Parquet] Replace remaining RapidJSON usage with simdjson #50781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0230946
Replace remaining RapidJSON usage with simdjson
Reranko05 Aug 3, 2026
777999e
Preserve GeoArrow CRS JSON formatting
Reranko05 Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cpp/src/parquet/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ if(ARROW_TESTING)
# "link" our dependencies so that include paths are configured
# correctly
target_link_libraries(parquet_testing PUBLIC ${ARROW_GTEST_GMOCK})
list(APPEND PARQUET_TEST_LINK_LIBS parquet_testing RapidJSON)
list(APPEND PARQUET_TEST_LINK_LIBS parquet_testing simdjson::simdjson)
endif()

if(NOT ARROW_BUILD_SHARED)
Expand Down
173 changes: 122 additions & 51 deletions cpp/src/parquet/geospatial/util_json_internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,52 +17,93 @@

#include "parquet/geospatial/util_json_internal.h"

#include <simdjson.h>
#include <string>

#include "arrow/extension_type.h"
#include "arrow/json/rapidjson_defs.h" // IWYU pragma: keep
#include "arrow/json/json_writer_internal.h"
#include "arrow/result.h"
#include "arrow/util/simdjson_internal.h"
#include "arrow/util/string.h"

#include <rapidjson/document.h>
#include <rapidjson/writer.h>

#include "parquet/exception.h"
#include "parquet/types.h"

namespace parquet {

namespace {
::arrow::Result<std::string> GeospatialGeoArrowCrsToParquetCrs(
const ::arrow::rapidjson::Document& document) {
namespace rj = ::arrow::rapidjson;
simdjson::ondemand::object object) {
auto crs_field = object["crs"];

if (!document.HasMember("crs") || document["crs"].IsNull()) {
if (crs_field.error() == simdjson::NO_SUCH_FIELD) {
// Parquet GEOMETRY/GEOGRAPHY do not have a concept of a null/missing
// CRS, but an omitted one is more likely to have meant "lon/lat" than
// a truly unspecified one (i.e., Engineering CRS with arbitrary XY units)
return "";
}

const auto& json_crs = document["crs"];
if (json_crs.IsString() && (json_crs == "EPSG:4326" || json_crs == "OGC:CRS84")) {
// crs can be left empty because these cases both correspond to
// longitude/latitude in WGS84 according to the Parquet specification
ARROW_ASSIGN_OR_RAISE(auto json_crs, ::arrow::internal::GetSimdjsonResult(
crs_field, "Failed to get 'crs' field: "));

ARROW_ASSIGN_OR_RAISE(bool is_null, ::arrow::internal::IsJsonNull(json_crs));
if (is_null) {
return "";
} else if (json_crs.IsObject()) {
// Attempt to detect common PROJJSON representations of longitude/latitude and return
// an empty crs to maximize compatibility with readers that do not implement CRS
// support. PROJJSON stores this in the "id" member like:
// {..., "id": {"authority": "...", "code": "..."}}
if (json_crs.HasMember("id")) {
const auto& identifier = json_crs["id"];
if (identifier.HasMember("authority") && identifier.HasMember("code")) {
if (identifier["authority"] == "OGC" && identifier["code"] == "CRS84") {
return "";
} else if (identifier["authority"] == "EPSG" && identifier["code"] == "4326") {
}

if (auto string = ::arrow::internal::GetJsonAs<std::string_view>(json_crs);
string.ok()) {
if (*string == "EPSG:4326" || *string == "OGC:CRS84") {
Comment on lines +54 to +56

@kou kou Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid if (...; ...) for readability?

Suggested change
if (auto string = ::arrow::internal::GetJsonAs<std::string_view>(json_crs);
string.ok()) {
if (*string == "EPSG:4326" || *string == "OGC:CRS84") {
auto crs_string_result = ::arrow::internal::GetJsonAs<std::string_view>(json_crs);
if (crs_string_result.ok()) {
auto crs_string = *crs_string_result;
if (crs_string == "EPSG:4326" || crs_string == "OGC:CRS84") {

// crs can be left empty because these cases both correspond to
// longitude/latitude in WGS84 according to the Parquet specification
return "";
}

// If we could not detect a longitude/latitude CRS, just write the string to the
// LogicalType crs (being sure to unescape a JSON string into a regular string)
return std::string(*string);
}

ARROW_ASSIGN_OR_RAISE(
auto crs_object,
::arrow::internal::GetJsonAs<simdjson::ondemand::object>(json_crs));

// Attempt to detect common PROJJSON representations of longitude/latitude and return
// an empty crs to maximize compatibility with readers that do not implement CRS
// support. PROJJSON stores this in the "id" member like:
// {..., "id": {"authority": "...", "code": "..."}}
auto id_field = crs_object["id"];

if (id_field.error() != simdjson::NO_SUCH_FIELD) {
ARROW_ASSIGN_OR_RAISE(auto identifier, ::arrow::internal::GetSimdjsonResult(
id_field, "Failed to get 'id' field: "));

auto authority_field = identifier["authority"];
auto code_field = identifier["code"];

if (authority_field.error() != simdjson::NO_SUCH_FIELD &&
code_field.error() != simdjson::NO_SUCH_FIELD) {
ARROW_ASSIGN_OR_RAISE(auto authority,
::arrow::internal::GetSimdjsonResult(
authority_field, "Failed to get 'authority' field: "));

ARROW_ASSIGN_OR_RAISE(auto code, ::arrow::internal::GetSimdjsonResult(
code_field, "Failed to get 'code' field: "));

ARROW_ASSIGN_OR_RAISE(auto authority_string,
::arrow::internal::GetJsonAs<std::string_view>(authority));

auto code_string = ::arrow::internal::GetJsonAs<std::string_view>(code);

if (code_string.ok()) {
if ((authority_string == "OGC" && *code_string == "CRS84") ||
(authority_string == "EPSG" && *code_string == "4326")) {
Comment on lines +96 to +100

@kou kou Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using _result suffix for arrow::Result variable for readability?

Suggested change
auto code_string = ::arrow::internal::GetJsonAs<std::string_view>(code);
if (code_string.ok()) {
if ((authority_string == "OGC" && *code_string == "CRS84") ||
(authority_string == "EPSG" && *code_string == "4326")) {
auto code_string_result = ::arrow::internal::GetJsonAs<std::string_view>(code);
if (code_string_result.ok()) {
auto code_string = *code_string_result;
if ((authority_string == "OGC" && code_string == "CRS84") ||
(authority_string == "EPSG" && code_string == "4326")) {

return "";
} else if (identifier["authority"] == "EPSG" && identifier["code"].IsInt() &&
identifier["code"].GetInt() == 4326) {
}
} else if (authority_string == "EPSG") {
auto code_int = ::arrow::internal::GetJsonAs<int64_t>(code);

if (code_int.ok() && *code_int == 4326) {
return "";
}
}
Expand All @@ -71,14 +112,26 @@ ::arrow::Result<std::string> GeospatialGeoArrowCrsToParquetCrs(

// If we could not detect a longitude/latitude CRS, just write the string to the
// LogicalType crs (being sure to unescape a JSON string into a regular string)
if (json_crs.IsString()) {
return json_crs.GetString();
} else {
rj::StringBuffer buffer;
rj::Writer<rj::StringBuffer> writer(buffer);
json_crs.Accept(writer);
return buffer.GetString();
RETURN_NOT_OK(::arrow::internal::GetSimdjsonResult(crs_object.reset(),
"Failed to reset 'crs' object: ")

@kou kou Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that all GetSimdjsonResult() messages have : suffix. How about adding it automatically?

diff --git a/cpp/src/arrow/util/simdjson_internal.h b/cpp/src/arrow/util/simdjson_internal.h
index 8ffb741da4..187dd3aaa5 100644
--- a/cpp/src/arrow/util/simdjson_internal.h
+++ b/cpp/src/arrow/util/simdjson_internal.h
@@ -85,7 +85,7 @@ template <typename T>
 Result<T> GetSimdjsonResult(simdjson::simdjson_result<T> result, std::string_view error) {
   T value;
   if (auto error_code = std::move(result).get(value); error_code != simdjson::SUCCESS) {
-    return Status::Invalid(error, simdjson::error_message(error_code));
+    return Status::Invalid(error, ": ", simdjson::error_message(error_code));
   }
   return value;
 }

(BTW, GetSimdjsonResult() name may be a bit strange because it doesn't return simdjson::result<T>. It gets arrow::Result<T> from simdjson::result<T>. ResolveSimdjsonResult(), SimdjsonResultToArrow() or something may be better.)

.status());

ARROW_ASSIGN_OR_RAISE(auto raw_crs,
::arrow::internal::GetSimdjsonResult(
crs_object.raw_json(), "Failed to get raw 'crs' JSON: "));

std::string minified(raw_crs.size(), '\0');
size_t minified_len = 0;

if (auto error =
simdjson::minify(raw_crs.data(), raw_crs.size(), minified.data(), minified_len);
error != simdjson::SUCCESS) {
return ::arrow::Status::Invalid("Failed to minify CRS JSON: ",
simdjson::error_message(error));
}

minified.resize(minified_len);
return minified;
}

// Utility for ensuring that a Parquet CRS is valid JSON when written to
Expand Down Expand Up @@ -125,18 +178,18 @@ ::arrow::Result<std::string> MakeGeoArrowCrsMetadata(
}

std::string EscapeCrsAsJsonIfRequired(std::string_view crs) {
namespace rj = ::arrow::rapidjson;
rj::Document document;
if (document.Parse(crs.data(), crs.length()).HasParseError()) {
rj::StringBuffer buffer;
rj::Writer<rj::StringBuffer> writer(buffer);
rj::Value v;
v.SetString(crs.data(), static_cast<int32_t>(crs.size()));
v.Accept(writer);
return std::string(buffer.GetString());
} else {
return std::string(crs);
simdjson::ondemand::parser parser;
simdjson::padded_string json(crs);

if (parser.iterate(json).error() != simdjson::SUCCESS) {
::arrow::json::JsonWriter writer;
writer.String(crs);

auto escaped = writer.GetString().ValueUnsafe();

@kou kou Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the return type to arrow::Result<std::string> from std::string to propagate an error?

return std::string(escaped);
}

return std::string(crs);
}

} // namespace
Expand All @@ -149,24 +202,42 @@ ::arrow::Result<std::shared_ptr<const LogicalType>> LogicalTypeFromGeoArrowMetad
return LogicalType::Geometry();
}

namespace rj = ::arrow::rapidjson;
rj::Document document;
if (document.Parse(serialized_data.data(), serialized_data.length()).HasParseError()) {
simdjson::ondemand::parser parser;
simdjson::padded_string json(serialized_data);

simdjson::ondemand::document document;
if (auto error = parser.iterate(json).get(document); error != simdjson::SUCCESS) {
return ::arrow::Status::Invalid("Invalid serialized JSON data: ", serialized_data);
}

ARROW_ASSIGN_OR_RAISE(std::string crs, GeospatialGeoArrowCrsToParquetCrs(document));
ARROW_ASSIGN_OR_RAISE(
auto object, ::arrow::internal::GetSimdjsonResult(document.get_object(),
"Failed to get JSON object: "));

if (document.HasMember("edges") && document["edges"] == "planar") {
ARROW_ASSIGN_OR_RAISE(std::string crs, GeospatialGeoArrowCrsToParquetCrs(object));

auto edges_field = object["edges"];

if (edges_field.error() == simdjson::NO_SUCH_FIELD) {
return LogicalType::Geometry(crs);
} else if (document.HasMember("edges") && document["edges"] == "spherical") {
}

ARROW_ASSIGN_OR_RAISE(auto edges, ::arrow::internal::GetSimdjsonResult(
edges_field, "Failed to get 'edges' field: "));

ARROW_ASSIGN_OR_RAISE(auto edges_string,
::arrow::internal::GetJsonAs<std::string_view>(edges));

if (edges_string == "planar") {
return LogicalType::Geometry(crs);
}

if (edges_string == "spherical") {
return LogicalType::Geography(crs,
LogicalType::EdgeInterpolationAlgorithm::SPHERICAL);
} else if (document.HasMember("edges")) {
return ::arrow::Status::Invalid("Unsupported GeoArrow edge type: ", serialized_data);
}

return LogicalType::Geometry(crs);
return ::arrow::Status::Invalid("Unsupported GeoArrow edge type: ", serialized_data);
}

::arrow::Result<std::shared_ptr<::arrow::DataType>> GeoArrowTypeFromLogicalType(
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/parquet/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ if not thrift_dep.found()
thrift_dep = thrift_proj.dependency('thrift')
endif

parquet_deps = [arrow_dep, rapidjson_dep, thrift_dep]
parquet_deps = [arrow_dep, thrift_dep]

if needs_parquet_encryption or get_option('parquet_require_encryption').auto()
openssl_dep = dependency('openssl', required: needs_parquet_encryption)
Expand Down
24 changes: 9 additions & 15 deletions cpp/src/parquet/reader_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>

#include "arrow/json/rapidjson_defs.h" // IWYU pragma: keep

#include <rapidjson/document.h>
#include <rapidjson/error/en.h>
#include <rapidjson/stringbuffer.h>

#include "arrow/array.h"
#include "arrow/array/array_binary.h"
#include "arrow/array/builder_binary.h"
Expand All @@ -44,6 +38,7 @@
#include "arrow/util/checked_cast.h"
#include "arrow/util/config.h"
#include "arrow/util/range.h"
#include "arrow/util/simdjson_internal.h"

#include "parquet/column_reader.h"
#include "parquet/column_scanner.h"
Expand All @@ -59,8 +54,6 @@
#include "parquet/test_util.h"
#include "parquet/types.h"

namespace rj = arrow::rapidjson;

using arrow::internal::checked_pointer_cast;
using arrow::internal::Zip;

Expand Down Expand Up @@ -1230,14 +1223,15 @@ TEST_F(TestJSONWithLocalFile, JSONOutputSortColumns) {
namespace {

::arrow::Status CheckJsonValid(std::string_view json_string) {
rj::Document json_doc;
constexpr auto kParseFlags = rj::kParseFullPrecisionFlag | rj::kParseNanAndInfFlag;
json_doc.Parse<kParseFlags>(json_string.data(), json_string.length());
if (json_doc.HasParseError()) {
return ::arrow::Status::Invalid("JSON parse error at offset ",
json_doc.GetErrorOffset(), ": ",
rj::GetParseError_En(json_doc.GetParseError()));
simdjson::ondemand::parser parser;
simdjson::ondemand::document document;

auto padded_json = simdjson::padded_string(json_string);

if (auto error = parser.iterate(padded_json).get(document)) {
return ::arrow::Status::Invalid("JSON parse error: ", simdjson::error_message(error));
}

return ::arrow::Status::OK();
}

Expand Down
6 changes: 3 additions & 3 deletions cpp/src/parquet/schema_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1581,9 +1581,9 @@ TEST(TestLogicalTypeOperation, LogicalTypeRepresentation) {
{LogicalType::Geometry(R"(crs with "quotes" and \backslashes\)"),
R"(Geometry(crs=crs with "quotes" and \backslashes\))",
R"({"Type": "Geometry", "crs": "crs with \"quotes\" and \\backslashes\\"})"},
{LogicalType::Geometry("crs with control characters \u0001 and \u001F"),
"Geometry(crs=crs with control characters \u0001 and \u001F)",
R"({"Type": "Geometry", "crs": "crs with control characters \u0001 and \u001F"})"},
{LogicalType::Geometry("crs with control characters \u0001 and \u001f"),
"Geometry(crs=crs with control characters \u0001 and \u001f)",
R"({"Type": "Geometry", "crs": "crs with control characters \u0001 and \u001f"})"},
{LogicalType::Geography(), "Geography(crs=, algorithm=spherical)",
R"({"Type": "Geography"})"},
{LogicalType::Geography("srid:1234",
Expand Down
15 changes: 4 additions & 11 deletions cpp/src/parquet/types.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,13 @@
#include <sstream>
#include <string>

#include "arrow/json/rapidjson_defs.h" // IWYU pragma: keep
#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/logging_internal.h"

#include <rapidjson/document.h>
#include <rapidjson/writer.h>

#include "parquet/exception.h"
#include "parquet/thrift_internal.h"
#include "parquet/types.h"
Expand Down Expand Up @@ -1785,13 +1782,9 @@ namespace {
void WriteCrsKeyAndValue(const std::string_view crs, std::ostream& json) {
// There is no restriction on the crs value here, and it may contain quotes
// or backslashes that would result in invalid JSON if unescaped.
namespace rj = ::arrow::rapidjson;
rj::StringBuffer buffer;
rj::Writer<rj::StringBuffer> writer(buffer);
rj::Value v;
v.SetString(crs.data(), static_cast<int32_t>(crs.size()));
v.Accept(writer);
json << R"(, "crs": )" << buffer.GetString();
::arrow::json::JsonWriter writer;
writer.String(crs);
json << R"(, "crs": )" << writer.GetString().ValueUnsafe();

@kou kou Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use JsonWriter for all JSON build instead of mixing manual JSON build and JsonWriter build?

}
} // namespace

Expand Down
Loading