Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9930 )
Change subject: IMPALA-6518,IMPALA-6340: Check that decimal types are compatible in FE ...................................................................... Patch Set 1: (4 comments) High-level questions/comments. I'm still going through the details. http://gerrit.cloudera.org:8080/#/c/9930/1/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/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@385 PS1, Line 385: rangeExpr.getType(), orderByElements_.get(0).getExpr().getType(), Let's change this to be strict regardless of decimal_v2. My understanding is that we don't support RANGE offset boundaries today, so this code change is not incompatible. See AnalyzeExprsTest.TestAnalyticExprs() http://gerrit.cloudera.org:8080/#/c/9930/1/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/1/fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java@218 PS1, Line 218: false, analyzer.getQueryOptions().isDecimal_v2()); Since this decimalV2 check is so common, let's add a function in Analyzer directly: boolean isDecimalV2() { return getQueryOptions().isDecimal_v2(); } http://gerrit.cloudera.org:8080/#/c/9930/1/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/1/fe/src/main/java/org/apache/impala/catalog/Type.java@292 PS1, Line 292: * If strict is true, only consider casts that result in no loss of precision. Why is the decimalV2 case different than the strict case? http://gerrit.cloudera.org:8080/#/c/9930/1/fe/src/main/java/org/apache/impala/catalog/Type.java@296 PS1, Line 296: Type t1, Type t2, boolean strict, boolean decimal_v2) { decimalV2 (camel-case in Java land) -- 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: 1 Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Comment-Date: Thu, 12 Apr 2018 00:19:22 +0000 Gerrit-HasComments: Yes
