asolimando commented on issue #25571:
URL: https://github.com/apache/datafusion/issues/25571#issuecomment-5775493251

   > cc [@asolimando](https://github.com/asolimando), got bitten by this while 
making some stats improvement reports. Digging in the PR history, I did not 
find a good reason for us to maintain diverging implementations for statistics 
in the built-in StatisticsProviders VS the stats provided in the ExecutionPlans 
themselves, but maybe I missed something?
   
   In the original design for https://github.com/apache/datafusion/issues/21443 
proposed the `StatisticsRegistry` outside the built-in path to avoid breaking 
changes, over time we agreed to have it in the default path (using 
`ExecutionPlan`'s defaults in absence of providers) in 
https://github.com/apache/datafusion/pull/23651, so "built-in providers" 
referred to optional the `StatisticsRegistry` itself, not for statistics 
propagation, as there was no fallback mechanism in the `StatisticsRegistry` to 
the statistics computation from `ExecutionPlan` nodes.
   
   After https://github.com/apache/datafusion/pull/23651, the name should have 
been changed as it's indeed confusing, apologies for having missed that.
   
   I see two possibilities for the future of these providers (more or less 
along the lines of what you suggested above):
   1. trimming them down as much as possible (there are some overlaps with 
`ExecutionPlan` built-ins for historical reasons), and use them to provide 
"advanced"/alternative statistics for some of the nodes (e.g., 
https://github.com/apache/datafusion/blob/714956b3a8aed43e8abd27ed400710417d231982/datafusion/physical-plan/src/operator_statistics/mod.rs#L623
 which is based on the 
https://en.wikipedia.org/wiki/Coupon_collector%27s_problem)
   2. remove them and leave custom providers (user-defined) and `ExecutionPlan` 
built-ins
   
   Now that stats-bench has landed, it's probably easier to integrate 
improvements into `ExecutionPlan` built-ins (and 1. is less needed), as we have 
a way to show that statistics estimation is getting closer to the runtime 
statistics, while before it was harder to get such improvements in (see 
https://github.com/apache/datafusion/issues/21120#issuecomment-4114949587 for 
instance).


-- 
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]

Reply via email to