alamb commented on code in PR #11415:
URL: https://github.com/apache/datafusion/pull/11415#discussion_r1675730066
##########
datafusion/core/src/datasource/file_format/write/orchestration.rs:
##########
@@ -50,7 +50,7 @@ pub(crate) async fn serialize_rb_stream_to_object_store(
mut data_rx: Receiver<RecordBatch>,
serializer: Arc<dyn BatchSerializer>,
mut writer: WriterType,
-) -> std::result::Result<(WriterType, u64), (WriterType, DataFusionError)> {
+) -> std::result::Result<(WriterType, u64), (Option<WriterType>,
DataFusionError)> {
Review Comment:
I think the rationale would be hard to discern only from the code. Could you
please update the documentation to reflect the change of what is returned on
error?
Specifically, I think it is that if there was an error on write, the writer
is dropped so we don't accidentally write to it or try and close it. For any
error not involving the writer, the writer is returned as well
cc @metesynnada as I remember at some point maybe you had a usecase for
calling `shutdown` on a writer after error 🤔 I may be misremembering
##########
datafusion/core/src/datasource/file_format/write/orchestration.rs:
##########
@@ -50,7 +50,7 @@ pub(crate) async fn serialize_rb_stream_to_object_store(
mut data_rx: Receiver<RecordBatch>,
serializer: Arc<dyn BatchSerializer>,
mut writer: WriterType,
-) -> std::result::Result<(WriterType, u64), (WriterType, DataFusionError)> {
+) -> std::result::Result<(WriterType, u64), (Option<WriterType>,
DataFusionError)> {
Review Comment:
not for this PR, but this signature is getting pretty gnarly. Maybe it is
time to try and encapsulate some of this logic into structs 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
