Skip to content

Commit ed599a8

Browse files
costincostin
committed
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. (cherry picked from commit 07470c9) (cherry picked from commit ab534e7)
1 parent 51aecc9 commit ed599a8

File tree

4 files changed

+83
-9
lines changed

4 files changed

+83
-9
lines changed

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -452,8 +452,14 @@ public Object visitDecimalLiteral(DecimalLiteralContext ctx) {
452452
@Override
453453
public Object visitIntegerLiteral(IntegerLiteralContext ctx) {
454454
BigDecimal bigD = new BigDecimal(ctx.getText());
455-
// TODO: this can be improved to use the smallest type available
456-
return new Literal(source(ctx), bigD.longValueExact(), DataType.INTEGER);
455+
456+
long value = bigD.longValueExact();
457+
DataType type = DataType.LONG;
458+
// try to downsize to int if possible (since that's the most common type)
459+
if ((int) value == value) {
460+
type = DataType.INTEGER;
461+
}
462+
return new Literal(source(ctx), value, type);
457463
}
458464

459465
@Override

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ private static Conversion conversionToLong(DataType from) {
154154
return Conversion.INTEGER_TO_LONG;
155155
}
156156
if (from == BOOLEAN) {
157-
return Conversion.BOOL_TO_INT; // We emit an int here which is ok because of Java's casting rules
157+
return Conversion.BOOL_TO_LONG;
158158
}
159159
if (from.isString()) {
160160
return Conversion.STRING_TO_LONG;
@@ -407,7 +407,9 @@ public enum Conversion {
407407

408408
NUMERIC_TO_BOOLEAN(fromLong(value -> value != 0)),
409409
STRING_TO_BOOLEAN(fromString(DataTypeConversion::convertToBoolean, "Boolean")),
410-
DATE_TO_BOOLEAN(fromDate(value -> value != 0));
410+
DATE_TO_BOOLEAN(fromDate(value -> value != 0)),
411+
412+
BOOL_TO_LONG(fromBool(value -> value ? 1L : 0L));
411413

412414
private final Function<Object, Object> converter;
413415

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/ExpressionTests.java

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
package org.elasticsearch.xpack.sql.parser;
7+
8+
import org.elasticsearch.test.ESTestCase;
9+
import org.elasticsearch.xpack.sql.expression.Expression;
10+
import org.elasticsearch.xpack.sql.expression.Literal;
11+
import org.elasticsearch.xpack.sql.expression.function.scalar.arithmetic.Neg;
12+
import org.elasticsearch.xpack.sql.type.DataType;
13+
14+
public class ExpressionTests extends ESTestCase {
15+
16+
private final SqlParser parser = new SqlParser();
17+
18+
public void testLiteralLong() throws Exception {
19+
Expression lt = parser.createExpression(String.valueOf(Long.MAX_VALUE));
20+
assertEquals(Literal.class, lt.getClass());
21+
Literal l = (Literal) lt;
22+
assertEquals(Long.MAX_VALUE, l.value());
23+
assertEquals(DataType.LONG, l.dataType());
24+
}
25+
26+
public void testLiteralLongNegative() throws Exception {
27+
// Long.MIN_VALUE doesn't work since it is being interpreted as negate positive.long which is 1 higher than Long.MAX_VALUE
28+
Expression lt = parser.createExpression(String.valueOf(-Long.MAX_VALUE));
29+
assertEquals(Neg.class, lt.getClass());
30+
Neg n = (Neg) lt;
31+
assertTrue(n.foldable());
32+
assertEquals(-Long.MAX_VALUE, n.fold());
33+
assertEquals(DataType.LONG, n.dataType());
34+
}
35+
36+
public void testLiteralInteger() throws Exception {
37+
Expression lt = parser.createExpression(String.valueOf(Integer.MAX_VALUE));
38+
assertEquals(Literal.class, lt.getClass());
39+
Literal l = (Literal) lt;
40+
assertEquals(Integer.MAX_VALUE, l.value());
41+
assertEquals(DataType.INTEGER, l.dataType());
42+
}
43+
44+
public void testLiteralIntegerWithShortValue() throws Exception {
45+
Expression lt = parser.createExpression(String.valueOf(Short.MAX_VALUE));
46+
assertEquals(Literal.class, lt.getClass());
47+
Literal l = (Literal) lt;
48+
assertEquals(Integer.valueOf(Short.MAX_VALUE), l.value());
49+
assertEquals(DataType.INTEGER, l.dataType());
50+
}
51+
52+
public void testLiteralIntegerWithByteValue() throws Exception {
53+
Expression lt = parser.createExpression(String.valueOf(Byte.MAX_VALUE));
54+
assertEquals(Literal.class, lt.getClass());
55+
Literal l = (Literal) lt;
56+
assertEquals(Integer.valueOf(Byte.MAX_VALUE), l.value());
57+
assertEquals(DataType.INTEGER, l.dataType());
58+
}
59+
}

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java

+12-5
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ public void testConversionToLong() {
4545
{
4646
Conversion conversion = DataTypeConversion.conversionFor(DataType.BOOLEAN, to);
4747
assertNull(conversion.convert(null));
48-
assertEquals(1, conversion.convert(true));
49-
assertEquals(0, conversion.convert(false));
48+
assertEquals(1L, conversion.convert(true));
49+
assertEquals(0L, conversion.convert(false));
5050
}
5151
Conversion conversion = DataTypeConversion.conversionFor(DataType.KEYWORD, to);
5252
assertNull(conversion.convert(null));
@@ -141,12 +141,19 @@ public void testConversionToBoolean() {
141141
assertEquals(true, conversion.convert(-10));
142142
assertEquals(false, conversion.convert(0));
143143
}
144+
{
145+
Conversion conversion = DataTypeConversion.conversionFor(DataType.LONG, DataType.BOOLEAN);
146+
assertNull(conversion.convert(null));
147+
assertEquals(true, conversion.convert(10L));
148+
assertEquals(true, conversion.convert(-10L));
149+
assertEquals(false, conversion.convert(0L));
150+
}
144151
{
145152
Conversion conversion = DataTypeConversion.conversionFor(DataType.DOUBLE, DataType.BOOLEAN);
146153
assertNull(conversion.convert(null));
147-
assertEquals(true, conversion.convert(10.0));
148-
assertEquals(true, conversion.convert(-10.0));
149-
assertEquals(false, conversion.convert(0.0));
154+
assertEquals(true, conversion.convert(10.0d));
155+
assertEquals(true, conversion.convert(-10.0d));
156+
assertEquals(false, conversion.convert(0.0d));
150157
}
151158
{
152159
Conversion conversion = DataTypeConversion.conversionFor(DataType.KEYWORD, DataType.BOOLEAN);

0 commit comments

Comments
 (0)