jayzhan211 commented on code in PR #12864:
URL: https://github.com/apache/datafusion/pull/12864#discussion_r1797538174
##########
datafusion/sqllogictest/test_files/subquery.slt:
##########
@@ -313,10 +319,12 @@ physical_plan
08)--------------CoalesceBatchesExec: target_batch_size=2
09)----------------RepartitionExec: partitioning=Hash([t2_id@0], 4),
input_partitions=4
10)------------------AggregateExec: mode=Partial, gby=[t2_id@0 as t2_id],
aggr=[sum(t2.t2_int)]
-11)--------------------MemoryExec: partitions=4, partition_sizes=[1, 0, 0, 0]
-12)------CoalesceBatchesExec: target_batch_size=2
-13)--------RepartitionExec: partitioning=Hash([t1_id@0], 4), input_partitions=4
-14)----------MemoryExec: partitions=4, partition_sizes=[1, 0, 0, 0]
+11)--------------------RepartitionExec: partitioning=RoundRobinBatch(4),
input_partitions=1
+12)----------------------MemoryExec: partitions=1, partition_sizes=[1]
+13)------CoalesceBatchesExec: target_batch_size=2
+14)--------RepartitionExec: partitioning=Hash([t1_id@0], 4), input_partitions=4
+15)----------RepartitionExec: partitioning=RoundRobinBatch(4),
input_partitions=1
+16)------------MemoryExec: partitions=1, partition_sizes=[1]
Review Comment:
In `main` branch, when you create table with values, `RoundRobinBatch` is
applied to it because we have cast expr. The value is i64 by default, so when
we have int column, we need to cast to i32.
```rust
fn benefits_from_input_partitioning(&self) -> Vec<bool> {
let all_simple_exprs = self
.expr
.iter()
.all(|(e, _)| e.as_any().is::<Column>() ||
e.as_any().is::<Literal>());
// If expressions are all either column_expr or Literal, then all
computations in this projection are reorder or rename,
// and projection would not benefit from the repartition,
benefits_from_input_partitioning will return false.
vec![!all_simple_exprs]
}
```
But, in this change, it is already cast in Value, so there is no cast expr
in Projection, so it is like `MemoryExec: partitions=1, partition_sizes=[1]`
instead of `MemoryExec: partitions=4, partition_sizes=[1, 0, 0, 0]`.
Therefore, I think the plan makes sense to me.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
