GH-50724: [C++] Add `JsonWriter::WriteValue` for simdjson values by Reranko05 · Pull Request #50725 · apache/arrow

Reranko05 · 2026-07-30T00:54:31Z

Rationale for this change

This PR continues the simdjson migration by adding support for serializing simdjson::ondemand::value directly with JsonWriter.

This provides a reusable API for future migration work and avoids requiring callers to implement their own recursive serialization logic. It also introduces a shared helper for dispatching simdjson::ondemand::value based on its JSON type, reducing duplicated type dispatch and extraction logic.

What changes are included in this PR?

Add JsonWriter::WriteValue(simdjson::ondemand::value).
Add VisitJsonValue to centralize JSON type dispatch and simdjson value extraction.
Recursively serialize:
- objects
- arrays
- strings
- booleans
- null values
- numeric values
Add unit tests covering:
- simple objects
- nested objects
- objects containing arrays
- complex nested values
- empty objects
Use simdjson::ondemand::document::get_value() in tests to obtain the root ondemand::value before serialization.

Are these changes tested?

Yes.

Added unit tests for JsonWriter::WriteValue covering the supported JSON value types and nested structures.

Are there any user-facing changes?

No.

Closes: #50724

GitHub Issue: [C++] Add JsonWriter::WriteValue for simdjson values #50724

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

kou · 2026-07-30T05:54:24Z

Could you rebase on main?

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Reranko05 · 2026-07-30T12:55:49Z

Rebased on main.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Reranko05 · 2026-07-30T14:57:27Z

@pitrou @kou @rok Could you review this when you have time?

pitrou · 2026-07-30T15:40:17Z

Can you explain in which concrete situation(s) it would be useful?

Reranko05 · 2026-07-30T15:47:06Z

@pitrou Some of the remaining code needs to convert a simdjson::ondemand::value back into a JSON string. For example, util_json_internal.cc currently uses RapidJSON's Accept(writer) for this. Since simdjson doesn't provide an equivalent function, JsonWriter::WriteValue() handles that conversion in one place instead of each caller having to implement it themselves.

::arrow::Result<std::string> GeospatialGeoArrowCrsToParquetCrs(
    const ::arrow::rapidjson::Document& document) {
  namespace rj = ::arrow::rapidjson;

  if (!document.HasMember("crs") || document["crs"].IsNull()) {
    // 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
    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") {
          return "";
        } else if (identifier["authority"] == "EPSG" && identifier["code"].IsInt() &&
                   identifier["code"].GetInt() == 4326) {
          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)
  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();
  }
}

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

taepper

Looks very good, a couple minor comments!

taepper · 2026-07-31T15:11:29Z

+        return Status::OK();
+      },
+
+      [&](sj::value number_value) -> Status {


Shouldn't this be a sj::number instead? Then we should be left with no results within the lambda body?

I kept sj::value because we still need get_number_type() to detect big integers, and simdjson::to_json_string() also requires the original value. Passing sj::number would move part of the number handling into VisitJsonValue. Does that seem like a reasonable tradeoff to you?

Ah, that is a rather annoying problem, I see. As this will be the only user of the library-function, we cannot see how good a solution will fit more general usages.

I see two options:

Provide another BigIntegerFn to the visitor

Move the switch on number_type also to the visitor and provide all four call-backs {DoubleFn,Int64Fn,Uint64Fn,BigIntegerFn}, dropping the argument NumberFn, instead. This would correspond to the number_type enum in simdjson.

enum class number_type { floating_point_number=1, /// a binary64 number signed_integer, /// a signed integer that fits in a 64-bit word using two's complement unsigned_integer, /// a positive integer larger or equal to 1<<63 big_integer /// a big integer that does not fit in a 64-bit word };

If all options of number_type could fit into the number-struct's representation this would not be a problem, but I guess this has been done because implementations that want to error on big_integer would need to pay a performance cost otherwise.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Reranko05 requested a review from pitrou as a code owner July 30, 2026 00:54

Copilot AI review requested due to automatic review settings July 30, 2026 00:54

Copilot AI reviewed Jul 30, 2026

github-actions Bot added Component: C++ awaiting review Awaiting review labels Jul 30, 2026

Copilot AI review requested due to automatic review settings July 30, 2026 12:55

Reranko05 force-pushed the gh-35460-jsonwriter-value branch from c9a1540 to bf9e22e Compare July 30, 2026 12:55

Copilot AI reviewed Jul 30, 2026

Reranko05 force-pushed the gh-35460-jsonwriter-value branch from bf9e22e to eb33737 Compare July 30, 2026 13:02

Copilot AI review requested due to automatic review settings July 30, 2026 13:02

Copilot AI reviewed Jul 30, 2026

Add JsonWriter::WriteValue for simdjson values

ad6f313

Reranko05 force-pushed the gh-35460-jsonwriter-value branch from eb33737 to ad6f313 Compare July 30, 2026 13:22

Copilot AI review requested due to automatic review settings July 30, 2026 13:22

Copilot AI reviewed Jul 30, 2026

Copilot AI review requested due to automatic review settings July 30, 2026 14:20

Copilot AI reviewed Jul 30, 2026

pitrou added the CI: Extra: C++ Run extra C++ CI label Jul 30, 2026

pitrou reviewed Jul 30, 2026

View reviewed changes

Comment thread cpp/src/arrow/json/json_writer_internal.cc Outdated

pitrou reviewed Jul 30, 2026

View reviewed changes

Comment thread cpp/src/arrow/json/json_writer_internal.cc Outdated

github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jul 30, 2026

taepper reviewed Jul 30, 2026

View reviewed changes

Comment thread cpp/src/arrow/json/json_writer_internal_test.cc Outdated

Comment thread cpp/src/arrow/json/json_writer_internal.cc Outdated

Comment thread cpp/src/arrow/json/json_writer_internal.cc Outdated

Reranko05 mentioned this pull request Jul 30, 2026

[C++] Extract reusable simdjson helper utilities #50746

Closed

Use document::get_value() in JsonWriter tests

042951d

Copilot AI review requested due to automatic review settings July 30, 2026 16:34

Reranko05 force-pushed the gh-35460-jsonwriter-value branch from da5440b to 042951d Compare July 30, 2026 16:34

Copilot AI reviewed Jul 30, 2026

Handle big integers in JsonWriter::WriteValue

4b38499

Copilot AI review requested due to automatic review settings July 30, 2026 17:54

Copilot AI reviewed Jul 30, 2026

Reranko05 added 2 commits July 31, 2026 18:53

Extract simdjson result helper

ae2437f

Add simdjson value visitor helper

00d156b

Copilot AI review requested due to automatic review settings July 31, 2026 14:19

Copilot AI reviewed Jul 31, 2026

Reranko05 requested review from pitrou and taepper July 31, 2026 14:31

taepper reviewed Jul 31, 2026

View reviewed changes

Move simdjson helper functions

d1f3a16

Copilot AI review requested due to automatic review settings July 31, 2026 16:07

Copilot AI reviewed Jul 31, 2026

Uh oh!

Conversation

Reranko05 commented Jul 30, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

kou commented Jul 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reranko05 commented Jul 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reranko05 commented Jul 30, 2026

Uh oh!

pitrou commented Jul 30, 2026

Uh oh!

Reranko05 commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

taepper left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

taepper Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reranko05 Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

taepper Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Reranko05 commented Jul 30, 2026 •

edited by github-actions Bot

Loading

Reranko05 commented Jul 30, 2026 •

edited

Loading

taepper left a comment •

edited

Loading

taepper Jul 31, 2026 •

edited

Loading