Skip to content

Commit 08d38da

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 5071d43 commit 08d38da

File tree

8 files changed

+72
-6
lines changed

8 files changed

+72
-6
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
@@ -162,7 +162,7 @@ protected Object doProcess(Object left, Object right) {
162162
return null;
163163
}
164164

165-
if (f == BinaryArithmeticOperation.MUL || f == BinaryArithmeticOperation.DIV || f == BinaryArithmeticOperation.MOD) {
165+
if (f == BinaryArithmeticOperation.DIV || f == BinaryArithmeticOperation.MOD) {
166166
if (!(left instanceof Number)) {
167167
throw new SqlIllegalArgumentException("A number is required; received {}", left);
168168
}
@@ -174,8 +174,8 @@ protected Object doProcess(Object left, Object right) {
174174
return f.apply(left, right);
175175
}
176176

177-
if (f == BinaryArithmeticOperation.ADD || f == BinaryArithmeticOperation.SUB) {
178-
return f.apply(left, right);
177+
if (f == BinaryArithmeticOperation.ADD || f == BinaryArithmeticOperation.SUB || f == BinaryArithmeticOperation.MUL) {
178+
return f.apply(left, right);
179179
}
180180

181181
// 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.isDateBased() || DataTypes.isInterval(r))|| !(l.isDateBased() || 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: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ protected Sub replaceChildren(Expression newLeft, Expression newRight) {
3434

3535
@Override
3636
protected TypeResolution resolveWithIntervals() {
37-
if (right().dataType().isDateBased() && DataTypes.isInterval(left().dataType())) {
37+
TypeResolution resolution = super.resolveWithIntervals();
38+
if (resolution.unresolved()) {
39+
return resolution;
40+
}
41+
if ((right().dataType().isDateBased()) && 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()));
4044
}

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
@@ -103,6 +103,17 @@ public static DataType commonType(DataType left, DataType right) {
103103
return right;
104104
}
105105
}
106+
// Interval * integer is a valid operation
107+
if (DataTypes.isInterval(left)) {
108+
if (right.isInteger()) {
109+
return left;
110+
}
111+
}
112+
if (DataTypes.isInterval(right)) {
113+
if (left.isInteger()) {
114+
return right;
115+
}
116+
}
106117
if (DataTypes.isInterval(left)) {
107118
// intervals widening
108119
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
@@ -209,6 +209,27 @@ public void testSubtractFromInterval() {
209209
error("SELECT INTERVAL 1 MONTH - CAST('2000-01-01' AS DATETIME)"));
210210
}
211211

212+
public void testAddIntervalAndNumberNotAllowed() {
213+
assertEquals("1:8: [+] has arguments with incompatible types [INTERVAL_DAY] and [INTEGER]",
214+
error("SELECT INTERVAL 1 DAY + 100"));
215+
assertEquals("1:8: [+] has arguments with incompatible types [INTEGER] and [INTERVAL_DAY]",
216+
error("SELECT 100 + INTERVAL 1 DAY"));
217+
}
218+
219+
public void testSubtractIntervalAndNumberNotAllowed() {
220+
assertEquals("1:8: [-] has arguments with incompatible types [INTERVAL_MINUTE] and [DOUBLE]",
221+
error("SELECT INTERVAL 10 MINUTE - 100.0"));
222+
assertEquals("1:8: [-] has arguments with incompatible types [DOUBLE] and [INTERVAL_MINUTE]",
223+
error("SELECT 100.0 - INTERVAL 10 MINUTE"));
224+
}
225+
226+
public void testMultiplyIntervalWithDecimalNotAllowed() {
227+
assertEquals("1:8: [*] has arguments with incompatible types [INTERVAL_MONTH] and [DOUBLE]",
228+
error("SELECT INTERVAL 1 MONTH * 1.234"));
229+
assertEquals("1:8: [*] has arguments with incompatible types [DOUBLE] and [INTERVAL_MONTH]",
230+
error("SELECT 1.234 * INTERVAL 1 MONTH"));
231+
}
232+
212233
public void testMultipleColumns() {
213234
assertEquals("1:43: Unknown column [xxx]\nline 1:8: Unknown column [xxx]",
214235
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
@@ -449,6 +449,10 @@ public void testCommonType() {
449449
assertEquals(FLOAT, commonType(FLOAT, INTEGER));
450450
assertEquals(DOUBLE, commonType(DOUBLE, FLOAT));
451451

452+
// numeric and intervals
453+
assertEquals(INTERVAL_YEAR_TO_MONTH, commonType(INTERVAL_YEAR_TO_MONTH, LONG));
454+
assertEquals(INTERVAL_HOUR_TO_MINUTE, commonType(INTEGER, INTERVAL_HOUR_TO_MINUTE));
455+
452456
// dates/datetimes and intervals
453457
assertEquals(DATETIME, commonType(DATE, DATETIME));
454458
assertEquals(DATETIME, commonType(DATETIME, DATE));

0 commit comments

Comments
 (0)