Skip to content

GH-50627: [C++] Migrate from_string.cc to simdjson#50644

Open
KHARSHAVARDHAN-eng wants to merge 1 commit into
apache:mainfrom
KHARSHAVARDHAN-eng:gh-50627-migrate-from-string-to-simdjson
Open

GH-50627: [C++] Migrate from_string.cc to simdjson#50644
KHARSHAVARDHAN-eng wants to merge 1 commit into
apache:mainfrom
KHARSHAVARDHAN-eng:gh-50627-migrate-from-string-to-simdjson

Conversation

@KHARSHAVARDHAN-eng

@KHARSHAVARDHAN-eng KHARSHAVARDHAN-eng commented Jul 26, 2026

Copy link
Copy Markdown

Rationale for this change

This PR continues the migration from RapidJSON to simdjson as part of GH-35460 by migrating the JSON parsing implementation in cpp/src/arrow/json/from_string.cc to use simdjson::dom.

What changes are included?

  • Replace RapidJSON parsing with simdjson::dom::parser and simdjson::dom::element.
  • Remove the remaining RapidJSON dependencies from from_string.cc.
  • Update the JSON conversion pipeline to operate on simdjson::dom::element.
  • Parse standard JSON directly using simdjson::dom.
  • Introduce a fallback sanitization step for legacy non-standard inputs (such as unquoted NaN/Inf and raw non-UTF-8 binary strings) to preserve the behavior exercised by the existing from_string test suite.

Testing

Verified with:

ninja -C cpp/build_bundled arrow-json-test
cpp/build_bundled/release/arrow-json-test

Output:

[==========] 244 tests from 61 test suites ran.
[  PASSED  ] 244 tests.

User-facing changes

None.

This change does not modify the public API or user-visible behavior.

Closes #50627.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #50627 has been automatically assigned in GitHub to PR creator.

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.

Before I review this PR.. I think this change belongs to another PR, could you verify and remove this file.

@Reranko05 Reranko05 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.

Can you run python -m pre_commit run clang-format --files cpp/src/arrow/json/from_string.cc to fix the formatting issues?

@Reranko05 Reranko05 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.

I noticed this migration uses the simdjson::dom API. In my earlier migration work on ObjectParser, I was suggested to use the ondemand API instead of dom. Should we use ondemand here as well, or is there a reason dom is preferred in this case? @kou, what do you think?

@KHARSHAVARDHAN-eng
KHARSHAVARDHAN-eng force-pushed the gh-50627-migrate-from-string-to-simdjson branch from e9ec2e6 to d4ed946 Compare July 26, 2026 12:52
@KHARSHAVARDHAN-eng

Copy link
Copy Markdown
Author

Thanks for pointing this out!

The branch originally started from another feature branch, which accidentally included an unrelated commit. I've rebased the branch onto main, removed the unrelated commit, and force-pushed the updated branch. The PR now contains only the changes for GH-50627.

@KHARSHAVARDHAN-eng

Copy link
Copy Markdown
Author

Done, thanks!

I've run the project's clang-format pre-commit hook, applied the formatting changes, and force-pushed the updated branch.

@Reranko05

Copy link
Copy Markdown
Contributor

Thanks! Let's wait for @kou to weigh in on the ondemand vs dom API discussion before proceeding.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++] Migrate to simdjson in from_string.cc and tests

2 participants