Skip to content

Commit 07470c9

Browse files
authored
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.
1 parent 6e9bd26 commit 07470c9

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

Lines changed: 8 additions & 2 deletions
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

Lines changed: 4 additions & 2 deletions
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

Lines changed: 59 additions & 0 deletions
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

Lines changed: 12 additions & 5 deletions
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)