Skip to content

Remove Datetime data type #336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matthewryanwells merged 14 commits into from
Aug 17, 2023
Merged

Remove Datetime data type #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b3317d5
removed datetime type, updated unit tests
matthewryanwells Aug 8, 2023
4b5684d
begun updating integ tests and jacoco coverage
matthewryanwells Aug 9, 2023
9410c31
finished updating tests, and completed test coverage
matthewryanwells Aug 9, 2023
5492a80
rebased and fixed tests
matthewryanwells Aug 10, 2023
983b679
removed duplicate data types, updated documentation to add back in da…
matthewryanwells Aug 10, 2023
54a11d3
Addressed comments including readding removed tests, removing duplica…
matthewryanwells Aug 10, 2023
6dc03bd
updated doctest, updated integ test
matthewryanwells Aug 11, 2023
4907017
updated failing integration tests, fixed indentation in ppl/DateTimeF…
matthewryanwells Aug 11, 2023
144ed84
applied spotless to Datetimevaluetest
matthewryanwells Aug 11, 2023
d70f065
rebasing upstream changes and fixing formatting
matthewryanwells Aug 14, 2023
d4e752e
Empty commit to fix rebase
matthewryanwells Aug 14, 2023
20798c1
delete ExprDatetimeValue file
matthewryanwells Aug 14, 2023
32e4ee7
fixed mistake in string timestamp casting, updated tests with fix, fi…
matthewryanwells Aug 14, 2023
f52cc8a
simplified expr string type timestamp cast, fixed checkstyle failure
matthewryanwells Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions core/src/main/java/org/opensearch/sql/data/model/ExprDateValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import com.google.common.base.Objects;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
Expand Down Expand Up @@ -57,11 +56,6 @@ public LocalTime timeValue() {
return LocalTime.of(0, 0, 0);
}

@Override
public LocalDateTime datetimeValue() {
return LocalDateTime.of(date, timeValue());
}

