GH-50859: [C++][Parquet] Move JsonWriter to simdjson utilities by rok · Pull Request #50990 · apache/arrow

rok · 2026-08-25T15:20:34Z

Rationale for this change

Parquet uses JsonWriter when ARROW_JSON=OFF, but its implementation was only built with Arrow JSON, causing link failures.

What changes are included in this PR?

Move JsonWriter to the simdjson utilities and update its callers and CMake/Meson builds.

Are these changes tested?

Yes. CMake shared/static and Meson Parquet builds pass with JSON disabled. Unit tests and pre-commit checks also pass.

Are there any user-facing changes?

No. This only fixes the affected build configuration.

AI disclosure - this was AI generated to test alternative approach to #50900.

GitHub Issue: [C++] parquet tools: linking error when ARROW_JSON=OFF with undefined reference to `arrow::json::JsonWriter::GetString() const' #50859

pitrou · 2026-08-25T16:06:57Z

The CI failures need fixing.

rok · 2026-08-25T16:55:01Z

@pitrou done.

Reranko05

@rok Since simdjson_internal.cc now exists, shouldn't all the non-template functions currently in simdjson_internal.h be moved into simdjson_internal.cc, leaving only declarations in the header?

rok · 2026-08-25T17:55:38Z

@rok Since simdjson_internal.cc now exists, shouldn't all the non-template functions currently in simdjson_internal.h be moved into simdjson_internal.cc, leaving only declarations in the header?

Done

Reranko05

LGTM!

rok · 2026-08-25T18:01:45Z

Again, this PR is purely AI generated. I will mark as ready for review once I review myself.

pitrou · 2026-08-27T12:27:37Z

@rok Are you willing to prioritize this?

rok · 2026-08-27T12:58:20Z

@pitrou I'll review this in about an hour and ping again

tadeja · 2026-08-27T13:35:19Z

@github-actions crossbow submit example-cpp-tutorial

github-actions · 2026-08-27T13:37:45Z

Revision: 973b4d2

Submitted crossbow builds: ursacomputing/crossbow @ actions-5765e8701c

Task	Status
example-cpp-tutorial

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.

rok · 2026-08-27T15:25:59Z

+                 SOURCES
+                 json_writer_internal_test.cc
+                 EXTRA_LINK_LIBS
+                 simdjson::simdjson)


Should we use arrow::simdjson here instead? Both should work, but arrow alieas would hide the vendored/system simdjson.

rok · 2026-08-27T15:36:47Z

@github-actions crossbow submit example-cpp-tutorial

github-actions · 2026-08-27T15:40:36Z

Revision: 86e85bd

Submitted crossbow builds: ursacomputing/crossbow @ actions-e4578321bd

Task	Status
example-cpp-tutorial

rok · 2026-08-27T15:53:43Z

@github-actions crossbow submit test-ubuntu-24.04-cpp-minimal-with-formats test-ubuntu-24.04-cpp-gcc-13-bundled test-ubuntu-24.04-cpp

Copilot

Pull request overview

Fixes Parquet (and other components) linking failures when ARROW_JSON=OFF by relocating arrow::json::JsonWriter and related simdjson helpers into Arrow util code that is built whenever simdjson support is enabled.

Changes:

Update Parquet and other call sites to include arrow/util/json_writer_internal.h instead of the JSON module header.
Move/enable building of JsonWriter and simdjson utility implementations outside the ARROW_JSON feature gate (CMake + Meson).
Refactor simdjson_internal.h to use exported (non-inline) function declarations with a new simdjson_internal.cc implementation, and relocate/adjust unit tests accordingly.

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
cpp/src/parquet/types.cc	Switch JsonWriter include to util header to avoid JSON-gated dependency.
cpp/src/parquet/printer.cc	Switch JsonWriter include to util header.
cpp/src/parquet/geospatial/util_json_internal.cc	Switch JsonWriter include to util header.
cpp/src/parquet/encryption/local_wrap_kms_client.cc	Switch JsonWriter include to util header.
cpp/src/parquet/encryption/key_metadata.cc	Switch JsonWriter include to util header.
cpp/src/parquet/encryption/key_material.cc	Switch JsonWriter include to util header.
cpp/src/parquet/encryption/file_system_key_material_store.cc	Switch JsonWriter include to util header.
cpp/src/arrow/util/simdjson_internal.h	Move simdjson helpers from inline definitions to exported declarations and adjust namespace style.
cpp/src/arrow/util/simdjson_internal.cc	New translation unit providing implementations for simdjson helpers previously inlined.
cpp/src/arrow/util/meson.build	Add JsonWriter internal test to util test suite when simdjson is needed; adjust deps.
cpp/src/arrow/util/json_writer_internal.h	New util header location for JsonWriter API used by Parquet when JSON is disabled.
cpp/src/arrow/util/json_writer_internal.cc	Update implementation to include the new util header path.
cpp/src/arrow/util/json_writer_internal_test.cc	Update test include to new util header path.
cpp/src/arrow/util/CMakeLists.txt	Add a dedicated JsonWriter internal test target when simdjson is enabled.
cpp/src/arrow/meson.build	Ensure simdjson dependency and util simdjson/json-writer sources are built when needed (incl. Parquet).
cpp/src/arrow/json/meson.build	Remove JsonWriter internal test from JSON test executable.
cpp/src/arrow/json/CMakeLists.txt	Remove JsonWriter internal test from JSON test target.
cpp/src/arrow/integration/json_internal.cc	Switch JsonWriter include to util header.
cpp/src/arrow/integration/json_integration.cc	Switch JsonWriter include to util header.
cpp/src/arrow/integration/json_integration_test.cc	Switch JsonWriter include to util header.
cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc	Switch JsonWriter include to util header.
cpp/src/arrow/extension/variable_shape_tensor.cc	Switch JsonWriter include to util header.
cpp/src/arrow/extension/opaque.cc	Switch JsonWriter include to util header.
cpp/src/arrow/extension/fixed_shape_tensor.cc	Switch JsonWriter include to util header.
cpp/src/arrow/dataset/file_json_test.cc	Switch JsonWriter include to util header.
cpp/src/arrow/CMakeLists.txt	Build and link JsonWriter + simdjson utility sources when simdjson is enabled; remove JSON-gated build of JsonWriter.
cpp/meson.build	Introduce `needs_simdjson` (driven by JSON or Parquet) to control simdjson-dependent compilation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-08-27T15:56:19Z

Revision: 86e85bd

Submitted crossbow builds: ursacomputing/crossbow @ actions-b71eaadb5c

Task	Status
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-minimal-with-formats

rok · 2026-08-27T16:17:34Z

@pitrou this is ready for humans!

rok · 2026-08-27T17:17:04Z

@github-actions crossbow submit test-ubuntu-24.04-cpp-minimal-with-formats test-ubuntu-24.04-cpp-gcc-13-bundled test-ubuntu-24.04-cpp example-cpp-tutorial

Copilot

Pull request overview

Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.

 #include "arrow/json/object_parser.h"
+#include "arrow/util/json_writer_internal.h"
 #include "arrow/util/secure_string.h"


github-actions · 2026-08-27T17:19:30Z

Revision: 4d93dff

Submitted crossbow builds: ursacomputing/crossbow @ actions-fca50b3ffc

Task	Status
example-cpp-tutorial
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-minimal-with-formats

rok · 2026-08-27T17:22:59Z

#50912 switched flight ODBC from RapidJSON to simdjson JsonWriter, but did not enable ARROW_WITH_SIMDJSON for ODBC-only builds. This PR now (4d93dff) makes JsonWriter available independently of ARROW_JSON, so it's possible to build ARROW_FLIGHT_SQL_ODBC=ON with ARROW_JSON=OFF.

github-actions Bot added Component: Parquet Component: C++ Component: FlightRPC awaiting committer review Awaiting committer review labels Aug 25, 2026

rok mentioned this pull request Aug 25, 2026

GH-50859: [C++] Build JsonWriter when Parquet is enabled without JSON #50900

Open

Reranko05 reviewed Aug 25, 2026

View reviewed changes

Reranko05 approved these changes Aug 25, 2026

View reviewed changes

pitrou mentioned this pull request Aug 27, 2026

GH-51013: [C++] Replace RapidJSON in JSON test utilities #51014

Open

rok force-pushed the gh-50859-parquet-without-json branch from 973b4d2 to 5a1f424 Compare August 27, 2026 14:51

rok commented Aug 27, 2026

View reviewed changes

github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Aug 27, 2026

simplify meson change

86e85bd

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 27, 2026

rok marked this pull request as ready for review August 27, 2026 15:50

rok requested review from lidavidm, pitrou and wgtmac as code owners August 27, 2026 15:50

Copilot AI lite review requested due to automatic review settings August 27, 2026 15:50

Copilot started reviewing on behalf of rok August 27, 2026 15:51 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

rok added the CI: Extra: C++ Run extra C++ CI label Aug 27, 2026

set ARROW_JSON for odbc build

4d93dff

Copilot AI review requested due to automatic review settings August 27, 2026 17:14

Copilot started reviewing on behalf of rok August 27, 2026 17:14 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Comment thread cpp/src/parquet/encryption/local_wrap_kms_client.cc

Comment on lines 20 to 22

#include "arrow/json/object_parser.h"

#include "arrow/util/json_writer_internal.h"

#include "arrow/util/secure_string.h"

Uh oh!

Conversation

rok commented Aug 25, 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!

pitrou commented Aug 25, 2026

Uh oh!

rok commented Aug 25, 2026

Uh oh!

Reranko05 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!

rok commented Aug 25, 2026

Uh oh!

Reranko05 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!

rok commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pitrou commented Aug 27, 2026

Uh oh!

rok commented Aug 27, 2026

Uh oh!

tadeja commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

rok Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

rok commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

rok commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

rok commented Aug 27, 2026

Uh oh!

rok commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented Aug 27, 2026

Uh oh!

rok commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rok commented Aug 25, 2026 •

edited by github-actions Bot

Loading

Reranko05 left a comment •

edited

Loading

Reranko05 left a comment •

edited

Loading

rok commented Aug 25, 2026 •

edited

Loading