buraksenn commented on PR #21473: URL: https://github.com/apache/datafusion/pull/21473#issuecomment-4216297949
> > Thanks @asolimando for the review. I've checked this again. To make it generic I've missed that I can actually make temporal types work with interval arithmetic analysis since they are `i64` backed as well. > > Sounds great! > > > Thus answering all: > > > > * I'm kinda afraid into getting this kind of complex behavior since I may not know what can be a side effect. Maybe I can add it to this PR and see the reviews > > I think it's enough to add a test to see what happens, before we would have been returning the same NDV value for the input column anyway, I don't think we are doing worse than that, let's add a test and possibly file an issue to not lose the context we built. > > > * I've missed this totally will push to this branch. > > * For this, adding support for temporal types seem to be better as I've missed whiel trying to make it generic so I'll open a PR parallel to this one. > > No worries, making it type agnostic was a good call IMO, but what can be handled with interval arithmetic should do it as we can benefit from all the handling of edge cases like the "incoherent predicate" above and possibly others I couldn't think of. > > > I'm working on both changes now and will handle this in the upcoming hours > > No worries, feel free to ping me if you need a review, happy to take a look, thanks for working on this and helping out on so many stats-related tasks :) Thanks @asolimando I've opened #21520 temporal types I think it is very straightforward and contained PR and only left non-arithmetic supporting changes here. -- 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]