@Override
public Instant timestampValue() {
return ZonedDateTime.of(date, timeValue(), UTC_ZONE_ID).toInstant();
Expand Down
99 changes: 0 additions & 99 deletions core/src/main/java/org/opensearch/sql/data/model/ExprDatetimeValue.java
View file
Open in desktop

This file was deleted.

22 changes: 8 additions & 14 deletions core/src/main/java/org/opensearch/sql/data/model/ExprStringValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package org.opensearch.sql.data.model;

import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
Expand Down Expand Up @@ -35,27 +36,20 @@ public String stringValue() {
}

@Override
public LocalDateTime datetimeValue() {
public Instant timestampValue() {
try {
return new ExprDatetimeValue(value).datetimeValue();
return new ExprTimestampValue(value).timestampValue();
} catch (SemanticCheckException e) {
try {
return new ExprDatetimeValue(
LocalDateTime.of(new ExprDateValue(value).dateValue(), LocalTime.of(0, 0, 0)))
.datetimeValue();
} catch (SemanticCheckException exception) {
throw new SemanticCheckException(
String.format(
"datetime:%s in unsupported format, please use 'yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]'",
value));
}
return new ExprTimestampValue(
LocalDateTime.of(new ExprDateValue(value).dateValue(), LocalTime.of(0, 0, 0)))
.timestampValue();
}
}

@Override
public LocalDate dateValue() {
try {
return new ExprDatetimeValue(value).dateValue();
return new ExprTimestampValue(value).dateValue();
} catch (SemanticCheckException e) {
return new ExprDateValue(value).dateValue();
}
Expand All @@ -64,7 +58,7 @@ public LocalDate dateValue() {
@Override
public LocalTime timeValue() {
try {
return new ExprDatetimeValue(value).timeValue();
return new ExprTimestampValue(value).timeValue();
} catch (SemanticCheckException e) {
return new ExprTimeValue(value).timeValue();
}
Expand Down
5 changes: 0 additions & 5 deletions core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZonedDateTime;
import java.time.format.DateTimeParseException;
Expand Down Expand Up @@ -57,10 +56,6 @@ public LocalDate dateValue(FunctionProperties functionProperties) {
return LocalDate.now(functionProperties.getQueryStartClock());
}

public LocalDateTime datetimeValue(FunctionProperties functionProperties) {
return LocalDateTime.of(dateValue(functionProperties), timeValue());
}

public Instant timestampValue(FunctionProperties functionProperties) {
return ZonedDateTime.of(dateValue(functionProperties), timeValue(), UTC_ZONE_ID).toInstant();
}
Expand Down
10 changes: 5 additions & 5 deletions core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ public ExprTimestampValue(String timestamp) {
}
}

/** localDateTime Constructor. */
public ExprTimestampValue(LocalDateTime localDateTime) {
this.timestamp = localDateTime.atZone(UTC_ZONE_ID).toInstant();
}

@Override
public String value() {
return timestamp.getNano() == 0
Expand Down Expand Up @@ -71,11 +76,6 @@ public LocalTime timeValue() {
return timestamp.atZone(UTC_ZONE_ID).toLocalTime();
}

@Override
public LocalDateTime datetimeValue() {
return timestamp.atZone(UTC_ZONE_ID).toLocalDateTime();
}

@Override
public boolean isDateTime() {
return true;
Expand Down
7 changes: 0 additions & 7 deletions core/src/main/java/org/opensearch/sql/data/model/ExprValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import java.io.Serializable;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.temporal.TemporalAmount;
import java.util.List;
Expand Down Expand Up @@ -133,12 +132,6 @@ default LocalDate dateValue() {
"invalid to get dateValue from value of type " + type());
}

/** Get datetime value. */
default LocalDateTime datetimeValue() {
throw new ExpressionEvaluationException(
"invalid to get datetimeValue from value of type " + type());
}

/** Get interval value. */
default TemporalAmount intervalValue() {
throw new ExpressionEvaluationException(
Expand Down
11 changes: 3 additions & 8 deletions core/src/main/java/org/opensearch/sql/data/model/ExprValueUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneOffset;
import java.time.temporal.TemporalAmount;
import java.util.ArrayList;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -66,10 +67,6 @@ public static ExprValue dateValue(LocalDate value) {
return new ExprDateValue(value);
}

public static ExprValue datetimeValue(LocalDateTime value) {
return new ExprDatetimeValue(value);
}

public static ExprValue timeValue(LocalTime value) {
return new ExprTimeValue(value);
}
Expand Down Expand Up @@ -128,14 +125,14 @@ public static ExprValue fromObjectValue(Object o) {
return floatValue((Float) o);
} else if (o instanceof LocalDate) {
return dateValue((LocalDate) o);
} else if (o instanceof LocalDateTime) {
return datetimeValue((LocalDateTime) o);
} else if (o instanceof LocalTime) {
return timeValue((LocalTime) o);
} else if (o instanceof Instant) {
return timestampValue((Instant) o);
} else if (o instanceof TemporalAmount) {
return intervalValue((TemporalAmount) o);
} else if (o instanceof LocalDateTime) {
return timestampValue(((LocalDateTime) o).toInstant(ZoneOffset.UTC));
} else {
throw new ExpressionEvaluationException("unsupported object " + o.getClass());
}
Expand All @@ -150,8 +147,6 @@ public static ExprValue fromObjectValue(Object o, ExprCoreType type) {
return new ExprDateValue((String) o);
case TIME:
return new ExprTimeValue((String) o);
case DATETIME:
return new ExprDatetimeValue((String) o);
default:
return fromObjectValue(o);
}
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ public enum ExprCoreType implements ExprType {
/** Date. */
DATE(STRING),
TIME(STRING),
DATETIME(STRING, DATE, TIME),
TIMESTAMP(STRING, DATETIME),
TIMESTAMP(STRING, DATE, TIME),
INTERVAL(UNDEFINED),

/** Struct. */
Expand Down
4 changes: 0 additions & 4 deletions core/src/main/java/org/opensearch/sql/expression/DSL.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,10 +819,6 @@ public static FunctionExpression castTimestamp(Expression value) {
return compile(FunctionProperties.None, BuiltinFunctionName.CAST_TO_TIMESTAMP, value);
}

public static FunctionExpression castDatetime(Expression value) {
Copy link

@GumpacG GumpacG Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change?

Copy link
Author

@matthewryanwells matthewryanwells Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this would be as there is no such thing as a datetime value to cast to anymore, unless it returns a timestamp which sounds unreasonable as we already have the castTimestamp to do that

return compile(FunctionProperties.None, BuiltinFunctionName.CAST_TO_DATETIME, value);
}

public static FunctionExpression typeof(Expression value) {
return compile(FunctionProperties.None, BuiltinFunctionName.TYPEOF, value);
}
Expand Down
10 changes: 0 additions & 10 deletions core/src/main/java/org/opensearch/sql/expression/aggregation/AggregatorFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import static org.opensearch.sql.data.type.ExprCoreType.ARRAY;
import static org.opensearch.sql.data.type.ExprCoreType.DATE;
import static org.opensearch.sql.data.type.ExprCoreType.DATETIME;
import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE;
import static org.opensearch.sql.data.type.ExprCoreType.FLOAT;
import static org.opensearch.sql.data.type.ExprCoreType.INTEGER;
Expand Down Expand Up @@ -71,9 +70,6 @@ private static DefaultFunctionResolver avg() {
.put(
new FunctionSignature(functionName, Collections.singletonList(DATE)),
(functionProperties, arguments) -> new AvgAggregator(arguments, DATE))
.put(
new FunctionSignature(functionName, Collections.singletonList(DATETIME)),
(functionProperties, arguments) -> new AvgAggregator(arguments, DATETIME))
.put(
new FunctionSignature(functionName, Collections.singletonList(TIME)),
(functionProperties, arguments) -> new AvgAggregator(arguments, TIME))
Expand Down Expand Up @@ -142,9 +138,6 @@ private static DefaultFunctionResolver min() {
.put(
new FunctionSignature(functionName, Collections.singletonList(DATE)),
(functionProperties, arguments) -> new MinAggregator(arguments, DATE))
.put(
new FunctionSignature(functionName, Collections.singletonList(DATETIME)),
(functionProperties, arguments) -> new MinAggregator(arguments, DATETIME))
.put(
new FunctionSignature(functionName, Collections.singletonList(TIME)),
(functionProperties, arguments) -> new MinAggregator(arguments, TIME))
Expand Down Expand Up @@ -177,9 +170,6 @@ private static DefaultFunctionResolver max() {
.put(
new FunctionSignature(functionName, Collections.singletonList(DATE)),
(functionProperties, arguments) -> new MaxAggregator(arguments, DATE))
.put(
new FunctionSignature(functionName, Collections.singletonList(DATETIME)),
(functionProperties, arguments) -> new MaxAggregator(arguments, DATETIME))
.put(
new FunctionSignature(functionName, Collections.singletonList(TIME)),
(functionProperties, arguments) -> new MaxAggregator(arguments, TIME))
Expand Down
25 changes: 0 additions & 25 deletions core/src/main/java/org/opensearch/sql/expression/aggregation/AvgAggregator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.util.List;
import java.util.Locale;
import org.opensearch.sql.data.model.ExprDateValue;
import org.opensearch.sql.data.model.ExprDatetimeValue;
import org.opensearch.sql.data.model.ExprDoubleValue;
import org.opensearch.sql.data.model.ExprIntegerValue;
import org.opensearch.sql.data.model.ExprNullValue;
Expand Down Expand Up @@ -47,8 +46,6 @@ public AvgState create() {
switch (dataType) {
case DATE:
return new DateAvgState();
case DATETIME:
return new DateTimeAvgState();
case TIMESTAMP:
return new TimestampAvgState();
case TIME:
Expand Down Expand Up @@ -128,28 +125,6 @@ protected AvgState iterate(ExprValue value) {
}
}

protected static class DateTimeAvgState extends AvgState {
@Override
public ExprValue result() {
if (0 == count.integerValue()) {
return ExprNullValue.of();
}

return new ExprDatetimeValue(
new ExprTimestampValue(
Instant.ofEpochMilli(
DSL.divide(DSL.literal(total), DSL.literal(count)).valueOf().longValue()))
.datetimeValue());
}

@Override
protected AvgState iterate(ExprValue value) {
total =
DSL.add(DSL.literal(total), DSL.literal(value.timestampValue().toEpochMilli())).valueOf();
return super.iterate(value);
}
}

protected static class TimestampAvgState extends AvgState {
@Override
public ExprValue result() {
Expand Down
Loading