Skip to content

GH-51252: [C++] Reduce boilerplate in OpaqueType JSON deserialization - #51253

Merged
pitrou merged 1 commit into
apache:mainfrom
Reranko05:reduce-opaque-clean
Sep 9, 2026
Merged

GH-51252: [C++] Reduce boilerplate in OpaqueType JSON deserialization#51253
pitrou merged 1 commit into
apache:mainfrom
Reranko05:reduce-opaque-clean

Conversation

@Reranko05

@Reranko05 Reranko05 commented Sep 9, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Collaborator

Rationale for this change

This revisits the implementation based on the review feedback from #50905 (comment) and uses Arrow's existing JsonObjectParser helper to provide a higher-level interface over simdjson.

Changes

  • Replace manual simdjson On-Demand parsing in OpaqueType::Deserialize() with JsonObjectParser.
  • Use JsonObjectParser::GetString() to retrieve the required type_name and vendor_name fields.
  • Remove manual JSON object iteration, key handling, and explicit JSON type checks.
  • Update OpaqueType::Deserialize tests to verify invalid input is rejected without relying on exact error messages.

Fixes: #51252

@github-actions github-actions Bot added the awaiting review Awaiting review label Sep 9, 2026
@Reranko05
Reranko05 marked this pull request as ready for review September 9, 2026 04:41
@Reranko05
Reranko05 requested a review from pitrou as a code owner September 9, 2026 04:41
Copilot AI lite review requested due to automatic review settings September 9, 2026 04:41
Copilot AI reviewed Sep 9, 2026

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 commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

@pitrou Ready for Review (CI failures are unrelated)

pitrou
pitrou approved these changes Sep 9, 2026
View reviewed changes

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A neat simplification, thank you @Reranko05 !

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 9, 2026
@pitrou
pitrou merged commit 2d69292 into apache:main Sep 9, 2026
73 of 94 checks passed
@pitrou pitrou removed the awaiting committer review Awaiting committer review label Sep 9, 2026
@pitrou pitrou mentioned this pull request Sep 9, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@pitrou pitrou pitrou approved these changes

Assignees

No one assigned

Labels

CI: Extra: C++ Run extra C++ CI Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C++] Reduce boilerplate in OpaqueType JSON deserialization

3 participants

@Reranko05 @pitrou