alamb commented on code in PR #18673:
URL: https://github.com/apache/datafusion/pull/18673#discussion_r2550168991
##########
datafusion/physical-plan/src/repartition/mod.rs:
##########
@@ -740,18 +739,23 @@ impl RepartitionExec {
impl DisplayAs for RepartitionExec {
fn fmt_as(&self, t: DisplayFormatType, f: &mut Formatter) ->
std::fmt::Result {
+ let input_partition_count =
self.input.output_partitioning().partition_count();
match t {
DisplayFormatType::Default | DisplayFormatType::Verbose => {
write!(
f,
"{}: partitioning={}, input_partitions={}",
self.name(),
self.partitioning(),
- self.input.output_partitioning().partition_count()
+ input_partition_count,
)?;
if self.preserve_order {
write!(f, ", preserve_order=true")?;
+ } else if input_partition_count <= 1
Review Comment:
```suggestion
} else if input_partition_count <= 1
// Make it explicit that repartition maintains sortedness
for a single input partition even
// when `preserve_sort order` is false
```
##########
datafusion/physical-plan/src/repartition/mod.rs:
##########
@@ -737,18 +737,21 @@ impl RepartitionExec {
impl DisplayAs for RepartitionExec {
fn fmt_as(&self, t: DisplayFormatType, f: &mut Formatter) ->
std::fmt::Result {
+ let input_partition_count =
self.input.output_partitioning().partition_count();
match t {
DisplayFormatType::Default | DisplayFormatType::Verbose => {
write!(
f,
"{}: partitioning={}, input_partitions={}",
self.name(),
self.partitioning(),
- self.input.output_partitioning().partition_count()
+ input_partition_count,
)?;
if self.preserve_order {
write!(f, ", preserve_order=true")?;
+ } else if input_partition_count <= 1 {
+ write!(f, ", maintains_sort_order=true")?;
Review Comment:
I agree -- this was confusing at first. Reading @ruchirK 's explanation in
the description makes sense, but I think it is somewhat subtle. Maybe we can
add some comments to the code to make it clearer.
I'll leave a suggestion
##########
datafusion/physical-plan/src/repartition/mod.rs:
##########
@@ -774,6 +775,10 @@ impl DisplayAs for RepartitionExec {
if self.preserve_order {
writeln!(f, "preserve_order={}", self.preserve_order)?;
+ } else if input_partition_count <= 1
Review Comment:
Since the tree format is designed to be less verbose, I personally suggest
not adding the additional information to the tree format as well -- people who
want more detail can use the indent format
--
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]
