Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Impala 2.8.0
    • Fix Version/s: Impala 2.9.0
    • Component/s: Frontend
    • Labels:
      None

      Description

      Impala's parser gives unary - and unary + the same precedence as binary + and -, which has lower precedence than other binary operators, such as / and *. This leads to strange typing, especially apparent due to the heuristics of Expr.convertNumericLiteralsFromDecimal() (see also IMPALA-3437).

      For example, division between positive decimal numbers performs decimal divide, as expected:

      > select typeof(1.0 / 3.0);
      +-------------------+
      | typeof(1.0 / 3.0) |
      +-------------------+
      | DECIMAL(6,4)      |
      +-------------------+
      > select 1.0 / 3.0;
      +-----------+
      | 1.0 / 3.0 |
      +-----------+
      | 0.3333    |
      +-----------+
      

      But if you add unary minus, things get weird:

      > select typeof(-1.0 / 3.0);
      +------------------------+
      | typeof(-1 * 1.0 / 3.0) |
      +------------------------+
      | DOUBLE                 |
      +------------------------+
      > select -1.0 / 3.0;
      +---------------------+
      | -1 * 1.0 / 3.0      |
      +---------------------+
      | -0.3333333333333333 |
      +---------------------+
      

      Other examples involving integers:

      > select -1 & 3;
      +------------+
      | -1 * 1 & 3 |
      +------------+
      | -1         |
      +------------+
      

      A workaround is to use parenthesis to force the correct parsing:

      > select (-1.0) / 3.0;
      +--------------+
      | (-1.0) / 3.0 |
      +--------------+
      | -0.3333      |
      +--------------+
      > select (-1) & 3;
      +----------+
      | (-1) & 3 |
      +----------+
      | 3        |
      +----------+
      

      The fix is to define higher precedence levels for unary minus/plus. Something like:

      diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup
      index 33f0591..d3af3be 100644
      --- a/fe/src/main/cup/sql-parser.cup
      +++ b/fe/src/main/cup/sql-parser.cup
      @@ -272,7 +272,7 @@ terminal
         KW_WHERE, KW_WITH;
      
       terminal COLON, SEMICOLON, COMMA, DOT, DOTDOTDOT, STAR, LPAREN, RPAREN, LBRACKET,
      -  RBRACKET, DIVIDE, MOD, ADD, SUBTRACT;
      +  RBRACKET, DIVIDE, MOD, ADD, SUBTRACT, UMINUS, UADD;
       terminal BITAND, BITOR, BITXOR, BITNOT;
       terminal EQUAL, NOT, NOTEQUAL, LESSTHAN, GREATERTHAN;
       terminal FACTORIAL; // Placeholder terminal for postfix factorial operator
      @@ -511,6 +511,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 UMINUS, UADD;
       precedence left FACTORIAL;
       precedence left KW_ORDER, KW_BY, KW_LIMIT;
       precedence left LPAREN, RPAREN;
      @@ -2591,8 +2592,10 @@ sign_chain_expr ::=
                                         new NumericLiteral(BigDecimal.valueOf(-1)), e);
           }
         :}
      +  %prec UMINUS
         | ADD expr:e
         {: RESULT = e; :}
      +  %prec UADD
         ;
      
       expr ::=
      

        Attachments

          Activity

            People

            • Assignee:
              dhecht Dan Hecht
              Reporter:
              dhecht Dan Hecht
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: