Re: [PR] Add short circuit [datafusion]

2025-03-28 Thread via GitHub
alamb commented on code in PR #15462: URL: https://github.com/apache/datafusion/pull/15462#discussion_r2019123741 ## benchmarks/queries/clickbench/README.md: ## @@ -120,13 +122,42 @@ LIMIT 10; ``` Results look like - +``` +-+-+---+--+

Re: [PR] Add short circuit [datafusion]

2025-03-28 Thread via GitHub
acking-you commented on PR #15462: URL: https://github.com/apache/datafusion/pull/15462#issuecomment-2762023923 Hello @alamb, the optimization SQL and documentation related to this PR have been completed, and all tests have passed. We may need to formally verify the performance, but I'm not

Re: [PR] Add short circuit [datafusion]

2025-03-28 Thread via GitHub
acking-you commented on PR #15462: URL: https://github.com/apache/datafusion/pull/15462#issuecomment-2762014991 > You're absolutely right, I got my logic wrong there. Embarrasing! It's okay. You've also taught me a lot. When I first started writing this, I really didn't consider the c

Re: [PR] Add short circuit [datafusion]

2025-03-28 Thread via GitHub
ctsk commented on PR #15462: URL: https://github.com/apache/datafusion/pull/15462#issuecomment-2762008525 You're absolutely right, I got my logic wrong there. Embarrasing! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and us

Re: [PR] Add short circuit [datafusion]

2025-03-28 Thread via GitHub
acking-you commented on PR #15462: URL: https://github.com/apache/datafusion/pull/15462#issuecomment-2761813612 > I think one issue is that the short-circuit logic is not handling cases where the the `rhs` contains NULLs. E.g. `true OR NULL` needs to evaluate to `NULL` After taking a

Re: [PR] Add short circuit [datafusion]

2025-03-28 Thread via GitHub
acking-you commented on PR #15462: URL: https://github.com/apache/datafusion/pull/15462#issuecomment-2761137454 > I think one issue is that the short-circuit logic is not handling cases where the the `rhs` contains NULLs. E.g. `true OR NULL` needs to evaluate to `NULL` Thank you very

Re: [PR] Add short circuit [datafusion]

2025-03-28 Thread via GitHub
ctsk commented on PR #15462: URL: https://github.com/apache/datafusion/pull/15462#issuecomment-2760574284 I think one issue is that the short-circuit logic is not handling cases where the the `rhs` contains NULLs. E.g. `true OR NULL` needs to evaluate to `NULL` -- This is an automated me

Re: [PR] Add short circuit [datafusion]

2025-03-27 Thread via GitHub
acking-you commented on PR #15462: URL: https://github.com/apache/datafusion/pull/15462#issuecomment-2759162126 Some tests failed, so let me take a look at what exactly is going on. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to Git