-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Greatest/Least Function Check NaN on REAL data types? #22391
Comments
We're changing the java presto function to follow our new nan definition, so we will be removing this check and treating NaN as greatest (I have a very much work in progress branch here: https://github.com/prestodb/presto/pull/22386/files). I was hoping to get this mostly done this week, but now I realized i'm oncall, so it may get delayed a week. |
Definitely whatever you do for DOUBLE you should also do for REAL (float) types. |
Just found this as the new definition of NaN under the issue #21936. I will be using it as the reference. edit: |
Yes the new definition is the same as the one proposed for velox as well facebookincubator/velox#7237. The new definition will define NaN=NaN as true and also consider NaN as largest for all ordering operations (>, max, sorting, etc.). That wiki page reflects the old behavior (which is still the current behavior) so we can track what's changing. It does not yet reflect the new behavior we are moving towards. |
#9308) Summary: Refactor the greatest/least functions using simple function API. Also, add support for NaN comparisons for DOUBLE and REAL. NaN is the biggest according to prestodb/presto#22391 Fixes #3728 Pull Request resolved: #9308 Reviewed By: xiaoxmeng Differential Revision: D55793910 Pulled By: mbasmanova fbshipit-source-id: c389bad91197f00ced549d816a15efab5a2dd910
facebookincubator#9308) Summary: Refactor the greatest/least functions using simple function API. Also, add support for NaN comparisons for DOUBLE and REAL. NaN is the biggest according to prestodb/presto#22391 Fixes facebookincubator#3728 Pull Request resolved: facebookincubator#9308 Reviewed By: xiaoxmeng Differential Revision: D55793910 Pulled By: mbasmanova fbshipit-source-id: c389bad91197f00ced549d816a15efab5a2dd910
Hi,
I am working on Greatest/Least function for PrestoSQL in Velox. In current PrestoDB implementations, Greatest/Least Function only checks NaN on DOUBLE data types.
https://github.com/prestodb/presto/blob/master/presto-main/src/main/java/com/facebook/presto/operator/scalar/AbstractGreatestLeast.java#L124
https://github.com/prestodb/presto/blob/master/presto-main/src/main/java/com/facebook/presto/operator/scalar/AbstractGreatestLeast.java#L160
Intuitively, might the same check also apply to REAL data type?
I want to make sure that Greatest/Least function in Velox has the same behavior as the one in PrestoDB.
Thank you!
The text was updated successfully, but these errors were encountered: