xudong963 commented on PR #15852: URL: https://github.com/apache/datafusion/pull/15852#issuecomment-2838052185
> I also think you should add a note to the [upgrade guide](https://github.com/apache/datafusion/blob/main/docs/source/library-user-guide/upgrading.md) for this change Added to https://github.com/apache/datafusion/issues/15771 > and I think the compiler may not flag the newly introduced method. Because we marked the old statistics method as deprecated, the compiler will mention users and navigate them to the new method. > Something that might be a better UX for downstream users might be to NOT provide a default implementation of `partition_statistics` and force them to implement it: Yes, I've experienced this during upgrading DF45, the version introduced a new method for LexOrdering IIRC, and gave it a default implementation; the default isn't suitable for our case, and I didn't notice the new method; it took me a long time to debug. QAQ > But then they would need some way to avoid the boiler plate code 🤔 Also agree, it's a trade-off, given that the compiler will metion as I said above and we'll also metion the change in the upgrade doc, life will be easier. Or after merging the PR, we can implement the new method in all places and then remove the default implemation. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
