IMPALA-4877: fix precedence of unary -/+ Currently, expressions such as "-2 / 3" parse as "-(2 / 3)". In practice this usually doesn't cause differences for most common expressions. However, it does interact with a heuristic that changes decimal expression types to double when one operand is non-decimal (see JIRA). For example, before this fix,
typeof(2.0/3.0) = DECIMAL typeof(-2.0/3.0) = DOUBLE because the latter expression parsed as "-1 * (2.0 / 3.0)". With this fix, both expressions result in a DECIMAL. Technically this is a compatibility breaking change but it seems like no one would expect the current behavior so I think we should fix it. Let me know if you disagree. Change-Id: I39cf388ae6e37e1b1e12ddea5fd3878c9c2620d1 Reviewed-on: http://gerrit.cloudera.org:8080/6044 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/09f32d40 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/09f32d40 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/09f32d40 Branch: refs/heads/master Commit: 09f32d406bfc3976b8312a7dcd273b7b0679ffbb Parents: c848074 Author: Dan Hecht <[email protected]> Authored: Thu Feb 16 16:42:41 2017 -0800 Committer: Impala Public Jenkins <[email protected]> Committed: Wed Mar 1 04:18:14 2017 +0000 ---------------------------------------------------------------------- be/src/exprs/expr-test.cc | 17 +++++++++++++++++ fe/src/main/cup/sql-parser.cup | 4 ++++ 2 files changed, 21 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/09f32d40/be/src/exprs/expr-test.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/expr-test.cc b/be/src/exprs/expr-test.cc index 37e816d..e9dbd76 100644 --- a/be/src/exprs/expr-test.cc +++ b/be/src/exprs/expr-test.cc @@ -1392,6 +1392,18 @@ DecimalTestCase decimal_cases[] = { { "cast(111111.1111 as decimal(20, 10)) / cast(.7777 as decimal(38, 38))", {{ true, 0, 38, 38 }, { false, 142871429985, 38, 6 }}}, + { "2.0 / 3.0", + {{ false, 6666, 6, 4}, + { false, 666666, 8, 6}}}, + { "-2.0 / 3.0", + {{ false, -6666, 6, 4}, + { false, -666666, 8, 6}}}, + { "2.0 / -3.0", + {{ false, -6666, 6, 4}, + { false, -666666, 8, 6}}}, + { "-2.0 / -3.0", + {{ false, 6666, 6, 4}, + { false, 666666, 8, 6}}}, // Test modulo operator { "cast(1.23 as decimal(8,2)) % cast(1 as decimal(10,3))", {{ false, 230, 9, 3 }}}, { "cast(1 as decimal(38,0)) % cast(.2 as decimal(38,1))", {{ false, 0, 38, 1 }}}, @@ -4028,12 +4040,17 @@ TEST_F(ExprTest, UnaryOperators) { TestValue("- -1", TYPE_TINYINT, 1); TestValue("+-1", TYPE_TINYINT, -1); TestValue("++1", TYPE_TINYINT, 1); + TestValue("~1", TYPE_TINYINT, -2); TestValue("+cast(1. as float)", TYPE_FLOAT, 1.0f); TestValue("+cast(1.0 as float)", TYPE_FLOAT, 1.0f); TestValue("-cast(1.0 as float)", TYPE_DOUBLE, -1.0); TestValue("1 - - - 1", TYPE_SMALLINT, 0); + + // IMPALA-4877: Verify that unary minus has high precedence and is integrated into + // literals. + TestValue("-1 & 8", TYPE_TINYINT, 8); } // TODO: I think a lot of these casts are not necessary and we should fix this http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/09f32d40/fe/src/main/cup/sql-parser.cup ---------------------------------------------------------------------- diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup index 867850d..8d0f739 100644 --- a/fe/src/main/cup/sql-parser.cup +++ b/fe/src/main/cup/sql-parser.cup @@ -273,6 +273,7 @@ terminal terminal COLON, SEMICOLON, COMMA, DOT, DOTDOTDOT, STAR, LPAREN, RPAREN, LBRACKET, RBRACKET, DIVIDE, MOD, ADD, SUBTRACT; +terminal UNARYSIGN; // Placeholder terminal for unary -/+ terminal BITAND, BITOR, BITXOR, BITNOT; terminal EQUAL, NOT, NOTEQUAL, LESSTHAN, GREATERTHAN; terminal FACTORIAL; // Placeholder terminal for postfix factorial operator @@ -513,6 +514,7 @@ precedence left EQUAL, NOTEQUAL, LESSTHAN, GREATERTHAN, KW_FROM, KW_DISTINCT; precedence left ADD, SUBTRACT; precedence left STAR, DIVIDE, MOD, KW_DIV; precedence left BITAND, BITOR, BITXOR, BITNOT; +precedence left UNARYSIGN; precedence left FACTORIAL; precedence left KW_ORDER, KW_BY, KW_LIMIT; precedence left LPAREN, RPAREN; @@ -2608,8 +2610,10 @@ sign_chain_expr ::= new NumericLiteral(BigDecimal.valueOf(-1)), e); } :} + %prec UNARYSIGN | ADD expr:e {: RESULT = e; :} + %prec UNARYSIGN ; expr ::=
