Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4370: Divide and modulo result types for DECIMAL version V2 ......................................................................
Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/5952/8/fe/src/main/java/org/apache/impala/analysis/TypesUtil.java File fe/src/main/java/org/apache/impala/analysis/TypesUtil.java: Line 100: ArithmeticExpr.Operator op, TQueryOptions queryOptions) throws AnalysisException { instead of queryoptions, how about just passing a bool in here? narrower interfaces are easier to understand. Line 152: * on whether DECIMAL version 1 or DECIMAL version 2 is enabled. since we're going to toss out the v1 behavior when we sunset cdh5, leave a bunch of todos in place so we know what exactly needs to change? Line 233: * precision. But an algorithm of reducing scale to a minimum reduction of 6 is "to a minimum of 6" http://gerrit.cloudera.org:8080/#/c/5952/8/fe/src/main/java/org/apache/impala/catalog/ScalarType.java File fe/src/main/java/org/apache/impala/catalog/ScalarType.java: Line 138: public static ScalarType createDecimalTypeClipPrecScale(int precision, int scale) { how about simply createClippedDecimalType()? or truncated instead of clipped. same for createAdjustedDecimalType. same checkstates as in 'adjusted' below? -- To view, visit http://gerrit.cloudera.org:8080/5952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I83e7f7787edfa4b4bddc25945090542a0e90881b Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
