[PR] fix(core): report the real partitioning from ShuffleWriterExec [datafusion-ballista]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new pull request, #2307:
URL: https://github.com/apache/datafusion-ballista/pull/2307

   # Which issue does this PR close?
   
   Closes #2306.
   
   # Rationale for this change
   
   `ShuffleWriterExec`'s `DisplayAs` wrote a string literal rather than a value:
   
   ```rust
   // "None" is retained for plan-shape stability: this writer never
   // repartitions, so the value can only ever be None.
   write!(f, "ShuffleWriterExec: partitioning: None")
   ```
   
   It used to print a value. #2106 removed the `shuffle_output_partitioning` field along with the hash-partitioning writer, and the token was frozen as a literal so the 22 approved TPC-H goldens would not churn in the same diff. That was a fine expedient, but it left the output carrying no information and kept *because* it carries none: a constant cannot drift, so it cannot break a golden file.
   
   It was also misleading. The `None` refers to the absence of a repartitioning scheme, an internal distinction a plan reader cannot infer. The writer preserves its input's partitioning, and `try_new` already sets its `PlanProperties` to exactly that, so the real value was available the whole time.
   
   There was a downstream cost too. #2305 labels DOT graph nodes from each operator's `DisplayAs`, which works everywhere except here: because the text was a constant, that PR had to keep a hand-written special case for `ShuffleWriterExec` to recover the partition count.
   
   # What changes are included in this PR?
   
   - Render `self.properties().output_partitioning()` in both `Default`/`Verbose` and `TreeRender`, matching how `SortShuffleWriterExec` already reports its own.
   - Add `display_as_reports_real_partitioning`, which renders two writers over inputs with different partitioning and asserts each reflects its own. A constant cannot pass it.
   - Regenerate the 22 approved goldens with `BALLISTA_GENERATE_GOLDEN=1`.
   
   The golden diff is 45 lines and entirely mechanical. Each new value is checkable against the child plan on the line below it, for example in `q1.txt`:
   
   ```
   === Stage 2 ===
   ShuffleWriterExec: partitioning: Hash([l_returnflag@0, l_linestatus@1], 16)
     ...
           UnresolvedShuffleExec: stage=1, partitioning: Hash([l_returnflag@0, l_linestatus@1], 16)
   
   === Stage 3 ===
   ShuffleWriterExec: partitioning: UnknownPartitioning(1)
     SortPreservingMergeExec: [l_returnflag@0 ASC NULLS LAST, l_linestatus@1 ASC NULLS LAST]
   ```
   
   Stage 2 inherits the hash partitioning it writes; stage 3 collapsed to one partition through the merge. Both were previously reported as `None`.
   
   # Are there any user-facing changes?
   
   `EXPLAIN` output and the scheduler's rendered stage plans now show the writer's actual output partitioning instead of a constant `None`. No public API changes.
   
   Two related warts noted in the issue are deliberately left alone here, since neither is needed for the fix: the two writers use different separators (`partitioning: ` versus `partitioning=`), and `ShuffleWriterExec` also carries a plain `impl Display` whose content differs from its `DisplayAs`.
   
   Once this lands, the `ShuffleWriterExec` special case in #2305 can be dropped and that operator labeled generically like every other one.


-- 
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-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix(core): report the real partitioning from ShuffleWriterExec [datafusion-ballista]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove merged PR #2307:
URL: https://github.com/apache/datafusion-ballista/pull/2307


-- 
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-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org