findepi commented on code in PR #12289:
URL: https://github.com/apache/datafusion/pull/12289#discussion_r1740932826
##########
datafusion/physical-plan/src/aggregates/topk/priority_map.rs:
##########
@@ -25,17 +25,12 @@ use datafusion_common::Result;
/// A `Map<K, V>` / `PriorityQueue` combo that evicts the worst values after
reaching `capacity`
pub struct PriorityMap {
- map: Box<dyn ArrowHashTable>,
- heap: Box<dyn ArrowHeap>,
+ map: Box<dyn ArrowHashTable + Send>,
+ heap: Box<dyn ArrowHeap + Send>,
capacity: usize,
mapper: Vec<(usize, usize)>,
}
-// JUSTIFICATION
Review Comment:
i didn't verify benchmarks, but per static code anaysis, PriorityMap is
required to be Send. if you just remove trait implementation (marker), code
won't compile.
With this PR, PriorityMap is **still** Send. The only difference is that
this is now inferred by the compiler, so safer.
--
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]
