JNI bindings to write CSV #12425

mythrocks · 2022-12-20T00:32:09Z

Description

This change adds JNI bindings to write tables out as CSV, to either the filesystem or memory.

The Java Table class now has additional methods:

Table.writeCSVToFile(): Writes the current table out to the specified file on the filesystem.
Table.writeCSVToBuffer(): Writes the current table out to a HostBufferConsumer.

These calls are analogous to cudf::io::write_csv().

Current limitations:

The cudf::io::csv_writer_options interface binds the CSV options tightly to the Table being written. This makes it a little clumsy to write multiple Tables to the same HostBufferConsumer, because each could be written with different, contradictory options. This has now been addressed. Chunked CSV writing is now permitted (albeit only the host memory buffers).
cudf::io::write_csv(file_name) overwrites the specified file, if it exists. There currently isn't a way to keep a file open, and write multiple tables to it; each write call overwrites the previous file.

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

This change adds JNI bindings to write tables out as CSV, to either the filesystem or memory. The Java Table class now has additional methods: 1. Table.writeCSVToFile(): Writes the current table out to the specified file on the filesystem. 2. Table.writeCSVToBuffer(): Writes the current table out to a HostBufferConsumer. These calls are analogous to cudf::io::write_csv(). Current limitations: 1. The cudf::io::csv_writer_options interface binds the CSV options tightly to the Table being written. This makes it a little clumsy to write multiple Tables to the same HostBufferConsumer, because each could be written with different, contradictory options. 2. cudf::io::write_csv(file_name) overwrites the specified file, if it exists. There currently isn't a way to keep a file open, and write multiple tables to it; each write call overwrites the previous file.

mythrocks · 2022-12-20T07:28:16Z

This makes it a little clumsy to write multiple Tables to the same HostBufferConsumer

I'm working on a workaround for this, now. Should have it shortly.

revans2 · 2022-12-20T14:59:01Z

This makes it a little clumsy to write multiple Tables to the same HostBufferConsumer

I'm working on a workaround for this, now. Should have it shortly.

Be very careful when using the HostBufferConsumer because we don't want to write out the header multiple times. Also the others have checks in the C++ code when doing chunked writes that the column types/etc match between multiple invocations. IF we are going to create our own chunked writer API, we should add in our own corresponding checks too.

mythrocks · 2022-12-20T16:21:30Z

This makes it a little clumsy to write multiple Tables to the same HostBufferConsumer

I'm working on a workaround for this, now. Should have it shortly.

Be very careful when using the HostBufferConsumer because we don't want to write out the header multiple times. Also the others have checks in the C++ code when doing chunked writes that the column types/etc match between multiple invocations. IF we are going to create our own chunked writer API, we should add in our own corresponding checks too.

Yes, sir. That's what prompted me to write a chunked writer. It will reside in JNI for now. I can see about moving it to cpp/ afterwards, if there's interest.

ajschmidt8 · 2022-12-20T21:22:27Z

The forked branch for this PR did not have the GH Actions CI files included because it was outdated, so I merged the latest changes into this branch to start CI.

1. Added setter to change Table instance in csv_writer_options. 2. Plumbing for new chunked writer. 3. Tests.

mythrocks · 2022-12-21T07:09:28Z

Sorry I missed your update, @ajschmidt8.

This new update might be out of date as well. I'll upmerge and push very shortly.

java/src/test/java/ai/rapids/cudf/TableTest.java

java/src/main/native/src/TableJni.cpp

java/src/main/java/ai/rapids/cudf/CSVWriterOptions.java

mythrocks · 2022-12-31T00:42:34Z

Re: the CI test failures:

/opt/conda/bin/git -c protocol.version=2 fetch --prune --progress --no-recurse-submodules origin +refs/heads/*:refs/remotes/origin/* +refs/tags/*:refs/tags/*
  Error: fatal: unable to access 'https://github.com/rapidsai/cudf/': The requested URL returned error: 429
  The process '/opt/conda/bin/git' failed with exit code 128

Accessing the CUDF repo on GitHub hit rate limiting. :]

mythrocks · 2023-01-01T01:17:26Z

Rerun tests

cpp/include/cudf/io/csv.hpp

vuule · 2023-01-03T19:35:00Z

cpp/include/cudf/io/csv.hpp

+   *
+   * @param table Table to be written
+   */
+  void set_table(table_view const& table) { _table = table; }


Looks good. As mentioned offline, we might want to look into separating the sink, input table and writer options to facilitate easier reuse of options. Not in scope for this PR.

ttnghia · 2023-01-03T20:30:02Z

Gently remind that the changed files need to be updated copyright year ;)

mythrocks · 2023-01-03T20:54:57Z

Gently remind that the changed files need to be updated copyright year ;)

