Skip to content

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

Open
Krishnanand-G wants to merge 1 commit into
apache:mainfrom
Krishnanand-G:GH-50859-json-writer-parquet
Open

GH-50859: [C++] Build JsonWriter when Parquet is enabled without JSON#50900
Krishnanand-G wants to merge 1 commit into
apache:mainfrom
Krishnanand-G:GH-50859-json-writer-parquet

Conversation

@Krishnanand-G

Copy link
Copy Markdown

Rationale for this change

#50859: with ARROW_JSON=OFF, parquet tools fail to link:

undefined reference to arrow::json::JsonWriter::GetString() const

json_writer_internal.cc was only compiled into the ARROW_JSON object library. Parquet still calls JsonWriter from types.cc, encryption, and geospatial helpers. ARROW_PARQUET already pulls in simdjson, so the writer can live outside the JSON feature flag.

What changes are included in this PR?

Move json/json_writer_internal.cc into a small ARROW_JSON_WRITER object library, built when ARROW_JSON or ARROW_PARQUET is on, and link it into libarrow.

Are these changes tested?

I did not run a full ARROW_JSON=OFF + Parquet extra build on this machine. The failure is a missing object in libarrow; Arrow CI (including jobs that set ARROW_JSON=OFF) is the check I am relying on.

Are there any user-facing changes?

No.

…t JSON

json_writer_internal.cc lived in the ARROW_JSON object library, so
ARROW_JSON=OFF left parquet tools with undefined JsonWriter symbols.

Signed-off-by: Krishnanand G <118352827+Krishnanand-G@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

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

@Reranko05 Reranko05 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM for the approach. However, the CI that originally caught this issue was external to Arrow. Could you please build locally with ARROW_JSON=OFF and ARROW_PARQUET=ON to verify the fix?

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.

2 participants