Skip to content

Commit 124cd18

Browse files
matrivmatriv
authored
SQL: Fix issue with mins & hours for DATEDIFF (#49252)
Previously, DATEDIFF for minutes and hours was doing a rounding calculation using all the time fields (secs, msecs/micros/nanos). Instead it should first truncate the 2 dates to the respective field (mins or hours) zeroing out all the more detailed time fields and then make the subtraction.
1 parent f27511c commit 124cd18

5 files changed

Lines changed: 93 additions & 7 deletions

File tree

‎docs/reference/sql/functions/date-time.asciidoc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,20 @@ include-tagged::{sql-specs}/docs/docs.csv-spec[dateDiffDateTimeSeconds]
395395
include-tagged::{sql-specs}/docs/docs.csv-spec[dateDiffDateQuarters]
396396
--------------------------------------------------
397397

398+
[NOTE]
399+
For `hour` and `minute`, `DATEDIFF` doesn't do any rounding, but instead first truncates
400+
the more detailed time fields on the 2 dates to zero and then calculates the subtraction.
401+
402+
[source, sql]
403+
--------------------------------------------------
404+
include-tagged::{sql-specs}/docs/docs.csv-spec[dateDiffDateTimeHours]
405+
--------------------------------------------------
406+
407+
[source, sql]
408+
--------------------------------------------------
409+
include-tagged::{sql-specs}/docs/docs.csv-spec[dateDiffDateTimeMinutes]
410+
--------------------------------------------------
411+
398412
[source, sql]
399413
--------------------------------------------------
400414
include-tagged::{sql-specs}/docs/docs.csv-spec[dateDiffDateMinutes]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ DATE_DIFF('mcs', '2019-09-04T11:25:21.123456Z'::datetime, '2019-09-04T11:22:33.9
307307

308308
diff_year | diff_quarter | diff_month | diff_week | diff_day | diff_hours | diff_min | diff_sec | diff_millis | diff_mcsec | diff_nsec
309309
-----------+--------------+------------+-----------+----------+------------+----------+-----------+-------------+------------+----------
310-
57 | -114 | 406 | -947 | 2825 | -123228 | 3762357 | -10265677 | 205849864 | -167135802 | 135802468
310+
57 | -114 | 406 | -947 | 2825 | -123228 | 3762356 | -10265677 | 205849864 | -167135802 | 135802468
311311
;
312312

313313
selectDiffWithDate

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2496,6 +2496,27 @@ SELECT DATE_DIFF('week', '2019-09-04T11:22:33.000Z'::datetime, '2016-12-08T22:33
24962496
// end::dateDiffDateTimeWeeks
24972497
;
24982498

2499+
dateDiffDateTimeHours
2500+
// tag::dateDiffDateTimeHours
2501+
SELECT DATEDIFF('hours', '2019-11-10T12:10:00.000Z'::datetime, '2019-11-10T23:59:59.999Z'::datetime) AS "diffInHours";
2502+
2503+
diffInHours
2504+
------------------------
2505+
11
2506+
// end::dateDiffDateTimeHours
2507+
;
2508+
2509+
2510+
dateDiffDateTimeMinutes
2511+
// tag::dateDiffDateTimeMinutes
2512+
SELECT DATEDIFF('minute', '2019-11-10T12:10:00.000Z'::datetime, '2019-11-10T12:15:59.999Z'::datetime) AS "diffInMinutes";
2513+
2514+
diffInMinutes
2515+
------------------------
2516+
5
2517+
// end::dateDiffDateTimeMinutes
2518+
;
2519+
24992520
dateDiffDateTimeSeconds
25002521
// tag::dateDiffDateTimeSeconds
25012522
SELECT DATE_DIFF('seconds', '2019-09-04T11:22:33.123Z'::datetime, '2019-07-12T22:33:11.321Z'::datetime) AS "diffInSeconds";

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateDiff.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,8 @@ private static int safeInt(long diff) {
114114
}
115115

116116
private static long diffInMinutes(ZonedDateTime start, ZonedDateTime end) {
117-
long secondsDiff = diffInSeconds(start, end);
118-
if (secondsDiff > 0) {
119-
return (long) Math.ceil(secondsDiff / 60.0d);
120-
} else {
121-
return (long) Math.floor(secondsDiff / 60.0d);
122-
}
117+
// Truncate first to minutes (ignore any seconds and sub-seconds fields)
118+
return (end.toEpochSecond() / 60) - (start.toEpochSecond() / 60);
123119
}
124120

125121
private static long diffInHours(ZonedDateTime start, ZonedDateTime end) {

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateDiffProcessorTests.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,61 @@ public void testDiffEdgeCases() {
284284
.makePipe().asProcessor().process(null));
285285
assertEquals(-436, new DateDiff(Source.EMPTY, l("ww"), dt2, dt1, zoneId)
286286
.makePipe().asProcessor().process(null));
287+
288+
dt1 = l(dateTime(1997, 9, 19, 0, 0, 0, 0));
289+
dt2 = l(dateTime(2004, 8, 2, 7, 59, 23, 0));
290+
assertEquals(60223, new DateDiff(Source.EMPTY, l("hour"), dt1, dt2, UTC)
291+
.makePipe().asProcessor().process(null));
292+
assertEquals(-60223, new DateDiff(Source.EMPTY, l("hours"), dt2, dt1, UTC)
293+
.makePipe().asProcessor().process(null));
294+
assertEquals(60223, new DateDiff(Source.EMPTY, l("hh"), dt1, dt2, zoneId)
295+
.makePipe().asProcessor().process(null));
296+
assertEquals(-60223, new DateDiff(Source.EMPTY, l("hh"), dt2, dt1, zoneId)
297+
.makePipe().asProcessor().process(null));
298+
299+
dt1 = l(dateTime(1997, 9, 19, 0, 0, 0, 0));
300+
dt2 = l(dateTime(2004, 8, 2, 7, 59, 59, 999999999));
301+
assertEquals(60223, new DateDiff(Source.EMPTY, l("hour"), dt1, dt2, UTC)
302+
.makePipe().asProcessor().process(null));
303+
assertEquals(-60223, new DateDiff(Source.EMPTY, l("hours"), dt2, dt1, UTC)
304+
.makePipe().asProcessor().process(null));
305+
assertEquals(60223, new DateDiff(Source.EMPTY, l("hh"), dt1, dt2, zoneId)
306+
.makePipe().asProcessor().process(null));
307+
assertEquals(-60223, new DateDiff(Source.EMPTY, l("hh"), dt2, dt1, zoneId)
308+
.makePipe().asProcessor().process(null));
309+
310+
dt1 = l(dateTime(2002, 4, 27, 0, 0, 0, 0));
311+
dt2 = l(dateTime(2004, 7, 28, 12, 34, 28, 0));
312+
assertEquals(1185874, new DateDiff(Source.EMPTY, l("minute"), dt1, dt2, UTC)
313+
.makePipe().asProcessor().process(null));
314+
assertEquals(-1185874, new DateDiff(Source.EMPTY, l("minutes"), dt2, dt1, UTC)
315+
.makePipe().asProcessor().process(null));
316+
assertEquals(1185874, new DateDiff(Source.EMPTY, l("mi"), dt1, dt2, zoneId)
317+
.makePipe().asProcessor().process(null));
318+
assertEquals(-1185874, new DateDiff(Source.EMPTY, l("n"), dt2, dt1, zoneId)
319+
.makePipe().asProcessor().process(null));
320+
321+
dt1 = l(dateTime(1995, 9, 3, 0, 0, 0, 0));
322+
dt2 = l(dateTime(2004, 7, 26, 12, 30, 34, 0));
323+
assertEquals(4679310, new DateDiff(Source.EMPTY, l("minute"), dt1, dt2, UTC)
324+
.makePipe().asProcessor().process(null));
325+
assertEquals(-4679310, new DateDiff(Source.EMPTY, l("minutes"), dt2, dt1, UTC)
326+
.makePipe().asProcessor().process(null));
327+
assertEquals(4679310, new DateDiff(Source.EMPTY, l("mi"), dt1, dt2, zoneId)
328+
.makePipe().asProcessor().process(null));
329+
assertEquals(-4679310, new DateDiff(Source.EMPTY, l("n"), dt2, dt1, zoneId)
330+
.makePipe().asProcessor().process(null));
331+
332+
dt1 = l(dateTime(1997, 5, 30, 0, 0, 0, 0));
333+
dt2 = l(dateTime(2004, 7, 28, 23, 30, 59, 999999999));
334+
assertEquals(3768450, new DateDiff(Source.EMPTY, l("minute"), dt1, dt2, UTC)
335+
.makePipe().asProcessor().process(null));
336+
assertEquals(-3768450, new DateDiff(Source.EMPTY, l("minutes"), dt2, dt1, UTC)
337+
.makePipe().asProcessor().process(null));
338+
assertEquals(3768450, new DateDiff(Source.EMPTY, l("mi"), dt1, dt2, zoneId)
339+
.makePipe().asProcessor().process(null));
340+
assertEquals(-3768450, new DateDiff(Source.EMPTY, l("n"), dt2, dt1, zoneId)
341+
.makePipe().asProcessor().process(null));
287342
}
288343

289344
public void testOverflow() {

0 commit comments

Comments
 (0)