-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat: Add CSV sink for the new streaming engine #20804
feat: Add CSV sink for the new streaming engine #20804
Conversation
@@ -221,15 +221,20 @@ fn to_graph_rec<'a>( | |||
nodes::io_sinks::ipc::IpcSinkNode::new(input_schema, path, ipc_writer_options)?, | |||
[(input_key, input.port)], | |||
), | |||
#[cfg(feature = "ipc")] |
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.
drive-by: this was not correct
.with_float_precision(options.serialize_options.float_precision) | ||
.with_null_value(options.serialize_options.null.clone()) | ||
.with_quote_style(options.serialize_options.quote_style) | ||
.n_threads(1) // Disable rayon parallelism |
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.
That was easy. :)
|
||
let mut buffer = Vec::with_capacity(allocation_size); | ||
let mut writer = CsvWriter::new(&mut buffer) | ||
.include_bom(false) // Handled once in the IO task. |
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.
Maybe we can hoist the writer out of the while loop?
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.
I don't think it matters much since the CsvWriter
creation is cheap, and we cannot reuse the allocation anyway because it needs to be passed to the IO task.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20804 +/- ##
==========================================
- Coverage 79.77% 79.75% -0.03%
==========================================
Files 1561 1562 +1
Lines 222029 222397 +368
Branches 2533 2533
==========================================
+ Hits 177132 177369 +237
- Misses 44313 44444 +131
Partials 584 584 ☔ View full report in Codecov by Sentry. |
ping @orlp.