Skip to content

GH-51037: [C++] Replace RapidJSON with simdjson in JSON parser - #51038

Merged
pitrou merged 23 commits into
apache:mainfrom
Reranko05:gh-35460-parser
Oct 6, 2026
Merged

pitrou merged 23 commits into
apache:mainfrom
Reranko05:gh-35460-parser

Conversation

@Reranko05

@Reranko05 Reranko05 commented Aug 29, 2026 •

Copy link
Copy Markdown
Collaborator

Rationale for this change

This PR continues the simdjson migration by replacing the RapidJSON-based parsing implementation used by the JSON parser.

The existing parser uses RapidJSON's SAX/handler interface to parse JSON values and populate Arrow builders. This change replaces that implementation with simdjson's ondemand API while retaining the existing builder and type-inference logic.

Changes

  • Replace the RapidJSON parser and handler interface with simdjson's ondemand API.
  • Parse JSON documents using simdjson::ondemand::parser::iterate_many.
  • Use ResolveSimdjsonResult() consistently when handling simdjson results.
  • Preserve support for nested objects and arrays.
  • Preserve explicit-schema and inferred-field behavior.
  • Preserve unexpected-field handling for Error, Ignore, and InferType.
  • Continue storing numeric values as raw JSON tokens.
  • Trim trailing whitespace from numeric raw tokens to preserve existing behavior.
  • Preserve JSON parse error propagation through Status::Invalid.
  • Remove the parser's RapidJSON-specific dependencies.

Are there any user-facing changes?

No

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Fixes: #51037

@Reranko05 Reranko05 added the CI: Extra: C++ Run extra C++ CI label Aug 29, 2026
@Reranko05
Reranko05 force-pushed the gh-35460-parser branch 3 times, most recently from d02c90a to d9c6d74 Compare August 30, 2026 05:57
@Reranko05
Reranko05 marked this pull request as ready for review August 30, 2026 06:34
Copilot AI lite review requested due to automatic review settings August 30, 2026 06:34
@Reranko05
Reranko05 requested review from pitrou and rok as code owners August 30, 2026 06:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@Reranko05
Reranko05 marked this pull request as draft September 4, 2026 11:31
@Reranko05

Reranko05 commented Sep 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

@pitrou @kou Should we use padded_string_view with a reusable buffer here as well, similar to the chunker?

@Reranko05

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit -g cpp

@github-actions

This comment was marked as outdated.

@pitrou

pitrou commented Sep 7, 2026

Copy link
Copy Markdown
Member

@pitrou @kou Should we use padded_string_view with a reusable buffer here as well, similar to the chunker?

We should check the Buffer capacity first to see if it has enough padding already.

@Reranko05

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit -g cpp

@github-actions

This comment was marked as outdated.

@Reranko05
Reranko05 marked this pull request as ready for review September 8, 2026 14:41
Copilot AI review requested due to automatic review settings September 8, 2026 14:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@Reranko05

Copy link
Copy Markdown
Collaborator Author

@pitrou @rok I’ve rebased the parser PR and incorporated the latest feedback, including reusing the simdjson::ondemand::parser and using padded_string_view when the Arrow Buffer has sufficient capacity.

The native C++ build and tests pass, but Crossbow’s ubuntu-cpp-emscripten job now fails in arrow-dataset-file-json-test:

