pushnanashi2 commented on code in PR #25074:
URL: https://github.com/apache/datafusion/pull/25074#discussion_r3960108652
##########
datafusion/optimizer/src/unions_to_filter.rs:
##########
@@ -278,16 +292,18 @@ fn wrap_branch(mut plan: LogicalPlan, wrappers:
&[Wrapper]) -> Result<LogicalPla
Ok(plan)
}
-fn strip_passthrough_nodes(mut plan: LogicalPlan) -> LogicalPlan {
+/// Removes aliases below a branch filter, but refuses to remove projections.
+///
+/// A projection may compute new values, so dropping it can make branches with
+/// different results appear equivalent to `unions_to_filter`.
+fn strip_passthrough_nodes(mut plan: LogicalPlan) -> Option<LogicalPlan> {
loop {
plan = match plan {
- LogicalPlan::Projection(Projection { input, .. }) => {
- Arc::unwrap_or_clone(input)
- }
+ LogicalPlan::Projection(_) => return None,
Review Comment:
Thanks, keeping the filter input intact makes sense. I’ll update the
implementation and add tests over the next day or two.
I’ll also check whether different underlying sources with otherwise
identical scan metadata can end up in the same GroupKey, since TableScan::eq
does not compare source. I haven’t reproduced this yet; I’ll investigate and
report back.
--
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]
