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: (2 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'], > * Yes, we want to remove decimal_v1. To me the point of the JIRA was to change the default behaviour of round(), not to prevent users from rounding to a floating point number with a non-constant argument if that's what they want to do. That seems to be going further than the JIRA really requires. I don't entirely understand what you mean by "the alias does not solve the limitations of our V2 functions". Do you just mean that the two versions have different pros and cons? If so, then it seems like we should let users make an informed decision about which function they actually want. Or do you mean that we should somehow add support for non-constant arguments to the V2 version? http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py@285 PS3, Line 285: [['round_v1','dround_v1'], > I agree with Alex. I don't think it makes much sense to have a round() func It's totally plausible to me that someone would write a query with a non-constant precision for round(). E.g. they have a case statement that has some logic to choose precision based on a column value. I wouldn't underestimate the creativity of people writing complex SQL statements. If we completely remove the ability to do that then there's no workaround once we remove decimal_v1. If you want to take that risk that's fine, but I don't see a compelling reason not to leave a floating point version accessible as a mitigation. -- 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 01:48:15 +0000 Gerrit-HasComments: Yes
