Skip to content

Commit 7b58ed0

Browse files
bpinteabpintea
authored
Fix milliseconds handling in intervals (#51675) (#52156)
This fixes: - the parsing of milliseconds in intervals: everything past the . used to be converted as-is to milliseconds, with no normalisation of the unit; thus, a value of .23 ended up as 23 millis in the interval, instead of 230. - the printing of a trailing .0, in case the interval lacks the fractional part; - tests generating a random millisecond value used to simply print it in the string about to be evaluated without a necessary front-filling of 0[s], where the amount was below 100/10. (The combination of first and last issues above, plus statistical "luck" made the incorrect handling pass the tests.) (cherry picked from commit 4de8c64)
1 parent d188dda commit 7b58ed0

File tree

8 files changed

+67
-50
lines changed

8 files changed

+67
-50
lines changed

‎x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/util/DateUtils.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
2727

2828

29-
//FIXME: Taken from sql-proto.
29+
//FIXME: Taken from sql-proto (StringUtils)
3030
//Ideally it should be shared but the dependencies across projects and and SQL-client make it tricky.
3131
// Maybe a gradle task would fix that...
3232
public class DateUtils {
@@ -136,8 +136,14 @@ public static String toString(Object value) {
136136
sb.append(":");
137137
durationInSec = durationInSec % SECONDS_PER_MINUTE;
138138
sb.append(indent(durationInSec));
139-
sb.append(".");
140-
sb.append(TimeUnit.NANOSECONDS.toMillis(d.getNano()));
139+
long millis = TimeUnit.NANOSECONDS.toMillis(d.getNano());
140+
if (millis > 0) {
141+
sb.append(".");
142+
while (millis % 10 == 0) {
143+
millis /= 10;
144+
}
145+
sb.append(millis);
146+
}
141147
return sb.toString();
142148
}
143149

‎x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,22 @@ public void testIPs() throws IOException {
9595
public void testDateTimeIntervals() throws IOException {
9696
assertQuery("SELECT INTERVAL '326' YEAR", "INTERVAL '326' YEAR", "interval_year", "P326Y", "+326-0", 7);
9797
assertQuery("SELECT INTERVAL '50' MONTH", "INTERVAL '50' MONTH", "interval_month", "P50M", "+0-50", 7);
98-
assertQuery("SELECT INTERVAL '520' DAY", "INTERVAL '520' DAY", "interval_day", "PT12480H", "+520 00:00:00.0", 23);
99-
assertQuery("SELECT INTERVAL '163' HOUR", "INTERVAL '163' HOUR", "interval_hour", "PT163H", "+6 19:00:00.0", 23);
100-
assertQuery("SELECT INTERVAL '163' MINUTE", "INTERVAL '163' MINUTE", "interval_minute", "PT2H43M", "+0 02:43:00.0", 23);
101-
assertQuery("SELECT INTERVAL '223.16' SECOND", "INTERVAL '223.16' SECOND", "interval_second", "PT3M43.016S", "+0 00:03:43.16", 23);
98+
assertQuery("SELECT INTERVAL '520' DAY", "INTERVAL '520' DAY", "interval_day", "PT12480H", "+520 00:00:00", 23);
99+
assertQuery("SELECT INTERVAL '163' HOUR", "INTERVAL '163' HOUR", "interval_hour", "PT163H", "+6 19:00:00", 23);
100+
assertQuery("SELECT INTERVAL '163' MINUTE", "INTERVAL '163' MINUTE", "interval_minute", "PT2H43M", "+0 02:43:00", 23);
101+
assertQuery("SELECT INTERVAL '223.16' SECOND", "INTERVAL '223.16' SECOND", "interval_second", "PT3M43.16S", "+0 00:03:43.16", 23);
102102
assertQuery("SELECT INTERVAL '163-11' YEAR TO MONTH", "INTERVAL '163-11' YEAR TO MONTH", "interval_year_to_month", "P163Y11M",
103103
"+163-11", 7);
104104
assertQuery("SELECT INTERVAL '163 12' DAY TO HOUR", "INTERVAL '163 12' DAY TO HOUR", "interval_day_to_hour", "PT3924H",
105-
"+163 12:00:00.0", 23);
105+
"+163 12:00:00", 23);
106106
assertQuery("SELECT INTERVAL '163 12:39' DAY TO MINUTE", "INTERVAL '163 12:39' DAY TO MINUTE", "interval_day_to_minute",
107-
"PT3924H39M", "+163 12:39:00.0", 23);
107+
"PT3924H39M", "+163 12:39:00", 23);
108108
assertQuery("SELECT INTERVAL '163 12:39:59.163' DAY TO SECOND", "INTERVAL '163 12:39:59.163' DAY TO SECOND",
109109
"interval_day_to_second", "PT3924H39M59.163S", "+163 12:39:59.163", 23);
110110
assertQuery("SELECT INTERVAL -'163 23:39:56.23' DAY TO SECOND", "INTERVAL -'163 23:39:56.23' DAY TO SECOND",
111-
"interval_day_to_second", "PT-3935H-39M-56.023S", "-163 23:39:56.23", 23);
111+
"interval_day_to_second", "PT-3935H-39M-56.23S", "-163 23:39:56.23", 23);
112112
assertQuery("SELECT INTERVAL '163:39' HOUR TO MINUTE", "INTERVAL '163:39' HOUR TO MINUTE", "interval_hour_to_minute",
113-
"PT163H39M", "+6 19:39:00.0", 23);
113+
"PT163H39M", "+6 19:39:00", 23);
114114
assertQuery("SELECT INTERVAL '163:39:59.163' HOUR TO SECOND", "INTERVAL '163:39:59.163' HOUR TO SECOND", "interval_hour_to_second",
115115
"PT163H39M59.163S", "+6 19:39:59.163", 23);
116116
assertQuery("SELECT INTERVAL '163:59.163' MINUTE TO SECOND", "INTERVAL '163:59.163' MINUTE TO SECOND", "interval_minute_to_second",

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

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,17 @@
88
exactIntervals
99
SELECT INTERVAL 1 YEAR AS y, INTERVAL 2 MONTH AS m, INTERVAL 3 DAY AS d, INTERVAL 4 HOUR AS h, INTERVAL 5 MINUTE AS mm, INTERVAL 6 SECOND AS s;
1010

11-
y | m | d | h | mm | s
12-
---------------+---------------+---------------+---------------+---------------+---------------
13-
+1-0 |+0-2 |+3 00:00:00.0 |+0 04:00:00.0 |+0 00:05:00.0 |+0 00:00:06.0
11+
y | m | d | h | mm | s
12+
---------------+---------------+-------------+-------------+-------------+-------------
13+
+1-0 |+0-2 |+3 00:00:00 |+0 04:00:00 |+0 00:05:00 |+0 00:00:06
1414
;
1515

1616
testExactIntervalPlural
1717
SELECT INTERVAL 1 YEARS AS y, INTERVAL 2 MONTHS AS m, INTERVAL 3 DAYS AS d, INTERVAL 4 HOURS AS h, INTERVAL 5 MINUTES AS mm, INTERVAL 6 SECONDS AS s;
1818

19-
y | m | d | h | mm | s
20-
---------------+---------------+---------------+---------------+---------------+---------------
21-
+1-0 |+0-2 |+3 00:00:00.0 |+0 04:00:00.0 |+0 00:05:00.0 |+0 00:00:06.0
19+
y | m | d | h | mm | s
20+
---------------+---------------+-------------+-------------+-------------+-------------
21+
+1-0 |+0-2 |+3 00:00:00 |+0 04:00:00 |+0 00:05:00 |+0 00:00:06
2222
;
2323

2424
// take the examples from https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/interval-literals?view=sql-server-2017
@@ -43,23 +43,23 @@ SELECT INTERVAL '3261' DAY;
4343

4444
INTERVAL '3261' DAY
4545
-------------------
46-
+3261 00:00:00.0
46+
+3261 00:00:00
4747
;
4848

4949
hour
5050
SELECT INTERVAL '163' HOUR;
5151

5252
INTERVAL '163' HOUR
5353
-------------------
54-
+6 19:00:00.0
54+
+6 19:00:00
5555
;
5656

5757
minute
5858
SELECT INTERVAL '163' MINUTE;
5959

6060
INTERVAL '163' MINUTE
6161
---------------------
62-
+0 02:43:00.0
62+
+0 02:43:00
6363
;
6464

6565
second
@@ -83,15 +83,15 @@ SELECT INTERVAL '163 12' DAY TO HOUR;
8383

8484
INTERVAL '163 12' DAY TO HOUR
8585
-----------------------------
86-
+163 12:00:00.0
86+
+163 12:00:00
8787
;
8888

8989
dayMinute
9090
SELECT INTERVAL '163 12:39' DAY TO MINUTE AS interval;
9191

9292
interval
9393
---------------
94-
+163 12:39:00.0
94+
+163 12:39:00
9595
;
9696

9797
daySecond
@@ -115,7 +115,7 @@ SELECT INTERVAL '163:39' HOUR TO MINUTE AS interval;
115115

116116
interval
117117
---------------
118-
+6 19:39:00.0
118+
+6 19:39:00
119119
;
120120

121121
hourSecond
@@ -139,7 +139,7 @@ SELECT INTERVAL 1 DAY + INTERVAL 53 MINUTES;
139139

140140
INTERVAL 1 DAY + INTERVAL 53 MINUTES
141141
------------------------------------
142-
+1 00:53:00.0
142+
+1 00:53:00
143143
;
144144

145145
datePlusIntervalInline
@@ -162,9 +162,9 @@ SELECT - INTERVAL '49-1' YEAR TO MONTH result;
162162
intervalMinusInterval
163163
SELECT INTERVAL '1' DAY - INTERVAL '2' HOURS AS result;
164164

165-
result
166-
---------------
167-
+0 22:00:00.0
165+
result
166+
-------------
167+
+0 22:00:00
168168
;
169169

170170

@@ -179,16 +179,16 @@ SELECT -2 * INTERVAL '3' YEARS AS result;
179179
intervalDayMultiply
180180
SELECT -2 * INTERVAL '1 23:45' DAY TO MINUTES AS result;
181181

182-
result
183-
---------------
184-
-3 23:30:00.0
182+
result
183+
-------------
184+
-3 23:30:00
185185
;
186186

187187
intervalHoursMultiply
188188
SELECT 4 * -INTERVAL '2' HOURS AS result1, -5 * -INTERVAL '3' HOURS AS result2;
189-
result1 | result2
190-
---------------+--------------
191-
-0 08:00:00.0 | +0 15:00:00.0
189+
result1 | result2
190+
-------------+------------
191+
-0 08:00:00 | +0 15:00:00
192192
;
193193

194194
intervalNullMath
@@ -206,11 +206,11 @@ SELECT languages, CAST (languages * INTERVAL '1 10:30' DAY TO MINUTES AS string)
206206

207207
languages | result
208208
---------------+---------------------------------------------
209-
2 | +2 21:00:00.0
210-
5 | +7 04:30:00.0
211-
4 | +5 18:00:00.0
212-
5 | +7 04:30:00.0
213-
1 | +1 10:30:00.0
209+
2 | +2 21:00:00
210+
5 | +7 04:30:00
211+
4 | +5 18:00:00
212+
5 | +7 04:30:00
213+
1 | +1 10:30:00
214214
;
215215

216216
dateMinusInterval

‎x-pack/plugin/sql/qa/src/main/resources/docs/docs.csv-spec

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -876,9 +876,9 @@ dtIntervalPlusInterval
876876
// tag::dtIntervalPlusInterval
877877
SELECT INTERVAL 1 DAY + INTERVAL 53 MINUTES AS result;
878878

879-
result
879+
result
880880
---------------
881-
+1 00:53:00.0
881+
+1 00:53:00
882882

883883
// end::dtIntervalPlusInterval
884884
;
@@ -909,9 +909,9 @@ dtIntervalMinusInterval
909909
// tag::dtIntervalMinusInterval
910910
SELECT INTERVAL '1' DAY - INTERVAL '2' HOURS AS result;
911911

912-
result
912+
result
913913
---------------
914-
+0 22:00:00.0
914+
+0 22:00:00
915915
// end::dtIntervalMinusInterval
916916
;
917917

‎x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/StringUtils.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,14 @@ public static String toString(Object value) {
129129
sb.append(":");
130130
durationInSec = durationInSec % SECONDS_PER_MINUTE;
131131
sb.append(indent(durationInSec));
132-
sb.append(".");
133-
sb.append(TimeUnit.NANOSECONDS.toMillis(d.getNano()));
132+
long millis = TimeUnit.NANOSECONDS.toMillis(d.getNano());
133+
if (millis > 0) {
134+
sb.append(".");
135+
while (millis % 10 == 0) {
136+
millis /= 10;
137+
}
138+
sb.append(millis);
139+
}
134140
return sb.toString();
135141
}
136142

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/literal/interval/Intervals.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,10 @@ TemporalAmount parse(Source source, String string) {
341341
+ ": negative value [{}] not allowed (negate the entire interval instead)",
342342
v);
343343
}
344+
if (units.get(unitIndex) == TimeUnit.MILLISECOND && number.length() < 3) {
345+
// normalize the number past DOT to millis
346+
v *= number.length() < 2 ? 100 : 10;
347+
}
344348
values[unitIndex++] = v;
345349
} catch (QlIllegalArgumentException siae) {
346350
throw new ParsingException(source, invalidIntervalMessage(string), siae.getMessage());

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/literal/interval/IntervalsTests.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public void testMinuteInterval() throws Exception {
8484
public void testSecondInterval() throws Exception {
8585
int randomSeconds = randomNonNegativeInt();
8686
int randomMillis = randomBoolean() ? (randomBoolean() ? 0 : 999) : randomInt(999);
87-
String value = format(Locale.ROOT, "%s%d.%d", sign, randomSeconds, randomMillis);
87+
String value = format(Locale.ROOT, "%s%d", sign, randomSeconds);
88+
value += randomMillis > 0 ? format(Locale.ROOT, ".%03d", randomMillis) : "";
8889
TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_SECOND);
8990
assertEquals(maybeNegate(sign, Duration.ofSeconds(randomSeconds).plusMillis(randomMillis)), amount);
9091
}
@@ -129,7 +130,7 @@ public void testDayToSecond() throws Exception {
129130

130131
boolean withMillis = randomBoolean();
131132
int randomMilli = withMillis ? randomInt(999) : 0;
132-
String millisString = withMillis ? "." + randomMilli : "";
133+
String millisString = withMillis && randomMilli > 0 ? format(Locale.ROOT, ".%03d", randomMilli) : "";
133134

134135
String value = format(Locale.ROOT, "%s%d %d:%d:%d%s", sign, randomDay, randomHour, randomMinute, randomSecond, millisString);
135136
TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_DAY_TO_SECOND);
@@ -152,7 +153,7 @@ public void testHourToSecond() throws Exception {
152153

153154
boolean withMillis = randomBoolean();
154155
int randomMilli = withMillis ? randomInt(999) : 0;
155-
String millisString = withMillis ? "." + randomMilli : "";
156+
String millisString = withMillis && randomMilli > 0 ? format(Locale.ROOT, ".%03d", randomMilli) : "";
156157

157158
String value = format(Locale.ROOT, "%s%d:%d:%d%s", sign, randomHour, randomMinute, randomSecond, millisString);
158159
TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_HOUR_TO_SECOND);
@@ -166,7 +167,7 @@ public void testMinuteToSecond() throws Exception {
166167

167168
boolean withMillis = randomBoolean();
168169
int randomMilli = withMillis ? randomInt(999) : 0;
169-
String millisString = withMillis ? "." + randomMilli : "";
170+
String millisString = withMillis && randomMilli > 0 ? format(Locale.ROOT, ".%03d", randomMilli) : "";
170171

171172
String value = format(Locale.ROOT, "%s%d:%d%s", sign, randomMinute, randomSecond, millisString);
172173
TemporalAmount amount = parseInterval(EMPTY, value, INTERVAL_MINUTE_TO_SECOND);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public void testStringInterval() {
159159
int randomSecond = randomInt(59);
160160
int randomMilli = randomInt(999);
161161

162-
String value = format(Locale.ROOT, "INTERVAL '%d %d:%d:%d.%d' DAY TO SECOND", randomDay, randomHour, randomMinute, randomSecond,
162+
String value = format(Locale.ROOT, "INTERVAL '%d %d:%d:%d.%03d' DAY TO SECOND", randomDay, randomHour, randomMinute, randomSecond,
163163
randomMilli);
164164
assertEquals(Duration.ofDays(randomDay).plusHours(randomHour).plusMinutes(randomMinute).plusSeconds(randomSecond)
165165
.plusMillis(randomMilli), intervalOf(value));
@@ -172,7 +172,7 @@ public void testNegativeStringInterval() {
172172
int randomSecond = randomInt(59);
173173
int randomMilli = randomInt(999);
174174

175-
String value = format(Locale.ROOT, "INTERVAL -'%d %d:%d:%d.%d' DAY TO SECOND", randomDay, randomHour, randomMinute, randomSecond,
175+
String value = format(Locale.ROOT, "INTERVAL -'%d %d:%d:%d.%03d' DAY TO SECOND", randomDay, randomHour, randomMinute, randomSecond,
176176
randomMilli);
177177
assertEquals(Duration.ofDays(randomDay).plusHours(randomHour).plusMinutes(randomMinute).plusSeconds(randomSecond)
178178
.plusMillis(randomMilli).negated(), intervalOf(value));

0 commit comments

Comments
 (0)