Jefffrey commented on code in PR #18678:
URL: https://github.com/apache/datafusion/pull/18678#discussion_r2525629884
##########
datafusion/optimizer/src/eliminate_nested_union.rs:
##########
@@ -55,6 +55,9 @@ impl OptimizerRule for EliminateNestedUnion {
_config: &dyn OptimizerConfig,
) -> Result<Transformed<LogicalPlan>> {
match plan {
+ LogicalPlan::Union(Union { mut inputs, .. }) if inputs.len() == 1
=> Ok(
Review Comment:
Worth adding a minor comment here, considering its not exactly related to
nested unions?
(Or could rename away from `EliminateNestedUnions` to something like
`SimplifyUnions` to be all encompassing)
##########
datafusion/optimizer/src/optimizer.rs:
##########
@@ -242,7 +241,6 @@ impl Optimizer {
Arc::new(EliminateLimit::new()),
Arc::new(PropagateEmptyRelation::new()),
// Must be after PropagateEmptyRelation
Review Comment:
It seems this comment was added with `EliminateOneUnion` so need to remove
it, reference:
https://github.com/apache/datafusion/commit/704e034abbc57645ae594288c49c550350540022#diff-42d66905c3fa6b245c3493fb4df4816e0fde3036941315ab42198fb16f3907df
(Any impact that the logic is now before `PropagateEmptyRelation`? Or
running multiple passes of optimizer makes order redundant anyway?)
--
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]
