GH-33209: [C++] Support for reading JSON Datasets #33732

benibus · 2023-01-17T23:17:26Z

This adds initial support the JSON file format to the Dataset library. Since there's currently no public API for writing JSON files, this only deals with the reader-side facilities.

Closes: [C++] Bind the JSON RecordBatchReader to Dataset #33209

github-actions · 2023-01-17T23:17:50Z

Closes: [C++] Bind the JSON RecordBatchReader to Dataset #33209

github-actions · 2023-01-17T23:17:53Z

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

benibus · 2023-02-07T23:25:00Z

I attempted to implement the newer ScanNode methods. However, I ran into some issues that have prevented feature parity with the legacy API. Primarily, the methods that use ScanOptions are able to prevent loading top-level fields if none of its children are expected to be materialized. However, I haven't figured out a good way to do this using FragmentScanRequest since it only provides FieldPaths (relative to the inspected fragment).

I suppose there's a possibility that the tests in FileFormatScanNodeMixin are expecting too much of the format implementation, though. From what I can tell, CSV is the only other format that implements the new API and even then it doesn't support nested fields - so the relevant tests in that fixture haven't been used anywhere before.

benibus · 2023-02-07T23:32:20Z

For reference, here is where that test would fail (in FileFormatScanNodeMixin::TestScanProjectedNested), whereas the equivalent test in FileFormatScanMixin currently passes.

westonpace

This is exciting. Sorry you took this on at a time when we have two mirror paths in the file format. Hopefully this will all be getting simplified soon.

cpp/src/arrow/dataset/file_json_test.cc

cpp/src/arrow/dataset/file_json.cc

westonpace

A few minor nits from a tidy/iwyu pass but mostly clean, thank you.

cpp/src/arrow/dataset/CMakeLists.txt

cpp/src/arrow/dataset/file_json.cc

cpp/src/arrow/dataset/file_json.h

cpp/src/arrow/dataset/file_json.cc

westonpace · 2023-02-10T18:15:28Z

cpp/src/arrow/dataset/file_json_test.cc

+TEST_P(TestJsonFormatScanNode, ScanProjectedMissingColumns) {
+  TestScanProjectedMissingCols();
+}
+


Just scanning through the CSV tests I wonder if there are a few JSON-specific tests that might be nice to have:

Confirm that alternate read/parse options (e.g. alternate block size or newlines allowed) get passed in correctly

Does the JSON reader handle the the presence of a unicode BOM well? Maybe not as big an issue here where we have to explicitly handle it for the CSV reader.

The reader doesn't explicitly handle BOM, but I suspect that rapidjson does, as it's at least referenced in their docs. Either way, I added a test for it (which currently passes).

Co-authored-by: Weston Pace <weston.pace@gmail.com>

benibus · 2023-02-16T04:19:42Z

@felipecrv Feel free to take a look if you find the time.

westonpace

Thanks for the addition. A very cool feature.

felipecrv · 2023-02-17T01:24:41Z

@benibus sorry, I only noticed this today. Feel free to mention my handle as soon as PR is out of draft status.

ursabot · 2023-02-17T03:33:52Z

Benchmark runs are scheduled for baseline = 9033573 and contender = 45918a9. 45918a9 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.28% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.26% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.29% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 45918a90 ec2-t3-xlarge-us-east-2
[Finished] 45918a90 test-mac-arm
[Finished] 45918a90 ursa-i9-9960x
[Finished] 45918a90 ursa-thinkcentre-m75q
[Finished] 9033573a ec2-t3-xlarge-us-east-2
[Failed] 9033573a test-mac-arm
[Finished] 9033573a ursa-i9-9960x
[Finished] 9033573a ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

benibus · 2023-02-17T17:07:37Z

@felipecrv No worries! I'll let you in earlier on the next one.

This adds initial support the JSON file format to the Dataset library. Since there's currently no public API for writing JSON files, this only deals with the reader-side facilities. * Closes: apache#33209 Lead-authored-by: benibus <bpharks@gmx.com> Co-authored-by: Ben Harkins <60872452+benibus@users.noreply.github.com> Co-authored-by: Weston Pace <weston.pace@gmail.com> Signed-off-by: Weston Pace <weston.pace@gmail.com>

