CuteChuanChuan commented on PR #27307: URL: https://github.com/apache/datafusion/pull/27307#issuecomment-6800090508
> [@CuteChuanChuan](https://github.com/CuteChuanChuan) Thanks for putting this in. I reviewed the updated conditional and the test cases, ran the tests with and without the change, had claude conduct its own review, and ran the CI test jobs locally on the PR. > > This looks good apart from one missing test. There should be a test that covers > > SELECT count(1) FILTER (WHERE c > 0) FROM (SELECT count(1) AS c FROM t); > > On main right now it incorrectly returns 5 instead of 1, with no error. This change fixes that, but the tests should reflect it. Something like this in the new block: > > ``` > query I > SELECT count(1) FILTER (WHERE c > 0) FROM (SELECT count(1) AS c FROM t); > ---- > 1 > ``` @HairstonE Thanks for pointing this out! Good catch! Added the count(1) case in 8ac0d1e. It returns 5 on main with no error and 1 with this change. -- 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]
