andygrove opened a new pull request, #2305: URL: https://github.com/apache/datafusion-ballista/pull/2305
# Which issue does this PR close? Closes #2303. # Rationale for this change `ExecutionGraphDot` named each physical operator by walking a chain of `downcast_ref` arms and fell through to a literal `"Unknown Operator"`. `SortMergeJoinExec` and `SortPreservingMergeExec` were both missing from that chain, so an ordinary joined or sorted query rendered an unlabeled box where the join should be, and lost the operator at the top of a sorted plan. Sort-merge is Ballista's default join because `new_with_ballista()` sets `prefer_hash_join = false`, so this was not an edge case. Adding two more arms would fix the two reported operators and leave the underlying problem in place. DataFusion adds and renames physical operators every release, so a hand-maintained chain drifts by default, and it drifts silently: the fallback logs at `debug`, so the only visible symptom is a blank box in a UI. `DisplayAs` is a supertrait of `ExecutionPlan`, so every operator already knows how to describe itself, and that description is exactly what `EXPLAIN` prints. Labeling from it removes the fall-through case entirely, so no operator can render blank again, and it keeps the graph consistent with the plan text users already read. # What changes are included in this PR? - Label a node as the operator's `name()` on the first line and its `DisplayFormatType::Default` rendering on the second, with the repeated name stripped from the detail. Two lines keeps the boxes narrow. - Drop the fifteen-arm `downcast_ref` chain and the `"Unknown Operator"` fallback, along with the now-unused `format_partitioning` and `format_expr_list` helpers. - Keep two special cases, only where Ballista knows better than the default: `DataSourceExec`, whose default rendering lists every file group and is far too long for a graph node, so the existing `get_file_scan` summary is retained; and `ShuffleWriterExec`, whose rendering is the constant `partitioning: None` because it never repartitions, so the input partition count is shown instead. Both are enhancements rather than requirements, so an operator dropping out of them degrades to the DataFusion default rather than to nothing. - Add `dot_labels_every_operator`, which plans a join and an `ORDER BY` under `new_with_ballista()` and asserts both `SortMergeJoinExec` and `SortPreservingMergeExec` appear in the graph and that no node carries an empty label. - Update the four existing golden strings for the new labels. # Are there any user-facing changes? The node labels in `/api/job/<id>/dot`, `/api/job/<id>/stage/<n>/dot`, and the TUI job graph popup change. Nothing was lost. Names are now the DataFusion type names (`HashJoinExec` rather than `HashJoin`), and the labels carry more detail than before: join type and mode on joins, output partitioning on the sort shuffle writer and on unresolved shuffles. No public API changes. -- 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]