RuntimeError: Aborted(). Build with -sASSERTIONS for more info.
    at abort (/build/cpp/debug/arrow-dataset-file-json-test.js:491:11)
    at _abort (/build/cpp/debug/arrow-dataset-file-json-test.js:4483:7)
    at invoke_v (/build/cpp/debug/arrow-dataset-file-json-test.js:5907:29)
    at arrow-dataset-file-json-test.wasm.std::__terminate(void (*)()) (wasm://wasm/arrow-dataset-file-json-test.wasm-0f1ca1f2:wasm-function[41574]:0x277e6cb)
    at arrow-dataset-file-json-test.wasm.std::terminate() (wasm://wasm/arrow-dataset-file-json-test.wasm-0f1ca1f2:wasm-function[41572]:0x277e6a4)
    at arrow-dataset-file-json-test.wasm.simdjson::fallback::ondemand::document_stream::start() (wasm://wasm/arrow-dataset-file-json-test.wasm-0f1ca1f2:wasm-function[20092]:0x10168d0)
    at arrow-dataset-file-json-test.wasm.arrow::Status arrow::Status arrow::json::HandlerBase::DoParse<arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>>(arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>&, std::__2::shared_ptr<arrow::Buffer> const&)::'lambda'(arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2> const&)::operator()<simdjson::padded_string>(arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2> const&) const (wasm://wasm/arrow-dataset-file-json-test.wasm-0f1ca1f2:wasm-function[20180]:0x1030c99)
    at invoke_viii (/build/cpp/debug/arrow-dataset-file-json-test.js:5885:29)
    at arrow-dataset-file-json-test.wasm.arrow::Status arrow::json::HandlerBase::DoParse<arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>>(arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>&, std::__2::shared_ptr<arrow::Buffer> const&) (wasm://wasm/arrow-dataset-file-json-test.wasm-0f1ca1f2:wasm-function[20178]:0x102f33c)
    at arrow-dataset-file-json-test.wasm.arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>::Parse(std::__2::shared_ptr<arrow::Buffer> const&) (wasm://wasm/arrow-dataset-file-json-test.wasm-0f1ca1f2:wasm-function[20177]:0x102ef89)

Do you have any idea what I need to do to fix this?

@pitrou

pitrou commented Sep 8, 2026

Copy link
Copy Markdown
Member

@Reranko05 No idea without taking a deeper look :-) But I'd like us to merge the chunker PR first and then come back to this one.

@Reranko05

Copy link
Copy Markdown
Collaborator Author

@pitrou Okay, will wait until chunker is merged.

Copilot AI review requested due to automatic review settings September 16, 2026 08:14
Copilot AI lite review requested due to automatic review settings October 6, 2026 09:09
@Reranko05

Copy link
Copy Markdown
Collaborator Author

parquet-reader-test failed in Crossbow CI which was fixed in #51661. Rebased to main.

@Reranko05

Copy link
Copy Markdown
Collaborator Author

@github-actions crossbow submit -g cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Resolve the critical result-handling issue and the two moderate compatibility/build issues.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (2)

Comment thread cpp/src/arrow/json/parser.cc
Comment thread cpp/src/arrow/json/parser.cc
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Revision: 5aa2590

Submitted crossbow builds: ursacomputing/crossbow @ actions-4c97a81f0f

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-debian-13-cpp-amd64 GitHub Actions
test-debian-13-cpp-i386 GitHub Actions
test-debian-experimental-cpp-gcc-15 GitHub Actions
test-fedora-42-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@taepper taepper left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If dropping support for NaN, Infinity and -Infinity is acceptable for now, then I think this is almost good to go

I noticed the following bug

Comment thread cpp/src/arrow/json/parser.cc
Copilot AI lite review requested due to automatic review settings October 6, 2026 11:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Critical parser regressions and unresolved dependency and test issues block approval.

Review effort: Lite
Findings: 3 High severity

Open (3)
Resolved since last review (2)

Comment thread cpp/src/arrow/json/parser.cc
Comment thread cpp/src/arrow/json/parser.cc Outdated
Comment thread cpp/src/arrow/json/parser.cc
Copilot AI lite review requested due to automatic review settings October 6, 2026 11:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Address top-level null handling, raw numeric token compatibility, and stale RapidJSON build dependencies.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity get_number rejects raw numeric tokens previously preserved

cpp/​src/​arrow/​json/​parser.cc:870

Calling get_number() here is not compatible with the parser's raw-token contract. The old RapidJSON path used kParseNumbersAsStringsFlag (and kParseNanAndInfFlag), so it retained arbitrary integer text—including values outside 64-bit ranges—and non-finite tokens for later string/decimal/float conversion; simdjson reports BIGINT_ERROR for out-of-range integers and its typed numeric accessors reject some non-finite tokens. As a result, valid inputs that previously worked for string or decimal columns now fail before RawNumber can store them. Validate the token without narrowing it, or explicitly special-case these token classes.

Comment thread cpp/src/arrow/json/parser.cc
@pitrou
pitrou merged commit bbbc06f into apache:main Oct 6, 2026
72 of 73 checks passed
@pitrou pitrou removed the awaiting change review Awaiting change review label Oct 6, 2026
@pitrou

pitrou commented Oct 6, 2026

Copy link
Copy Markdown
Member

Thank you for this @Reranko05 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++] Replace RapidJSON with simdjson in JSON parser

7 participants