Skip to content

Commit 6c4a966

Browse files
opensearch-trigger-bot[bot]github-actions[bot]
opensearch-trigger-bot[bot]
and
github-actions[bot]
authored
Fix span on negative timestamp (#4017) (#4028)
* Fix span on negative timestamp * Fix span on negative timestamp * typo * Refine code --------- (cherry picked from commit ad3fc1f) Signed-off-by: Heng Qian <qianheng@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 54f1287 commit 6c4a966

File tree

3 files changed

+85
-11
lines changed

3 files changed

+85
-11
lines changed

‎core/src/main/java/org/opensearch/sql/utils/DateTimeUtils.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public class DateTimeUtils {
3636
* @return Rounded date/time value in utc millis
3737
*/
3838
public static long roundFloor(long utcMillis, long unitMillis) {
39-
return utcMillis - utcMillis % unitMillis;
39+
long res = utcMillis - utcMillis % unitMillis;
40+
return (utcMillis < 0 && res != utcMillis) ? res - unitMillis : res;
4041
}
4142

4243
/**
@@ -65,7 +66,9 @@ public static long roundMonth(long utcMillis, int interval) {
6566
(zonedDateTime.getYear() - initDateTime.getYear()) * 12L
6667
+ zonedDateTime.getMonthValue()
6768
- initDateTime.getMonthValue();
68-
long monthToAdd = (monthDiff / interval - 1) * interval;
69+
long multiplier = monthDiff / interval - 1;
70+
if (monthDiff < 0 && monthDiff % interval != 0) --multiplier;
71+
long monthToAdd = multiplier * interval;
6972
return initDateTime.plusMonths(monthToAdd).toInstant().toEpochMilli();
7073
}
7174

@@ -84,7 +87,9 @@ public static long roundQuarter(long utcMillis, int interval) {
8487
((zonedDateTime.getYear() - initDateTime.getYear()) * 12L
8588
+ zonedDateTime.getMonthValue()
8689
- initDateTime.getMonthValue());
87-
long monthToAdd = (monthDiff / (interval * 3L) - 1) * interval * 3;
90+
long multiplier = monthDiff / (interval * 3L) - 1;
91+
if (monthDiff < 0 && monthDiff % (interval * 3L) != 0) --multiplier;
92+
long monthToAdd = multiplier * interval * 3;
8893
return initDateTime.plusMonths(monthToAdd).toInstant().toEpochMilli();
8994
}
9095

@@ -99,7 +104,9 @@ public static long roundYear(long utcMillis, int interval) {
99104
ZonedDateTime initDateTime = ZonedDateTime.of(1970, 1, 1, 0, 0, 0, 0, UTC_ZONE_ID);
100105
ZonedDateTime zonedDateTime = Instant.ofEpochMilli(utcMillis).atZone(UTC_ZONE_ID);
101106
int yearDiff = zonedDateTime.getYear() - initDateTime.getYear();
102-
int yearToAdd = (yearDiff / interval) * interval;
107+
int multiplier = yearDiff / interval;
108+
if (yearDiff < 0 && yearDiff % interval != 0) --multiplier;
109+
int yearToAdd = multiplier * interval;
103110
return initDateTime.plusYears(yearToAdd).toInstant().toEpochMilli();
104111
}
105112

‎core/src/test/java/org/opensearch/sql/utils/DateTimeUtilsTest.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import java.time.format.DateTimeFormatter;
1717
import java.util.concurrent.TimeUnit;
1818
import org.junit.jupiter.api.Test;
19+
import org.opensearch.sql.planner.physical.collector.Rounding.DateTimeUnit;
1920

2021
public class DateTimeUtilsTest {
2122
@Test
@@ -105,4 +106,76 @@ void testRelativeZonedDateTimeWithWrongInput() {
105106
IllegalArgumentException.class, () -> getRelativeZonedDateTime("1d+1y", zonedDateTime));
106107
assertEquals(e.getMessage(), "Unexpected character '1' at position 0 in input: 1d+1y");
107108
}
109+
110+
@Test
111+
void testRoundOnTimestampBeforeEpoch() {
112+
long actual =
113+
LocalDateTime.parse("1961-05-12T23:40:05")
114+
.atZone(ZoneOffset.UTC)
115+
.toInstant()
116+
.toEpochMilli();
117+
long rounded = DateTimeUnit.MINUTE.round(actual, 1);
118+
assertEquals(
119+
LocalDateTime.parse("1961-05-12T23:40:00")
120+
.atZone(ZoneOffset.UTC)
121+
.toInstant()
122+
.toEpochMilli(),
123+
Instant.ofEpochMilli(rounded).toEpochMilli());
124+
125+
rounded = DateTimeUnit.HOUR.round(actual, 1);
126+
assertEquals(
127+
LocalDateTime.parse("1961-05-12T23:00:00")
128+
.atZone(ZoneOffset.UTC)
129+
.toInstant()
130+
.toEpochMilli(),
131+
Instant.ofEpochMilli(rounded).toEpochMilli());
132+
133+
rounded = DateTimeUnit.DAY.round(actual, 1);
134+
assertEquals(
135+
LocalDateTime.parse("1961-05-12T00:00:00")
136+
.atZone(ZoneOffset.UTC)
137+
.toInstant()
138+
.toEpochMilli(),
139+
Instant.ofEpochMilli(rounded).toEpochMilli());
140+
141+
rounded = DateTimeUnit.DAY.round(actual, 3);
142+
assertEquals(
143+
LocalDateTime.parse("1961-05-12T00:00:00")
144+
.atZone(ZoneOffset.UTC)
145+
.toInstant()
146+
.toEpochMilli(),
147+
Instant.ofEpochMilli(rounded).toEpochMilli());
148+
149+
rounded = DateTimeUnit.WEEK.round(actual, 1);
150+
assertEquals(
151+
LocalDateTime.parse("1961-05-08T00:00:00")
152+
.atZone(ZoneOffset.UTC)
153+
.toInstant()
154+
.toEpochMilli(),
155+
Instant.ofEpochMilli(rounded).toEpochMilli());
156+
157+
rounded = DateTimeUnit.MONTH.round(actual, 1);
158+
assertEquals(
159+
LocalDateTime.parse("1961-05-01T00:00:00")
160+
.atZone(ZoneOffset.UTC)
161+
.toInstant()
162+
.toEpochMilli(),
163+
Instant.ofEpochMilli(rounded).toEpochMilli());
164+
165+
rounded = DateTimeUnit.QUARTER.round(actual, 1);
166+
assertEquals(
167+
LocalDateTime.parse("1961-04-01T00:00:00")
168+
.atZone(ZoneOffset.UTC)
169+
.toInstant()
170+
.toEpochMilli(),
171+
Instant.ofEpochMilli(rounded).toEpochMilli());
172+
173+
rounded = DateTimeUnit.YEAR.round(actual, 2);
174+
assertEquals(
175+
LocalDateTime.parse("1960-01-01T00:00:00")
176+
.atZone(ZoneOffset.UTC)
177+
.toInstant()
178+
.toEpochMilli(),
179+
Instant.ofEpochMilli(rounded).toEpochMilli());
180+
}
108181
}

‎integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLAggregationIT.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -502,13 +502,7 @@ public void testCountBySpanForCustomFormats() throws IOException {
502502
actual,
503503
schema("timestamp_span", "timestamp"),
504504
schema("count(custom_no_delimiter_ts)", "bigint"));
505-
// TODO: Span has different behavior between pushdown and non-pushdown for timestamp before
506-
// 1971. V2 engine will have the same issue.
507-
// https://github.com/opensearch-project/sql/issues/3827
508-
verifyDataRows(
509-
actual,
510-
rows(1, isPushdownEnabled() ? "1961-04-12 09:00:00" : "1961-04-12 10:00:00"),
511-
rows(1, "1984-10-20 15:00:00"));
505+
verifyDataRows(actual, rows(1, "1961-04-12 09:00:00"), rows(1, "1984-10-20 15:00:00"));
512506

513507
actual =
514508
executeQuery(

0 commit comments

Comments
 (0)