Taras Bobrovytsky 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: (5 comments) http://gerrit.cloudera.org:8080/#/c/8398/3/be/src/exprs/decimal-functions-ir.cc File be/src/exprs/decimal-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/8398/3/be/src/exprs/decimal-functions-ir.cc@119 PS3, Line 119: Decimal16Value > Do we always need Decimal16? Or should we switch on ByteSize() like the oth Yes, the result is a decimal with precision 38, so it always requires 16 bytes. 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'], > I wonder if we should keep the functionality around under another alias in Ok, are you suggesting to leave the code as is right now and add "fround" when we get rid of decimal_v1? Also, added comment. http://gerrit.cloudera.org:8080/#/c/8398/3/common/function-registry/impala_functions.py@289 PS3, Line 289: [['round','dround','round_v1','dround_v1'], > This function is used both decimal_v1/v2 modes. Done http://gerrit.cloudera.org:8080/#/c/8398/1/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java: http://gerrit.cloudera.org:8080/#/c/8398/1/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@407 PS1, Line 407: Type == nu > situation This comment was modified in patch 2. http://gerrit.cloudera.org:8080/#/c/8398/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java: http://gerrit.cloudera.org:8080/#/c/8398/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@2439 PS3, Line 2439: AnalyzesOk("select round(cast(1.123 as double), int_col) from functional.alltypes", > add a positive test with a non-NULL constant second argument to both v1/v2 Done -- 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 <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 15 Nov 2017 00:35:41 +0000 Gerrit-HasComments: Yes
