alamb commented on a change in pull request #8285:
URL: https://github.com/apache/arrow/pull/8285#discussion_r495581230
##########
File path: rust/datafusion/benches/sort_limit_query_sql.rs
##########
@@ -66,21 +66,23 @@ fn create_context() -> ExecutionContext {
)
.unwrap();
- let mem_table = MemTable::load(&csv).unwrap();
+ // let mem_table = MemTable::load(&csv).await.unwrap();
+ //
+ // // create local execution context
+ // let ctx = ExecutionContext::new();
+ // ctx.state.config.concurrency = 1;
+ // ctx.register_table("aggregate_test_100", Box::new(mem_table));
+ // Arc::new(Mutex::new(ctx))
- // create local execution context
- let mut ctx = ExecutionContext::new();
- ctx.state.config.concurrency = 1;
- ctx.register_table("aggregate_test_100", Box::new(mem_table));
- ctx
+ unimplemented!()
Review comment:
🤔
##########
File path: rust/datafusion/src/physical_plan/merge.rs
##########
@@ -103,33 +106,38 @@ impl ExecutionPlan for MergeExec {
)),
1 => {
// bypass any threading if there is a single partition
- self.input.execute(0)
+ self.input.execute(0).await
}
_ => {
let partitions_per_thread = (input_partitions /
self.concurrency).max(1);
let range: Vec<usize> = (0..input_partitions).collect();
let chunks = range.chunks(partitions_per_thread);
- let threads: Vec<JoinHandle<Result<Vec<RecordBatch>>>> = chunks
- .map(|chunk| {
- let chunk = chunk.to_vec();
- let input = self.input.clone();
- thread::spawn(move || {
- let mut batches = vec![];
+
+ let mut tasks = vec![];
+ for chunk in chunks {
+ let chunk = chunk.to_vec();
+ let input = self.input.clone();
+ let task: JoinHandle<Result<Vec<Arc<RecordBatch>>>> =
Review comment:
Yeah, this is really much nicer in my opinion -- we spawn tasks (not
threads) -- and thus we won't create more threads than cpus and give users
better control over how the tasks are run. 👍
##########
File path: rust/rust-toolchain
##########
@@ -1 +1 @@
-nightly-2020-04-22
+nightly-2020-08-24
Review comment:
I understand this upgrade was helpful during development, but I suggest
we don't upgrade upon final merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
