asolimando commented on code in PR #23651:
URL: https://github.com/apache/datafusion/pull/23651#discussion_r3658552927
##########
datafusion/physical-plan/src/statistics.rs:
##########
@@ -168,12 +216,32 @@ impl StatisticsContext {
);
}
- if let Some(cached) = self.cache.borrow().get(plan, partition) {
- return Ok(Arc::clone(cached));
+ if let Some(cached) = self.cached_statistics(plan, partition) {
+ return Ok(cached);
}
let children = plan.children();
let requests = plan.child_stats_requests(partition);
+ // Resolved for the built-in `statistics_from_inputs` fallback below.
+ let child_statistics = self.resolve_children(plan, &children,
&requests)?;
Review Comment:
It makes sense, it's true that children statistics computation can fail, and
that would be unfortunate for providers that don't even need those stats, so it
makes sense to test the providers first, I have implemented this in
0b6a0213ccd93f7948360036855e04a420831028.
I have added also a test to cover against regressions in this area (child
throws error for statistics, but the provider `Skip`s statistics, so it
succeeds with the updated code).
--
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]
