alamb commented on code in PR #7510:
URL: https://github.com/apache/arrow-datafusion/pull/7510#discussion_r1320170044


##########
datafusion/core/src/physical_plan/stream.rs:
##########
@@ -169,7 +171,16 @@ impl RecordBatchReceiverStreamBuilder {
         let check = async move {
             while let Some(result) = join_set.join_next().await {
                 match result {
-                    Ok(()) => continue, // nothing to report
+                    Ok(task_result) => {
+                        match task_result {
+                            // nothing to report
+                            Ok(_) => continue,
+                            // This means a blocking task error
+                            Err(e) => {
+                                return Some(internal_err!("Spawned Task error: 
{e}"));

Review Comment:
   `ExecutionError` might be more appropriate than `InternalError` given an 
error in a spill file is likely not a bug in DataFusion, but rather some issue 
in the environment. 



##########
datafusion/core/src/physical_plan/common.rs:
##########
@@ -109,9 +109,13 @@ pub(crate) fn spawn_buffered(
             builder.spawn(async move {
                 while let Some(item) = input.next().await {
                     if sender.send(item).await.is_err() {

Review Comment:
   The only reason an item can't be sent via a channel I think is if the other 
end (the `Receiver`) was dropped. This can certainly happen if some other part 
of the query errors, but I also think it can happen with the plan shuts down 
early due to a `LIMIT` or something similar. 
   
   Thus I am not sure we should propagate an error in this case



-- 
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]

Reply via email to