Skip to content

Commit

Permalink
Fix a regression issue of parsing datetime string with custom time fo…
Browse files Browse the repository at this point in the history
…rmat in Span (#3079)

(cherry picked from commit 4ff1fe3)
  • Loading branch information
LantaoJin committed Nov 1, 2024
1 parent 95809c7 commit 2728f01
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ public static Rounding<?> createRounding(SpanExpression span) {
if (DOUBLE.isCompatible(type)) {
return new DoubleRounding(interval);
}
if (type.equals(DATETIME)) {
if (type.equals(DATETIME) || type.typeName().equalsIgnoreCase(DATETIME.typeName())) {
return new DatetimeRounding(interval, span.getUnit().getName());
}
if (type.equals(TIMESTAMP)) {
if (type.equals(TIMESTAMP) || type.typeName().equalsIgnoreCase(TIMESTAMP.typeName())) {
return new TimestampRounding(interval, span.getUnit().getName());
}
if (type.equals(DATE)) {
if (type.equals(DATE) || type.typeName().equalsIgnoreCase(DATE.typeName())) {
return new DateRounding(interval, span.getUnit().getName());
}
if (type.equals(TIME)) {
if (type.equals(TIME) || type.typeName().equalsIgnoreCase(TIME.typeName())) {
return new TimeRounding(interval, span.getUnit().getName());
}
return new UnknownRounding();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@

package org.opensearch.sql.planner.physical.collector;

import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.opensearch.sql.data.type.ExprCoreType.DATE;
import static org.opensearch.sql.data.type.ExprCoreType.STRING;
import static org.opensearch.sql.data.type.ExprCoreType.TIME;
import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP;

import org.junit.jupiter.api.Test;
import org.opensearch.sql.data.model.ExprTimeValue;
import org.opensearch.sql.data.model.ExprValueUtils;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.exception.ExpressionEvaluationException;
import org.opensearch.sql.expression.DSL;
import org.opensearch.sql.expression.span.SpanExpression;
Expand All @@ -26,6 +30,35 @@ void time_rounding_illegal_span() {
ExpressionEvaluationException.class, () -> rounding.round(new ExprTimeValue("23:30:00")));
}

@Test
void datetime_rounding_span() {
SpanExpression dateSpan = DSL.span(DSL.ref("date", DATE), DSL.literal(1), "d");
Rounding rounding = Rounding.createRounding(dateSpan);
assertInstanceOf(Rounding.DateRounding.class, rounding);
SpanExpression timeSpan = DSL.span(DSL.ref("time", TIME), DSL.literal(1), "h");
rounding = Rounding.createRounding(timeSpan);
assertInstanceOf(Rounding.TimeRounding.class, rounding);
SpanExpression timestampSpan = DSL.span(DSL.ref("timestamp", TIMESTAMP), DSL.literal(1), "h");
rounding = Rounding.createRounding(timestampSpan);
assertInstanceOf(Rounding.TimestampRounding.class, rounding);
}

@Test
void datetime_rounding_non_core_type_span() {
SpanExpression dateSpan =
DSL.span(DSL.ref("date", new MockDateExprType()), DSL.literal(1), "d");
Rounding rounding = Rounding.createRounding(dateSpan);
assertInstanceOf(Rounding.DateRounding.class, rounding);
SpanExpression timeSpan =
DSL.span(DSL.ref("time", new MockTimeExprType()), DSL.literal(1), "h");
rounding = Rounding.createRounding(timeSpan);
assertInstanceOf(Rounding.TimeRounding.class, rounding);
SpanExpression timestampSpan =
DSL.span(DSL.ref("timestamp", new MockTimestampExprType()), DSL.literal(1), "h");
rounding = Rounding.createRounding(timestampSpan);
assertInstanceOf(Rounding.TimestampRounding.class, rounding);
}

@Test
void round_unknown_type() {
SpanExpression span = DSL.span(DSL.ref("unknown", STRING), DSL.literal(1), "");
Expand All @@ -41,4 +74,25 @@ void resolve() {
() -> Rounding.DateTimeUnit.resolve(illegalUnit),
"Unable to resolve unit " + illegalUnit);
}

static class MockDateExprType implements ExprType {
@Override
public String typeName() {
return "DATE";
}
}

static class MockTimeExprType implements ExprType {
@Override
public String typeName() {
return "TIME";
}
}

static class MockTimestampExprType implements ExprType {
@Override
public String typeName() {
return "TIMESTAMP";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
package org.opensearch.sql.ppl;

import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE;
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_DATE_FORMATS;
import static org.opensearch.sql.util.MatcherUtils.rows;
import static org.opensearch.sql.util.MatcherUtils.schema;
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
import static org.opensearch.sql.util.MatcherUtils.verifySchema;
import static org.opensearch.sql.util.MatcherUtils.verifySome;

Expand All @@ -20,6 +22,7 @@ public class DateTimeImplementationIT extends PPLIntegTestCase {
@Override
public void init() throws IOException {
loadIndex(Index.DATE);
loadIndex(Index.DATE_FORMATS);
}

@Test
Expand Down Expand Up @@ -176,4 +179,38 @@ public void nullDateTimeInvalidDateValueMonth() throws IOException {
verifySchema(result, schema("f", null, "datetime"));
verifySome(result.getJSONArray("datarows"), rows(new Object[] {null}));
}

@Test
public void testSpanDatetimeWithCustomFormat() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | eval a = 1 | stats count() as cnt by span(yyyy-MM-dd, 1d) as span",
TEST_INDEX_DATE_FORMATS));
verifySchema(result, schema("cnt", null, "integer"), schema("span", null, "date"));
verifyDataRows(result, rows(2, "1984-04-12"));
}

@Test
public void testSpanDatetimeWithEpochMillisFormat() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | eval a = 1 | stats count() as cnt by span(epoch_millis, 1d) as span",
TEST_INDEX_DATE_FORMATS));
verifySchema(result, schema("cnt", null, "integer"), schema("span", null, "timestamp"));
verifyDataRows(result, rows(2, "1984-04-12 00:00:00"));
}

@Test
public void testSpanDatetimeWithDisjunctiveDifferentFormats() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | eval a = 1 | stats count() as cnt by span(yyyy-MM-dd_OR_epoch_millis,"
+ " 1d) as span",
TEST_INDEX_DATE_FORMATS));
verifySchema(result, schema("cnt", null, "integer"), schema("span", null, "timestamp"));
verifyDataRows(result, rows(2, "1984-04-12 00:00:00"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ private static ExprValue createOpenSearchDateType(Content value, ExprType type)
}
} else {
// custom format
return parseDateTimeString(value.stringValue(), dt);
return parseDateTimeString(value.objectValue().toString(), dt);
}
}
if (value.isString()) {
Expand Down

0 comments on commit 2728f01

Please sign in to comment.