### Rationale for this change Enable Support for reading JSON Datasets #33732 on Java side ### What changes are included in this PR? Support for reading JSON Datasets ### Are these changes tested? Unit test added ### Are there any user-facing changes? No * Closes: #36421 Lead-authored-by: david dali susanibar arce <davi.sarces@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

…he#36422) ### Rationale for this change Enable Support for reading JSON Datasets apache#33732 on Java side ### What changes are included in this PR? Support for reading JSON Datasets ### Are these changes tested? Unit test added ### Are there any user-facing changes? No * Closes: apache#36421 Lead-authored-by: david dali susanibar arce <davi.sarces@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

github-actions bot added the Component: C++ label Jan 17, 2023

benibus force-pushed the GH-33209-json-dataset branch from d96db04 to 8760cb2 Compare January 25, 2023 22:21

benibus force-pushed the GH-33209-json-dataset branch from 8760cb2 to b926a05 Compare February 7, 2023 15:38

benibus marked this pull request as ready for review February 7, 2023 23:32

benibus requested a review from westonpace as a code owner February 7, 2023 23:32

westonpace requested changes Feb 9, 2023

View reviewed changes

benibus force-pushed the GH-33209-json-dataset branch from 0cae0e1 to 831df82 Compare February 9, 2023 18:37

benibus requested a review from westonpace February 9, 2023 18:49

westonpace requested changes Feb 10, 2023

View reviewed changes

benibus and others added 12 commits February 15, 2023 21:10

JSON format implementation for legacy scanner

44b8dc2

Update header name

c82de85

Add partial support for ScanNode API

26ffd07

Apply suggestions from code review

6194a65

Co-authored-by: Weston Pace <weston.pace@gmail.com>

Fix formatting

f5ad076

Address most points from code review

533934d

Apply more suggestions from code review

c47538b

Co-authored-by: Weston Pace <weston.pace@gmail.com>

Common test fixtures additions/fixes

57b8b3e

Add json-specific tests

42ed919

Make test names consistent

c6f1f62

Add test for unicode BOM

ed4e4a2

Fix unignored ParseOptions, update test

5bbd53e

benibus force-pushed the GH-33209-json-dataset branch from 67b62c1 to 5bbd53e Compare February 16, 2023 03:37

benibus mentioned this pull request Feb 16, 2023

[Python] Add bindings for JSON format in Dataset #34216

Closed

benibus requested a review from westonpace February 16, 2023 03:58

westonpace approved these changes Feb 16, 2023

View reviewed changes

westonpace merged commit 45918a9 into apache:master Feb 16, 2023

nealrichardson mentioned this pull request Mar 3, 2023

[R] Bindings for JSON Dataset #34436

Closed

This was referenced Jun 30, 2023

[Java] Bindings for JSON Dataset #36421

Closed

GH-36421: [Java] Enable Support for reading JSON Datasets #36422

Merged

thisisnic mentioned this pull request Oct 11, 2023

[R] open_dataset - format is unclear #38178

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-33209: [C++] Support for reading JSON Datasets #33732

GH-33209: [C++] Support for reading JSON Datasets #33732

benibus commented Jan 17, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Jan 17, 2023

github-actions bot commented Jan 17, 2023

benibus commented Feb 7, 2023

benibus commented Feb 7, 2023

westonpace left a comment

westonpace left a comment

westonpace Feb 10, 2023

benibus Feb 16, 2023

benibus commented Feb 16, 2023

westonpace left a comment

felipecrv commented Feb 17, 2023

ursabot commented Feb 17, 2023

benibus commented Feb 17, 2023

GH-33209: [C++] Support for reading JSON Datasets #33732

GH-33209: [C++] Support for reading JSON Datasets #33732

Conversation

benibus commented Jan 17, 2023 • edited by github-actions bot Loading

github-actions bot commented Jan 17, 2023

github-actions bot commented Jan 17, 2023

benibus commented Feb 7, 2023

benibus commented Feb 7, 2023

westonpace left a comment

Choose a reason for hiding this comment

westonpace left a comment

Choose a reason for hiding this comment

westonpace Feb 10, 2023

Choose a reason for hiding this comment

benibus Feb 16, 2023

Choose a reason for hiding this comment

benibus commented Feb 16, 2023

westonpace left a comment

Choose a reason for hiding this comment

felipecrv commented Feb 17, 2023

ursabot commented Feb 17, 2023

benibus commented Feb 17, 2023

benibus commented Jan 17, 2023 •

edited by github-actions bot

Loading