Browse Source

SQL: Allow long literals (#31777)

Fix bug that caused integral literals to be only Integer (rejecting
Long). This commit fixes that and picks either an Integer or Long based
on size.
Costin Leau 7 years ago
parent
commit
07470c950b

+ 8 - 2
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java

@@ -452,8 +452,14 @@ abstract class ExpressionBuilder extends IdentifierBuilder {
     @Override
     public Object visitIntegerLiteral(IntegerLiteralContext ctx) {
         BigDecimal bigD = new BigDecimal(ctx.getText());
-        // TODO: this can be improved to use the smallest type available
-        return new Literal(source(ctx), bigD.longValueExact(), DataType.INTEGER);
+
+        long value = bigD.longValueExact();
+        DataType type = DataType.LONG;
+        // try to downsize to int if possible (since that's the most common type)
+        if ((int) value == value) {
+            type = DataType.INTEGER;
+        }
+        return new Literal(source(ctx), value, type);
     }
 
     @Override

+ 4 - 2
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java

@@ -154,7 +154,7 @@ public abstract class DataTypeConversion {
             return Conversion.INTEGER_TO_LONG;
         }
         if (from == BOOLEAN) {
-            return Conversion.BOOL_TO_INT; // We emit an int here which is ok because of Java's casting rules
+            return Conversion.BOOL_TO_LONG;
         }
         if (from.isString()) {
             return Conversion.STRING_TO_LONG;
@@ -407,7 +407,9 @@ public abstract class DataTypeConversion {
 
         NUMERIC_TO_BOOLEAN(fromLong(value -> value != 0)),
         STRING_TO_BOOLEAN(fromString(DataTypeConversion::convertToBoolean, "Boolean")),
-        DATE_TO_BOOLEAN(fromDate(value -> value != 0));
+        DATE_TO_BOOLEAN(fromDate(value -> value != 0)),
+
+        BOOL_TO_LONG(fromBool(value -> value ? 1L : 0L));
 
         private final Function<Object, Object> converter;
 

+ 59 - 0
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java

@@ -0,0 +1,59 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.sql.parser;
+
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.Literal;
+import org.elasticsearch.xpack.sql.expression.function.scalar.arithmetic.Neg;
+import org.elasticsearch.xpack.sql.type.DataType;
+
+public class ExpressionTests extends ESTestCase {
+
+    private final SqlParser parser = new SqlParser();
+
+    public void testLiteralLong() throws Exception {
+        Expression lt = parser.createExpression(String.valueOf(Long.MAX_VALUE));
+        assertEquals(Literal.class, lt.getClass());
+        Literal l = (Literal) lt;
+        assertEquals(Long.MAX_VALUE, l.value());
+        assertEquals(DataType.LONG, l.dataType());
+    }
+
+    public void testLiteralLongNegative() throws Exception {
+        // Long.MIN_VALUE doesn't work since it is being interpreted as negate positive.long which is 1 higher than Long.MAX_VALUE
+        Expression lt = parser.createExpression(String.valueOf(-Long.MAX_VALUE));
+        assertEquals(Neg.class, lt.getClass());
+        Neg n = (Neg) lt;
+        assertTrue(n.foldable());
+        assertEquals(-Long.MAX_VALUE, n.fold());
+        assertEquals(DataType.LONG, n.dataType());
+    }
+
+    public void testLiteralInteger() throws Exception {
+        Expression lt = parser.createExpression(String.valueOf(Integer.MAX_VALUE));
+        assertEquals(Literal.class, lt.getClass());
+        Literal l = (Literal) lt;
+        assertEquals(Integer.MAX_VALUE, l.value());
+        assertEquals(DataType.INTEGER, l.dataType());
+    }
+
+    public void testLiteralIntegerWithShortValue() throws Exception {
+        Expression lt = parser.createExpression(String.valueOf(Short.MAX_VALUE));
+        assertEquals(Literal.class, lt.getClass());
+        Literal l = (Literal) lt;
+        assertEquals(Integer.valueOf(Short.MAX_VALUE), l.value());
+        assertEquals(DataType.INTEGER, l.dataType());
+    }
+
+    public void testLiteralIntegerWithByteValue() throws Exception {
+        Expression lt = parser.createExpression(String.valueOf(Byte.MAX_VALUE));
+        assertEquals(Literal.class, lt.getClass());
+        Literal l = (Literal) lt;
+        assertEquals(Integer.valueOf(Byte.MAX_VALUE), l.value());
+        assertEquals(DataType.INTEGER, l.dataType());
+    }
+}

+ 12 - 5
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java

@@ -45,8 +45,8 @@ public class DataTypeConversionTests extends ESTestCase {
         {
             Conversion conversion = DataTypeConversion.conversionFor(DataType.BOOLEAN, to);
             assertNull(conversion.convert(null));
-            assertEquals(1, conversion.convert(true));
-            assertEquals(0, conversion.convert(false));
+            assertEquals(1L, conversion.convert(true));
+            assertEquals(0L, conversion.convert(false));
         }
         Conversion conversion = DataTypeConversion.conversionFor(DataType.KEYWORD, to);
         assertNull(conversion.convert(null));
@@ -141,12 +141,19 @@ public class DataTypeConversionTests extends ESTestCase {
             assertEquals(true, conversion.convert(-10));
             assertEquals(false, conversion.convert(0));
         }
+        {
+            Conversion conversion = DataTypeConversion.conversionFor(DataType.LONG, DataType.BOOLEAN);
+            assertNull(conversion.convert(null));
+            assertEquals(true, conversion.convert(10L));
+            assertEquals(true, conversion.convert(-10L));
+            assertEquals(false, conversion.convert(0L));
+        }
         {
             Conversion conversion = DataTypeConversion.conversionFor(DataType.DOUBLE, DataType.BOOLEAN);
             assertNull(conversion.convert(null));
-            assertEquals(true, conversion.convert(10.0));
-            assertEquals(true, conversion.convert(-10.0));
-            assertEquals(false, conversion.convert(0.0));
+            assertEquals(true, conversion.convert(10.0d));
+            assertEquals(true, conversion.convert(-10.0d));
+            assertEquals(false, conversion.convert(0.0d));
         }
         {
             Conversion conversion = DataTypeConversion.conversionFor(DataType.KEYWORD, DataType.BOOLEAN);