Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8398 )
Change subject: IMPALA-3436: Return a decimal when rounding a double ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py File common/function-registry/impala_functions.py: http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py@285 PS3, Line 285: [['round_v1','dround_v1'], > My point about "the alias does not solve the limitations of our V2 function But is that even possible to implement given that we have to statically determine the return precision and scale for decimal? http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py@285 PS3, Line 285: [['round_v1','dround_v1'], > I agree could be legit uses cases for non-const precisions. I don't think there's anything inherently wrong with a version of round() that returns the nearest floating point number to the exact answer. That's the nature of all floating point calculations. It's clearly not the right tool for 99% of queries, but the question should be whether it's ever useful. I think there are some fraction of queries where it probably is the best tool available. Generally I don't think we should remove such things without ensuring that an alternative is available, unless it's really unavoidable. Otherwise we put users in a bind when they want to upgrade because they need to rethink their queries totally instead of just adjusting them. There is some precedent for this version of round(), e.g. the below systems all return floating point values if given a floating point input: https://docs.snowflake.net/manuals/sql-reference/functions/round.html https://docs.microsoft.com/en-us/sql/t-sql/functions/round-transact-sql http://docs.aws.amazon.com/redshift/latest/dg/r_ROUND.html http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py@285 PS3, Line 285: [['round_v1','dround_v1'], > Just to clarify. I think what Alex is saying is that the proper fix is so t Right, but is that even possible? -- To view, visit http://gerrit.cloudera.org:8080/8398 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I240ec70da7996bbfefcf6438eba4dff8d33d7bd6 Gerrit-Change-Number: 8398 Gerrit-PatchSet: 3 Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 16 Nov 2017 16:22:34 +0000 Gerrit-HasComments: Yes
