Skip to content

Commit 7473742

Browse files
committed
SQL: Fix issue regarding INTERVAL * number (#42014)
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: #41239 Fixes: #41200 (cherry picked from commit 91039ba)
1 parent 9191b02 commit 7473742

File tree

8 files changed

+71
-5
lines changed

8 files changed

+71
-5
lines changed

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

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

185+
intervalHoursMultiply
186+
SELECT 4 * -INTERVAL '2' HOURS AS result1, -5 * -INTERVAL '3' HOURS AS result2;
187+
result1 | result2
188+
---------------+--------------
189+
-0 08:00:00.0 | +0 15:00:00.0
190+
;
191+
192+
intervalAndFieldMultiply
193+
schema::languages:byte|result:string
194+
SELECT languages, CAST (languages * INTERVAL '1 10:30' DAY TO MINUTES AS string) AS result FROM test_emp ORDER BY emp_no LIMIT 5;
195+
196+
languages | result
197+
---------------+---------------------------------------------
198+
2 | +2 21:00:00.0
199+
5 | +7 04:30:00.0
200+
4 | +5 18:00:00.0
201+
5 | +7 04:30:00.0
202+
1 | +1 10:30:00.0
203+
;
204+
185205
dateMinusInterval
186206
SELECT CAST('2018-05-13T12:34:56' AS DATETIME) - INTERVAL '2-8' YEAR TO MONTH AS result;
187207

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: 7 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,12 @@ 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))|| !(l.isDateOrTimeBased() || DataTypes.isInterval(l))) {
61+
return new TypeResolution(format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
62+
}
5763
return TypeResolution.TYPE_RESOLVED;
5864
}
5965
}

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/expression/predicate/operator/arithmetic/Sub.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ protected Sub replaceChildren(Expression newLeft, Expression newRight) {
3434

3535
@Override
3636
protected TypeResolution resolveWithIntervals() {
37+
TypeResolution resolution = super.resolveWithIntervals();
38+
if (resolution.unresolved()) {
39+
return resolution;
40+
}
3741
if ((right().dataType().isDateOrTimeBased()) && DataTypes.isInterval(left().dataType())) {
3842
return new TypeResolution(format(null, "Cannot subtract a {}[{}] from an interval[{}]; do you mean the reverse?",
3943
right().dataType().typeName, right().source().text(), left().source().text()));

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: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,27 @@ 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+
assertEquals("1:8: [+] has arguments with incompatible types [INTEGER] and [INTERVAL_DAY]",
248+
error("SELECT 100 + INTERVAL 1 DAY"));
249+
}
250+
251+
public void testSubtractIntervalAndNumberNotAllowed() {
252+
assertEquals("1:8: [-] has arguments with incompatible types [INTERVAL_MINUTE] and [DOUBLE]",
253+
error("SELECT INTERVAL 10 MINUTE - 100.0"));
254+
assertEquals("1:8: [-] has arguments with incompatible types [DOUBLE] and [INTERVAL_MINUTE]",
255+
error("SELECT 100.0 - INTERVAL 10 MINUTE"));
256+
}
257+
258+
public void testMultiplyIntervalWithDecimalNotAllowed() {
259+
assertEquals("1:8: [*] has arguments with incompatible types [INTERVAL_MONTH] and [DOUBLE]",
260+
error("SELECT INTERVAL 1 MONTH * 1.234"));
261+
assertEquals("1:8: [*] has arguments with incompatible types [DOUBLE] and [INTERVAL_MONTH]",
262+
error("SELECT 1.234 * INTERVAL 1 MONTH"));
263+
}
264+
244265
public void testMultipleColumns() {
245266
assertEquals("1:43: Unknown column [xxx]\nline 1:8: Unknown column [xxx]",
246267
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
@@ -627,6 +627,10 @@ public void testCommonType() {
627627
assertEquals(FLOAT, commonType(FLOAT, INTEGER));
628628
assertEquals(DOUBLE, commonType(DOUBLE, FLOAT));
629629

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

0 commit comments

Comments
 (0)