GH-50724: [C++] Add JsonWriter::WriteValue for simdjson values

[Reranko05]

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]

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]

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]

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

[pitrou]

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

[Reranko05]

@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]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

cpp/src/arrow/util/simdjson_internal.h:156

• Some error prefixes passed to GetSimdjsonResult are missing the usual ": " suffix (e.g., "Failed to get signed integer"). Since GetSimdjsonResult appends simdjson::error_message(...), the resulting message gets concatenated without a delimiter.

          ARROW_ASSIGN_OR_RAISE(
              auto number,
              GetSimdjsonResult(value.get_int64(), "Failed to get signed integer"));
          return int64_fn(number);
        }

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

cpp/src/arrow/util/simdjson_internal.h:26

• simdjson_internal.h uses int64_t/uint64_t, std::is_same_v, and std::move but doesn’t include the corresponding standard headers (<cstdint>, <type_traits>, <utility>). Relying on transitive includes can break builds depending on include order; make this header self-contained.

#include <string_view>

#include <simdjson.h>

#include "arrow/result.h"
#include "arrow/status.h"

cpp/src/arrow/json/json_writer_internal_test.cc:189

• JsonWriter::GetString() returns Result<std::string_view>, but these new tests compare the Result directly to a string literal (e.g. EXPECT_EQ(writer.GetString(), ...)), which won’t compile. Please unwrap the result with ASSERT_OK_AND_ASSIGN (or ASSERT_OK_AND_ASSIGN(auto json, ...)) in all the added WriteValue* tests (same pattern appears multiple times below).

  JsonWriter writer;
  ASSERT_OK(writer.WriteValue(value));

  EXPECT_EQ(writer.GetString(), R"({"a":42,"b":"hello"})");

cpp/src/arrow/util/simdjson_internal.h:156

• The error strings passed to GetSimdjsonResult(...) in the number-type cases are missing the trailing ": ", so the resulting Status::Invalid(error, ...) message will read awkwardly (e.g. "Failed to get signed integerThe error message"). Make these consistent with the other error prefixes in this helper.

        case simdjson::ondemand::number_type::signed_integer: {
          ARROW_ASSIGN_OR_RAISE(
              auto number,
              GetSimdjsonResult(value.get_int64(), "Failed to get signed integer"));
          return int64_fn(number);

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

cpp/src/arrow/util/simdjson_internal.h:228

• GetJsonAs reports "Expected or null" on type errors, but (except for SimdjsonNull) the implementation does not accept null—value.get(typed_value) will fail on null. This makes the error message misleading (e.g., "Expected boolean or null" when null is actually rejected).

  if (error_code != simdjson::SUCCESS) {
    simdjson::ondemand::json_type json_type;
    if (value.type().get(json_type) != simdjson::SUCCESS) {
      return Status::Invalid("Expected ", JsonTypeName<SimdjsonValueType>(),
                             " or null, got malformed JSON value");
    }
    return Status::Invalid("Expected ", JsonTypeName<SimdjsonValueType>(),
                           " or null, got JSON type ", JsonTypeName(json_type));

[Reranko05]

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

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

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

[kou]

+1

[Copilot]

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