simonvandel commented on PR #23548: URL: https://github.com/apache/datafusion/pull/23548#issuecomment-4990958946
Thanks @alamb After merging in your fix, CI started to fail because the `intersect()` test in datafusion/optimizer/tests/optimizer_integration.rs that https://github.com/apache/datafusion/pull/22903 improved, regressed. I think this happens because we are not adequately tracking that functional dependencies through group-by expressions. I **think** that we could mark **nullable = true** here https://github.com/apache/datafusion/blob/a287c0a959310a6a52cafaf9492adc65d0755037/datafusion/common/src/functional_dependencies.rs#L503 since a group-by considers Nulls equal so there can never be more than 1 null value after the group-by, but not 100% sure. I pushed https://github.com/apache/datafusion/pull/23548/commits/9601db86a9f052fbea3dbd5eed8f43ca0dae28da to restore the snapshot to before https://github.com/apache/datafusion/pull/22903. Would you be OK to merge the PR as-is now, although it regresses the optimization that https://github.com/apache/datafusion/pull/22903 added? Perhaps we could add a follow-up issue for better group-by tracking to unlock the regression of the https://github.com/apache/datafusion/pull/22903 optimization -- 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]
