adriangb commented on code in PR #27087:
URL: https://github.com/apache/datafusion/pull/27087#discussion_r4466694339
##########
datafusion/physical-plan/src/joins/hash_join/stream.rs:
##########
@@ -245,18 +245,23 @@ pub(super) struct EmitUnmatchedBuildRowsState {
/// `Scheduled` means the reporting `OnceFut` has been constructed but is lazy:
/// the coordinator has not necessarily observed the report. Only `Delivered`
/// guarantees the coordinator saw it, so `Drop` must still cancel a
`Scheduled`
-/// partition — otherwise sibling partitions can wait forever for a report that
+/// partition. Otherwise sibling partitions can wait forever for a report that
/// never runs.
#[derive(Debug, PartialEq, Eq)]
enum BuildReportState {
NotReported,
Scheduled,
Delivered,
+ /// Dropped before delivery and reported as canceled.
Canceled,
Finalized,
}
/// Owns the stream-side lifecycle for one partition's build-data report.
+///
+/// In `Partitioned` mode, dropping the handle before delivery reports the
+/// partition as canceled. `PartitionStatus::CanceledUnknown` says what that
Review Comment:
This is referencing a doc in another module. But that Doc is orthogonally
related: `PartitionStatus` is for `shared_bounds` (which is dynamic filter
building). This struct is for the build side partition management in general.
IMO we should do something like:
1. `BuildReportState`, `BuildReportHandle` hold information on what it means
for a partition to be cancelled in general. We stop the build side immediately,
any row routed to that build side will be dropped, but we still run all
partitions on the probe side (unless all build sides were cancelled) since we
don't know what probe side partitions will route to which build side partitions.
2. `PartitionStatus` should clarify what this means for dynamic filters.
--
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]
