Skip to content
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

Update DATE_ADD/ADDDATE and DATE_SUB/SUBDATE functions. #122

Merged
merged 12 commits into from
Dec 16, 2022
Merged

Update DATE_ADD/ADDDATE and DATE_SUB/SUBDATE functions. #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8986a60
Rework on `DATE_ADD`/`ADDDATE` and `DATE_SUB`/`SUBDATE` functions.
Yury-Fridlyand Oct 7, 2022
f44782c
Update function implementations according to PR feedback.
Yury-Fridlyand Oct 11, 2022
6e7c86b
Split UT cases into smaller ones.
Yury-Fridlyand Oct 11, 2022
b0557aa
Correct mistakes in docs.
Yury-Fridlyand Oct 12, 2022
131b9ae
Fix switching timezone in datetime ITs.
Yury-Fridlyand Oct 12, 2022
9845ad3
Fix conversion to timestamp.
Yury-Fridlyand Oct 14, 2022
1b33a1c
Address PR feedback.
Yury-Fridlyand Oct 18, 2022
b19ea96
Update implementation and tests to use `FunctionProperties`.
Yury-Fridlyand Dec 8, 2022
734a822
Minor improvements.
Yury-Fridlyand Dec 10, 2022
02576bc
Minor improvements.
Yury-Fridlyand Dec 10, 2022
c206b9d
Address PR feedback.
Yury-Fridlyand Dec 13, 2022
8a452c9
Address PR feedback.
Yury-Fridlyand Dec 13, 2022
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
3 changes: 1 addition & 2 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 @@ -13,7 +13,6 @@
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
Expand Down Expand Up @@ -69,7 +68,7 @@ public LocalDateTime datetimeValue() {

@Override
public Instant timestampValue() {
return ZonedDateTime.of(date, timeValue(), ZoneId.systemDefault()).toInstant();
return ZonedDateTime.of(date, timeValue(), ExprTimestampValue.ZONE).toInstant();
}

@Override
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/org/opensearch/sql/data/model/ExprDatetimeValue.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.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
Expand Down Expand Up @@ -71,7 +70,7 @@ public LocalTime timeValue() {

@Override
public Instant timestampValue() {
return ZonedDateTime.of(datetime, ZoneId.of("UTC")).toInstant();
return ZonedDateTime.of(datetime, ExprTimestampValue.ZONE).toInstant();
}

@Override
Expand Down
15 changes: 14 additions & 1 deletion 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 @@ -13,14 +13,14 @@
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeParseException;
import java.util.Objects;
import lombok.RequiredArgsConstructor;
import org.opensearch.sql.data.type.ExprCoreType;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.expression.function.FunctionProperties;

/**
* Expression Time Value.
Expand Down Expand Up @@ -57,6 +57,19 @@ public LocalTime timeValue() {
return time;
}

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(), ExprTimestampValue.ZONE)
.toInstant();
}

@Override
public String toString() {
return String.format("TIME '%s'", value());
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -30,7 +30,7 @@ public class ExprTimestampValue extends AbstractExprValue {
/**
* todo. only support UTC now.
*/
private static final ZoneId ZONE = ZoneId.of("UTC");
public static final ZoneId ZONE = ZoneId.of("UTC");
Copy link

Choose a reason for hiding this comment

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

Maybe specify that ZONE is UTC_ZONE?


private final Instant timestamp;

Expand Down
30 changes: 30 additions & 0 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 @@ -6,6 +6,10 @@

package org.opensearch.sql.data.model;

import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.temporal.TemporalAmount;
import java.util.ArrayList;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -61,6 +65,22 @@ public static ExprValue intervalValue(TemporalAmount value) {
return new ExprIntervalValue(value);
}

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);
}

public static ExprValue timestampValue(Instant value) {
return new ExprTimestampValue(value);
}

/**
* {@link ExprTupleValue} constructor.
*/
Expand Down Expand Up @@ -115,6 +135,16 @@ public static ExprValue fromObjectValue(Object o) {
return stringValue((String) o);
} else if (o instanceof Float) {
return floatValue((Float) o);
} else if (o instanceof LocalDate) {
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
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 {
throw new ExpressionEvaluationException("unsupported object " + o.getClass());
}
Expand Down
16 changes: 0 additions & 16 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 @@ -290,10 +290,6 @@ public static FunctionExpression multiply(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.MULTIPLY, expressions);
}

public static FunctionExpression adddate(Expression... expressions) {
Copy link

Choose a reason for hiding this comment

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

What was the reason for getting rid of all of these functions? Were they simply not used?

Copy link
Author

Choose a reason for hiding this comment

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

  1. They were used in tests only
  2. I rewrote tests and made new DSL definitions for those functions in DateTimeTestBase
  3. These functions require a FunctionProperties object as of now, they crash on FunctionProperties.None

return compile(FunctionProperties.None, BuiltinFunctionName.ADDDATE, expressions);
}

public static FunctionExpression convert_tz(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.CONVERT_TZ, expressions);
}
Expand All @@ -306,14 +302,6 @@ public static FunctionExpression datetime(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.DATETIME, expressions);
}

public static FunctionExpression date_add(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.DATE_ADD, expressions);
}

public static FunctionExpression date_sub(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.DATE_SUB, expressions);
}

public static FunctionExpression day(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.DAY, expressions);
}
Expand Down Expand Up @@ -374,10 +362,6 @@ public static FunctionExpression second(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.SECOND, expressions);
}

public static FunctionExpression subdate(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.SUBDATE, expressions);
}

public static FunctionExpression time(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.TIME, expressions);
}
Expand Down
Loading