kosiew commented on code in PR #24000:
URL: https://github.com/apache/datafusion/pull/24000#discussion_r3689120109
##########
datafusion/physical-plan/src/filter.rs:
##########
@@ -401,8 +401,11 @@ impl FilterExec {
}
};
- let total_byte_size =
- input_total_byte_size.with_estimated_selectivity(selectivity);
+ let total_byte_size = if is_infeasible {
Review Comment:
Nice catch making `total_byte_size` exact for the infeasible case. I think
there's still one path that misses the same invariant though.
`FilterExec` can also prove an empty result when the input already has
`num_rows: Exact(0)`. For example, with a satisfiable predicate, `num_rows`
stays `Exact(0)` and `cap_at_rows` now correctly produces exact zero row
bounded column stats. However, this line still returns
`input_total_byte_size.with_estimated_selectivity(selectivity)`, so
`total_byte_size` remains `Absent` or `Inexact`.
Could we key this off the computed `num_rows == Precision::Exact(0)` (or
another shared empty output predicate) instead of only `is_infeasible`? That
would preserve the exact empty result invariant regardless of how the filter
proves the output is empty.
Could you also add a regression covering an input with `num_rows: Exact(0)`
and `total_byte_size: Absent` or `Inexact`?
--
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]
