-
Notifications
You must be signed in to change notification settings - Fork 267
Feat: to_csv #3004
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feat: to_csv #3004
Conversation
| CsvWriteOptions options = 2; | ||
| } | ||
|
|
||
| message CsvWriteOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3004 +/- ##
============================================
+ Coverage 56.12% 59.58% +3.45%
- Complexity 976 1381 +405
============================================
Files 119 167 +48
Lines 11743 15562 +3819
Branches 2251 2577 +326
============================================
+ Hits 6591 9272 +2681
- Misses 4012 4992 +980
- Partials 1140 1298 +158 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
parthchandra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Sorry for the delay in reviewing). This looks pretty good to me, pending ci.
Also a minor comment on escaping. Can you confirm that this behaviour is consistent with Spark?
| fn escape_value(value: &str, quote: &str, escape: &str, output: &mut String) { | ||
| for ch in value.chars() { | ||
| let ch_str = ch.to_string(); | ||
| if ch_str == quote || ch_str == escape { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSV spec does not have any special escape for escape and the preferred way to escape the double quote is another double quote (but only if the string is enclosed in a double quote!). - https://datatracker.ietf.org/doc/html/rfc4180#section-2
Not sure what Spark does here.
Which issue does this PR close?
Rationale for this change
Basic implementation of the spark to_csv function added - https://spark.apache.org/docs/latest/api/sql/index.html#to_csv
What changes are included in this PR?
How are these changes tested?
Benchmark results (need optimization):