:)) Good catch.
I thought I was safe because the commits are from last year, but we're doing squashed commits. I'll update.

codecov · 2023-01-03T23:29:25Z

Codecov Report

Base: 86.58% // Head: 85.69% // Decreases project coverage by -0.88% ⚠️

Coverage data is based on head (8d9b374) compared to base (b6dccb3).
Patch has no changes to coverable lines.

Additional details and impacted files

@@               Coverage Diff                @@
##           branch-23.02   #12425      +/-   ##
================================================
- Coverage         86.58%   85.69%   -0.89%     
================================================
  Files               155      155              
  Lines             24368    24797     +429     
================================================
+ Hits              21098    21249     +151     
- Misses             3270     3548     +278

Impacted Files	Coverage Δ
python/cudf/cudf/_version.py	`1.41% <0.00%> (-98.59%)`	⬇️
python/cudf/cudf/core/buffer/spill_manager.py	`72.50% <0.00%> (-7.50%)`	⬇️
python/cudf/cudf/core/buffer/spillable_buffer.py	`90.04% <0.00%> (-2.81%)`	⬇️
python/cudf/cudf/utils/dtypes.py	`77.77% <0.00%> (-1.69%)`	⬇️
python/cudf/cudf/options.py	`86.11% <0.00%> (-1.59%)`	⬇️
python/cudf/cudf/core/single_column_frame.py	`94.30% <0.00%> (-1.27%)`	⬇️
...ython/custreamz/custreamz/tests/test_dataframes.py	`98.38% <0.00%> (-1.01%)`	⬇️
python/dask_cudf/dask_cudf/io/parquet.py	`91.81% <0.00%> (-0.59%)`	⬇️
python/cudf/cudf/core/multiindex.py	`91.66% <0.00%> (-0.51%)`	⬇️
python/cudf/cudf/core/algorithms.py	`90.00% <0.00%> (-0.48%)`	⬇️
... and 37 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mythrocks · 2023-01-04T17:51:21Z

Conflicting files
java/src/main/native/src/TableJni.cpp

~~I'm on it now.~~ Resolved now.

mythrocks · 2023-01-04T19:45:27Z

Waiting on rapidsai/rapids-cmake#337 to re-kick the build.

revans2

Most of the changes look fine to me. The one issue that I have is that for the hive text file format the cudf CSV write is not going to be compatible. By default hive does not do any special escapes for string values. CUDF does escaping.

cudf/cpp/src/io/csv/writer_impl.cu

Line 75 in 8d1e2cc

