-
Notifications
You must be signed in to change notification settings - Fork 6
Update expected output for coalsece short-circuit #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| # Datafusion - Datafusion expected results: | ||
| # Datafusion - Expected - 0 | ||
| query error DataFusion error: Arrow error: Divide by zero error | ||
| query I rowsort label-3927 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given these are in files "good" I think they should be succeeding
I confirmed (for this one) that this is the answer that postgres gives
postgres=# SELECT + CAST ( - 13 AS INTEGER ) / COALESCE ( - + 14, + - NULLIF ( - COUNT ( * ), 19 * - - NULLIF ( 12, + MIN ( ALL - - ( + - 11 ) ) ) / + - 4 + - 84 + + 36 - - 57 + - + 68 * NULLIF ( + 57, + 45 ) ) * + 24 / CAST ( - 0 AS INTEGER ) / + 90, - 93 ) col0;
col0
------
0
(1 row)| -143550 | ||
|
|
||
| # Datafusion - Datafusion expected results: | ||
| # Datafusion - Expected - -29 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is actually kind of cool -- @Omega359 's script left in the expected results, so we can see that after this change DataFusion is now actually generating the expected results
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case,
-29
-900
adriangb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I did not manually check every single result but I see that all of the ones I looked at now match the original expected value - amazing!
|
Thanks @adriangb |
Now that Coalesce does lazy evaluation, more queries will succeed. See this PR for more details