Skip to content

Commit 542d1ee

Browse files
committed
SQL: Fix issue regarding INTERVAL * number
Interval * integer number is a valid operation which previously was only supported for foldables (literals) and not when a field was involved. That was because: 1. There was no common type returned for that combination 2. The `BinaryArithmeticOperation` was permitting the multiplication (called by fold()) but the BinaryArithmeticProcessor didn't allow it Moreover the error message for invalid arithmetic operations was wrong because of the issue with the overloading methods of `LoggerMessageFormat.format`. Fixes: elastic#41239
1 parent cce1a88 commit 542d1ee

File tree

7 files changed

+57
-5
lines changed

7 files changed

+57
-5
lines changed

x-pack/plugin/sql/qa/src/main/resources/datetime-interval.csv-spec

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,19 @@ SELECT -2 * INTERVAL '1 23:45' DAY TO MINUTES AS result;
182182
-3 23:30:00.0
183183
;
184184

185+
intervalAndFieldMultiply
186+
schema::languages:byte|result:string
187+
SELECT languages, CAST (languages * INTERVAL '1 10:30' DAY TO MINUTES AS string) AS result FROM test_emp ORDER BY emp_no LIMIT 5;
188+
189+
languages | result
190+
---------------+---------------------------------------------
191+
2 | +2 21:00:00.0
192+
5 | +7 04:30:00.0
193+
4 | +5 18:00:00.0
194+
5 | +7 04:30:00.0
195+
1 | +1 10:30:00.0
196+
;
197+
185198
dateMinusInterval
186199
SELECT CAST('2018-05-13T12:34:56' AS DATETIME) - INTERVAL '2-8' YEAR TO MONTH AS result;
187200

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/BinaryArithmeticProcessor.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ protected Object doProcess(Object left, Object right) {
164164
return null;
165165
}
166166

167-
if (f == BinaryArithmeticOperation.MUL || f == BinaryArithmeticOperation.DIV || f == BinaryArithmeticOperation.MOD) {
167+
if (f == BinaryArithmeticOperation.DIV || f == BinaryArithmeticOperation.MOD) {
168168
if (!(left instanceof Number)) {
169169
throw new SqlIllegalArgumentException("A number is required; received {}", left);
170170
}
@@ -176,8 +176,8 @@ protected Object doProcess(Object left, Object right) {
176176
return f.apply(left, right);
177177
}
178178

179-
if (f == BinaryArithmeticOperation.ADD || f == BinaryArithmeticOperation.SUB) {
180-
return f.apply(left, right);
179+
if (f == BinaryArithmeticOperation.ADD || f == BinaryArithmeticOperation.SUB || f == BinaryArithmeticOperation.MUL) {
180+
return f.apply(left, right);
181181
}
182182

183183
// this should not occur

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/DateTimeArithmeticOperation.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ protected TypeResolution resolveType() {
4343
// 2. 3. 4. intervals
4444
if ((DataTypes.isInterval(l) || DataTypes.isInterval(r))) {
4545
if (DataTypeConversion.commonType(l, r) == null) {
46-
return new TypeResolution(format("[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
46+
return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
4747
} else {
4848
return resolveWithIntervals();
4949
}
@@ -54,6 +54,15 @@ protected TypeResolution resolveType() {
5454
}
5555

5656
protected TypeResolution resolveWithIntervals() {
57+
DataType l = left().dataType();
58+
DataType r = right().dataType();
59+
60+
if (!(r.isDateOrTimeBased() || DataTypes.isInterval(r))) {
61+
return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
62+
}
63+
if (!(l.isDateOrTimeBased() || DataTypes.isInterval(l))) {
64+
return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
65+
}
5766
return TypeResolution.TYPE_RESOLVED;
5867
}
5968
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Mul.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ protected TypeResolution resolveType() {
4747
return TypeResolution.TYPE_RESOLVED;
4848
}
4949

50-
return new TypeResolution(format("[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
50+
return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
5151
}
5252

5353
@Override

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,17 @@ public static DataType commonType(DataType left, DataType right) {
121121
return right;
122122
}
123123
}
124+
// Interval * integer is a valid operation
125+
if (DataTypes.isInterval(left)) {
126+
if (right.isInteger()) {
127+
return left;
128+
}
129+
}
130+
if (DataTypes.isInterval(right)) {
131+
if (left.isInteger()) {
132+
return right;
133+
}
134+
}
124135
if (DataTypes.isInterval(left)) {
125136
// intervals widening
126137
if (DataTypes.isInterval(right)) {

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,21 @@ public void testSubtractFromInterval() {
241241
error("SELECT INTERVAL 1 MONTH - CAST('12:23:56.789' AS TIME)"));
242242
}
243243

244+
public void testAddIntervalAndNumberNotAllowed() {
245+
assertEquals("1:8: [+] has arguments with incompatible types [INTERVAL_DAY] and [INTEGER]",
246+
error("SELECT INTERVAL 1 DAY + 100"));
247+
}
248+
249+
public void testSubtractIntervalAndNumberNotAllowed() {
250+
assertEquals("1:8: [-] has arguments with incompatible types [INTERVAL_MINUTE] and [DOUBLE]",
251+
error("SELECT INTERVAL 10 MINUTE - 100.0"));
252+
}
253+
254+
public void testMultiplyIntervalWithDecimalNotAllowed() {
255+
assertEquals("1:8: [*] has arguments with incompatible types [INTERVAL_MONTH] and [DOUBLE]",
256+
error("SELECT INTERVAL 1 MONTH * 1.234"));
257+
}
258+
244259
public void testMultipleColumns() {
245260
assertEquals("1:43: Unknown column [xxx]\nline 1:8: Unknown column [xxx]",
246261
error("SELECT xxx FROM test GROUP BY DAY_oF_YEAR(xxx)"));

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,10 @@ public void testCommonType() {
628628
assertEquals(FLOAT, commonType(FLOAT, INTEGER));
629629
assertEquals(DOUBLE, commonType(DOUBLE, FLOAT));
630630

631+
// numeric and intervals
632+
assertEquals(INTERVAL_YEAR_TO_MONTH, commonType(INTERVAL_YEAR_TO_MONTH, LONG));
633+
assertEquals(INTERVAL_HOUR_TO_MINUTE, commonType(INTEGER, INTERVAL_HOUR_TO_MINUTE));
634+
631635
// dates/datetimes and intervals
632636
assertEquals(DATETIME, commonType(DATE, DATETIME));
633637
assertEquals(DATETIME, commonType(DATETIME, DATE));

0 commit comments

Comments
 (0)