struct escape_strings_fn {

Should we spend some time to make this optional? Or am I missing what this is for?

java/src/main/java/ai/rapids/cudf/CSVWriterOptions.java

java/src/main/native/src/csv_chunked_writer.hpp

mythrocks · 2023-01-05T00:17:54Z

CUDF does escaping.

Thank you for catching that. I will try tackle that in a follow-up issue, specifically to make the escaping optional.

1. Removed TODO. 2. Changed default null value to empty string.

mythrocks · 2023-01-05T20:52:52Z

/merge

mythrocks · 2023-01-05T20:53:36Z

I've merged this. Thank you to all who reviewed.
A couple of follow-up PRs will, well, follow.

mythrocks added feature request New feature or request Spark Functionality that helps Spark RAPIDS non-breaking Non-breaking change labels Dec 20, 2022

mythrocks self-assigned this Dec 20, 2022

mythrocks marked this pull request as draft December 20, 2022 00:32

mythrocks requested a review from a team as a code owner December 20, 2022 00:32

github-actions bot added the Java Affects Java cuDF API. label Dec 20, 2022

Support for chunked CSV writes in JNI:

7fa0204

1. Added setter to change Table instance in csv_writer_options. 2. Plumbing for new chunked writer. 3. Tests.

mythrocks force-pushed the hive-text-writer branch from 91cc650 to 7fa0204 Compare December 21, 2022 07:07

github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Dec 21, 2022

mythrocks added 4 commits December 20, 2022 23:09

Merge remote-tracking branch 'origin/branch-23.02' into hive-text-writer

faa64c4

Merge remote-tracking branch 'origin/branch-23.02' into hive-text-writer

2fa91e7

Added tests header inclusion.

e446ae3

Formatting.

54a5a87

razajafri reviewed Dec 21, 2022

View reviewed changes

java/src/test/java/ai/rapids/cudf/TableTest.java Show resolved Hide resolved

java/src/main/native/src/TableJni.cpp Outdated Show resolved Hide resolved

razajafri reviewed Dec 21, 2022

View reviewed changes

java/src/main/java/ai/rapids/cudf/CSVWriterOptions.java Show resolved Hide resolved

mythrocks added 6 commits December 21, 2022 15:10

Support to specify TRUE/FALSE strings.

c8f74de

Added tests for combinations of True/False reps, header inclusion, etc.

ebbfcb8

Removed JNI's non-chunked CSV writes to memory.

cce5574

Merge remote-tracking branch 'origin/branch-23.02' into hive-text-writer

7089163

Added newline at the end of the file, per CUDF guideline.

15693f1

Removed unnecessary whitespace at top of file.

15e84c5

mythrocks marked this pull request as ready for review December 28, 2022 18:36

mythrocks requested a review from a team as a code owner December 28, 2022 18:36

hyperbolic2346 approved these changes Dec 31, 2022

View reviewed changes

mythrocks mentioned this pull request Dec 31, 2022

Refactor jni writer data sink #12458

Closed

3 tasks

mythrocks requested a review from ttnghia January 3, 2023 18:03

vuule reviewed Jan 3, 2023

View reviewed changes

cpp/include/cudf/io/csv.hpp Show resolved Hide resolved

vuule approved these changes Jan 3, 2023

View reviewed changes

mythrocks added 3 commits January 3, 2023 13:50

Updated documentation for _inter_column_delimiter.

d3642a4

Updated copyright date.

c83e0d9

Merge remote-tracking branch 'origin/branch-23.02' into hive-text-writer

6dee89a

mythrocks requested a review from vuule January 3, 2023 22:17

ttnghia approved these changes Jan 4, 2023

View reviewed changes

Merge remote-tracking branch 'origin/branch-23.02' into hive-text-writer

e4fa895

razajafri approved these changes Jan 4, 2023

View reviewed changes

revans2 reviewed Jan 4, 2023

View reviewed changes

java/src/main/java/ai/rapids/cudf/CSVWriterOptions.java Outdated Show resolved Hide resolved

java/src/main/java/ai/rapids/cudf/CSVWriterOptions.java Outdated Show resolved Hide resolved

java/src/main/native/src/csv_chunked_writer.hpp Outdated Show resolved Hide resolved

mythrocks added 2 commits January 4, 2023 16:26

Review fixes:

bfd2cd3

1. Removed TODO. 2. Changed default null value to empty string.

Merge remote-tracking branch 'origin/branch-23.02' into hive-text-writer

b57e8d9

mythrocks requested a review from revans2 January 5, 2023 00:28

revans2 approved these changes Jan 5, 2023

View reviewed changes

Merge branch 'branch-23.02' into hive-text-writer

8d9b374

mythrocks added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Jan 5, 2023

rapids-bot bot merged commit 5d2cf8c into rapidsai:branch-23.02 Jan 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JNI bindings to write CSV #12425

JNI bindings to write CSV #12425

mythrocks commented Dec 20, 2022 •

edited

Loading

mythrocks commented Dec 20, 2022

revans2 commented Dec 20, 2022

mythrocks commented Dec 20, 2022

ajschmidt8 commented Dec 20, 2022 •

edited

Loading

mythrocks commented Dec 21, 2022

mythrocks commented Dec 31, 2022

mythrocks commented Jan 1, 2023

vuule Jan 3, 2023

ttnghia commented Jan 3, 2023

mythrocks commented Jan 3, 2023

codecov bot commented Jan 3, 2023 •

edited

Loading

mythrocks commented Jan 4, 2023 •

edited

Loading

mythrocks commented Jan 4, 2023

revans2 left a comment

mythrocks commented Jan 5, 2023

mythrocks commented Jan 5, 2023

mythrocks commented Jan 5, 2023

JNI bindings to write CSV #12425

JNI bindings to write CSV #12425

Conversation

mythrocks commented Dec 20, 2022 • edited Loading

Description

Current limitations:

Checklist

mythrocks commented Dec 20, 2022

revans2 commented Dec 20, 2022

mythrocks commented Dec 20, 2022

ajschmidt8 commented Dec 20, 2022 • edited Loading

mythrocks commented Dec 21, 2022

mythrocks commented Dec 31, 2022

mythrocks commented Jan 1, 2023

vuule Jan 3, 2023

Choose a reason for hiding this comment

ttnghia commented Jan 3, 2023

mythrocks commented Jan 3, 2023

codecov bot commented Jan 3, 2023 • edited Loading

Codecov Report

mythrocks commented Jan 4, 2023 • edited Loading

mythrocks commented Jan 4, 2023

revans2 left a comment

Choose a reason for hiding this comment

mythrocks commented Jan 5, 2023

mythrocks commented Jan 5, 2023

mythrocks commented Jan 5, 2023

mythrocks commented Dec 20, 2022 •

edited

Loading

ajschmidt8 commented Dec 20, 2022 •

edited

Loading

codecov bot commented Jan 3, 2023 •

edited

Loading

mythrocks commented Jan 4, 2023 •

edited

Loading