SQL: Fix issue with negative literels and parentheses

Previously when a numeric literal was enclosed in parentheses and then negated, the negation was lost and the number was considered positive, e.g.: `-(5)` was considered as `5` instead of `-5` `- ( (1.28) )` was considered as `1.28` instead of `-1.28` Fixes: elastic#48009
matriv · Oct 16, 2019 · d0a1c2b · d0a1c2b
1 parent 27c24e2
commit d0a1c2b
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 8 deletions.
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java
@@ -528,7 +528,7 @@ public Literal visitInterval(IntervalContext interval) {
 
         TimeUnit leading = visitIntervalField(interval.leading);
         TimeUnit trailing = visitIntervalField(interval.trailing);
-        
+
         // only YEAR TO MONTH or DAY TO HOUR/MINUTE/SECOND are valid declaration
         if (trailing != null) {
             if (leading == TimeUnit.YEAR && trailing != TimeUnit.MONTH) {
@@ -869,11 +869,28 @@ private static Source minusAwareSource(SqlBaseParser.NumberContext ctx) {
         if (parentCtx != null) {
             if (parentCtx instanceof SqlBaseParser.NumericLiteralContext) {
                 parentCtx = parentCtx.getParent();
-                if (parentCtx != null && parentCtx instanceof SqlBaseParser.ConstantDefaultContext) {
+                if (parentCtx instanceof ConstantDefaultContext) {
                     parentCtx = parentCtx.getParent();
-                    if (parentCtx != null && parentCtx instanceof SqlBaseParser.ValueExpressionDefaultContext) {
+                    if (parentCtx instanceof ValueExpressionDefaultContext) {
                         parentCtx = parentCtx.getParent();
-                        if (parentCtx != null && parentCtx instanceof SqlBaseParser.ArithmeticUnaryContext) {
+
+                        // Skip parentheses, e.g.: - (( (2.15) ) )
+                        while (parentCtx instanceof PredicatedContext) {
+                            parentCtx = parentCtx.getParent();
+                            if (parentCtx instanceof SqlBaseParser.BooleanDefaultContext) {
+                                parentCtx = parentCtx.getParent();
+                            }
+                            if (parentCtx instanceof SqlBaseParser.ExpressionContext) {
+                                parentCtx = parentCtx.getParent();
+                            }
+                            if (parentCtx instanceof SqlBaseParser.ParenthesizedExpressionContext) {
+                                parentCtx = parentCtx.getParent();
+                            }
+                            if (parentCtx instanceof ValueExpressionDefaultContext) {
+                                parentCtx = parentCtx.getParent();
+                            }
+                        }
+                        if (parentCtx instanceof ArithmeticUnaryContext) {
                             if (((ArithmeticUnaryContext) parentCtx).MINUS() != null) {
                                 return source(parentCtx);
                             }

diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java
@@ -202,26 +202,34 @@ public void testNegativeLiteral() {
         Expression expr = parser.createExpression("- 6");
         assertEquals(Literal.class, expr.getClass());
         assertEquals("- 6", expr.sourceText());
+
+        expr = parser.createExpression("- ( 6 )");
+        assertEquals(Literal.class, expr.getClass());
+        assertEquals("- ( 6 )", expr.sourceText());
+
+        expr = parser.createExpression("- ( ( (1.25) )    )");
+        assertEquals(Literal.class, expr.getClass());
+        assertEquals("- ( ( (1.25) )    )", expr.sourceText());
     }
 
     public void testComplexArithmetic() {
-        String sql = "-(((a-2)-(-3))+b)";
+        String sql = "-(((a-2)- (( - ( (  3)  )) )  )+ b)";
         Expression expr = parser.createExpression(sql);
         assertEquals(Neg.class, expr.getClass());
         Neg neg = (Neg) expr;
         assertEquals(sql, neg.sourceText());
         assertEquals(1, neg.children().size());
         assertEquals(Add.class, neg.children().get(0).getClass());
         Add add = (Add) neg.children().get(0);
-        assertEquals("((a-2)-(-3))+b", add.sourceText());
+        assertEquals("((a-2)- (( - ( (  3)  )) )  )+ b", add.sourceText());
         assertEquals(2, add.children().size());
         assertEquals("?b", add.children().get(1).toString());
         assertEquals(Sub.class, add.children().get(0).getClass());
         Sub sub1 = (Sub) add.children().get(0);
-        assertEquals("(a-2)-(-3)", sub1.sourceText());
+        assertEquals("(a-2)- (( - ( (  3)  )) )", sub1.sourceText());
         assertEquals(2, sub1.children().size());
         assertEquals(Literal.class, sub1.children().get(1).getClass());
-        assertEquals("-3", sub1.children().get(1).sourceText());
+        assertEquals("- ( (  3)  )", sub1.children().get(1).sourceText());
         assertEquals(Sub.class, sub1.children().get(0).getClass());
         Sub sub2 = (Sub) sub1.children().get(0);
         assertEquals(2, sub2.children().size());