Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/9930 )
Change subject: WIP: IMPALA-6518,IMPALA-6340: Check that decimal types are compatible in FE ...................................................................... Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java: http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@377 PS8, Line 377: Analyzer analyzer, AnalyticWindow.Boundary boundary) throws AnalysisException { > No need to pass analyzer. Done http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java File fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java: http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java@217 PS8, Line 217: Preconditions.checkState(!t0.isDecimal() && !t1.isDecimal()); > remove Done http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/Expr.java@429 PS8, Line 429: * If strictDecimal is true, the function will throw an exception if it is not possible > This isn't accurate because we will still allow decimal->double conversions Done http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/StatementBase.java File fe/src/main/java/org/apache/impala/analysis/StatementBase.java: http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/analysis/StatementBase.java@178 PS8, Line 178: * message. > Needs comment for 'strictDecimal' Done http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/ScalarType.java File fe/src/main/java/org/apache/impala/catalog/ScalarType.java: http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/ScalarType.java@433 PS8, Line 433: * is INVALID_TYPE. > Especially these very visible and heavily used functions need a comment for Done http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/Type.java File fe/src/main/java/org/apache/impala/catalog/Type.java: http://gerrit.cloudera.org:8080/#/c/9930/8/fe/src/main/java/org/apache/impala/catalog/Type.java@293 PS8, Line 293: * If strictDecimal is true, only consider casts that result in no loss of information > This is a nice description! I suggest replicating it in more places. Done -- To view, visit http://gerrit.cloudera.org:8080/9930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id406f4189e01a909152985fabd5cca7a1527a568 Gerrit-Change-Number: 9930 Gerrit-PatchSet: 8 Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-Comment-Date: Thu, 26 Apr 2018 21:07:09 +0000 Gerrit-HasComments: Yes
