tschwarzinger commented on code in PR #26249:
URL: https://github.com/apache/datafusion/pull/26249#discussion_r4256977395
##########
datafusion/physical-plan/src/joins/hash_join/shared_bounds.rs:
##########
@@ -729,6 +773,22 @@ impl SharedBuildAccumulator {
// Only one build partition contains rows, so its filter covers
every
// possible probe match without routing.
Arc::clone(&partition_filters[real_partition_ids[0]])
+ } else if let Some(lookups) = lookups.filter(|lookups| {
+ self.probe_range_partitioning.is_none()
+ && lookups.iter().any(|l| matches!(l, PartitionLookup::Map(_)))
Review Comment:
Maybe we can add
`let lookup_expression_beneficial = lookups.iter().any(|l| matches!(l,
PartitionLookup::Map(_)))`
or something similar to make it more obvious why this check is in place. At
first I thought this is a correctness check but it seems like the construction
of the `lookup` vector guarantees that the expression can be created correctly.
Hmm after second thought, why is this check there? Wouldn't it also be
beneficial if all partitions where different constants? Not sure how
cancellation etc. plays into that.
--
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]
