Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12001 to look at the new patch set (#6). Change subject: IMPALA-7902: NumericLiteral fixes, refactoring ...................................................................... IMPALA-7902: NumericLiteral fixes, refactoring The work to clean up the rewriter logic must start with a stable AST, which must start with sprucing up some issues with the leaf nodes. This CR tackles the NumericLiteral used to hold numbers. IMPALA-7896: Literals should not need explicit analyze step Partial fix: removes the need to analyze a numeric literal: analyze() is a no-op. This eliminates the need to do a "fake" analysis with a null analyzer: numeric literals are now created analyzed. This is useful because the catalog module creates numeric literals outside of a query (and outside of an analyzer.) Since a literal is (mostly) immutable (except for type), the constructor now sets the type and cost, then marks the node as analyzed. A later call to analyze() has nothing to do. Code that created and dummy-analyzed numeric literals changed to use static create() methods resulting in simpler literal creation. IMPALA-7886: NumericLiteral constructor fails to round values to Decimal type IMPALA-7887: NumericLiteral fails to detect numeric overflow IMPALA-7888: Incorrect NumericLiteral overflow checks for FLOAT, DOUBLE IMPALA-7891: Analyzer does not detect numeric overflow in CAST IMPALA-7894: Parser does not catch double overflow These are all caused by the somewhat messy state of the numeric check code after years of incremental changes. This patch centralizes all checks into a series of constants and methods so that they are uniform. All values are set in constructors which now all uniformly check that the value is legal for the type. (Explicit) cast operations verify that the cast is valid. Multiple semi-parallel versions of the same logic is replaced by calls to a single implementation. The numeric checks now follow the SQL standard which says that implementations should fail if a cast would trucate most significant digits, but round when truncating the least significant. In this commit, the rules apply only to numeric literals created outside of the parser. The existing analysis rules are unchanged. IMPALA-7865: Repeated type widening of arithmetic expressions Partial fix. Replaces the "is explicit cast" flag in the numeric literal with the explicit type. This allows reseting an implicit type back to the explciit type if an arithmetic expression is analyzed multiple times. A later patch will feed this type information into the type inference mechanism to complete the fix. Finally, adds a set of new exceptions that begin to unify error reporting. These handle casts (SqlCastException), value validation (InvalidValueException) and unsupported features (UnsupportedFeatureException.) These all derive from AnalysisException for backward compatibility. Tests use the new exceptions to check for expected errors rather than parsing text strings (which tend to change.) Testing: * Added unit tests just for numeric literals. Refactored code to simplify the tests. * Added a test case for the obscure case in Decimal V1 of an implicit cast overflow. * Ran all FE tests. Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 --- M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java M fe/src/main/java/org/apache/impala/analysis/Expr.java M fe/src/main/java/org/apache/impala/analysis/LiteralExpr.java M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java M fe/src/main/java/org/apache/impala/catalog/ScalarFunction.java M fe/src/main/java/org/apache/impala/catalog/ScalarType.java A fe/src/main/java/org/apache/impala/common/InvalidValueException.java A fe/src/main/java/org/apache/impala/common/SqlCastException.java A fe/src/main/java/org/apache/impala/common/UnsupportedFeatureException.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java D fe/src/test/java/org/apache/impala/analysis/ExprTest.java A fe/src/test/java/org/apache/impala/analysis/LiteralExprTest.java A fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java M fe/src/test/java/org/apache/impala/catalog/HdfsPartitionTest.java 18 files changed, 1,328 insertions(+), 271 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/12001/6 -- To view, visit http://gerrit.cloudera.org:8080/12001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I484600747b2871d3a6fe9153751973af9a8534f2 Gerrit-Change-Number: 12001 Gerrit-PatchSet: 6 Gerrit-Owner: Paul Rogers <par0...@yahoo.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <par0...@yahoo.com>