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 TIME and DATE functions #134

Merged
merged 18 commits into from
Oct 31, 2022
Merged

Update TIME and DATE functions #134

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
65c34ac
Added test cases and support for strict date validation. Added abilit…
MitchellGale Oct 13, 2022
4a4e425
Update `ExprTimeValue` by `datetimeValue` and other interfaces. Add c…
Yury-Fridlyand Oct 13, 2022
da22234
core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFun…
Yury-Fridlyand Oct 13, 2022
e421349
Fix conversion to timestamp.
Yury-Fridlyand Oct 14, 2022
26c1165
Added try catch to return null for invalid date in DATE
MitchellGale Oct 17, 2022
be3ea22
Added null test for TIME and DATE
MitchellGale Oct 17, 2022
4f7edd1
Added exprDate to accept time.
MitchellGale Oct 17, 2022
a00863c
Accept date time for time and date
MitchellGale Oct 18, 2022
99c98b0
Adding coverage
MitchellGale Oct 19, 2022
f9e38e9
Fixed code coverage for time passed to date function case.
MitchellGale Oct 19, 2022
2016222
Adding more test cases for datetime and milliseconds for DATE and TIM…
MitchellGale Oct 19, 2022
4c0928d
Removed unused imports.
MitchellGale Oct 19, 2022
95c5e5c
Removed unused DATETIMEFORMATTERS.
MitchellGale Oct 20, 2022
61289bb
Added support for HH:mm for time/datetime inputs. Removed call for lo…
MitchellGale Oct 25, 2022
310f653
Added more doc tests.
MitchellGale Oct 27, 2022
484b28c
Removed blocks from cherry-picked PR.
MitchellGale Oct 28, 2022
157059f
Reverted change in ExprDateTimeValue
MitchellGale Oct 28, 2022
77bd46e
Reverted reverted changes in ExprDateTimeValue. Reverted change in Ex…
MitchellGale Oct 28, 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
4 changes: 3 additions & 1 deletion 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 @@ -6,6 +6,8 @@

package org.opensearch.sql.data.model;

import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL;

import com.google.common.base.Objects;
import java.time.Instant;
import java.time.LocalDate;
Expand Down Expand Up @@ -33,7 +35,7 @@ public class ExprDateValue extends AbstractExprValue {
*/
public ExprDateValue(String date) {
try {
this.date = LocalDate.parse(date);
this.date = LocalDate.parse(date, DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL);
} catch (DateTimeParseException e) {
throw new SemanticCheckException(String.format("date:%s in unsupported format, please use "
+ "yyyy-MM-dd", date));
Expand Down
27 changes: 24 additions & 3 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 @@ -6,9 +6,14 @@

package org.opensearch.sql.data.model;

import static org.opensearch.sql.utils.DateTimeFormatters.TIME_FORMATTER_VARIABLE_NANOS;
import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL;

import java.time.Instant;
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;
import java.util.Objects;
Expand All @@ -22,14 +27,15 @@
*/
@RequiredArgsConstructor
public class ExprTimeValue extends AbstractExprValue {

private final LocalTime time;

/**
* Constructor.
* Constructor of ExprTimeValue.
*/
public ExprTimeValue(String time) {
try {
this.time = LocalTime.parse(time, TIME_FORMATTER_VARIABLE_NANOS);
this.time = LocalTime.parse(time, DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL);
} catch (DateTimeParseException e) {
throw new SemanticCheckException(String.format("time:%s in unsupported format, please use "
+ "HH:mm:ss[.SSSSSSSSS]", time));
Expand All @@ -51,6 +57,21 @@ public LocalTime timeValue() {
return time;
}

@Override
public LocalDate dateValue() {
return LocalDate.now();
}

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

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

@Override
public String toString() {
return String.format("TIME '%s'", value());
Expand Down
8 changes: 7 additions & 1 deletion core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import org.opensearch.sql.data.model.ExprValue;
import org.opensearch.sql.data.type.ExprCoreType;
import org.opensearch.sql.exception.ExpressionEvaluationException;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.expression.function.BuiltinFunctionName;
import org.opensearch.sql.expression.function.BuiltinFunctionRepository;
import org.opensearch.sql.expression.function.DefaultFunctionResolver;
Expand Down Expand Up @@ -651,7 +652,11 @@ private ExprValue exprConvertTZ(ExprValue startingDateTime, ExprValue fromTz, Ex
*/
private ExprValue exprDate(ExprValue exprValue) {
if (exprValue instanceof ExprStringValue) {
return new ExprDateValue(exprValue.stringValue());
try {
Copy link

Choose a reason for hiding this comment

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

We should be able to remove the if statement and get the same result. The try...catch will cover both cases.

Copy link
Author

Choose a reason for hiding this comment

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

I think we still need it because it can be a stringValue or a dateValue.

Copy link

Choose a reason for hiding this comment

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

is there a case where an ExprStringValue doesn't have a stringValue()?

return new ExprDateValue(exprValue.stringValue());
} catch (SemanticCheckException e) {
return ExprNullValue.of();
}
} else {
return new ExprDateValue(exprValue.dateValue());
}
Expand Down Expand Up @@ -944,6 +949,7 @@ private ExprValue exprSubDateInterval(ExprValue date, ExprValue expr) {
*/
private ExprValue exprTime(ExprValue exprValue) {
if (exprValue instanceof ExprStringValue) {
//try catch
Copy link

Choose a reason for hiding this comment

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

todo?

return new ExprTimeValue(exprValue.stringValue());
} else {
return new ExprTimeValue(exprValue.timeValue());
Expand Down
16 changes: 14 additions & 2 deletions core/src/main/java/org/opensearch/sql/utils/DateTimeFormatters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,25 @@ public class DateTimeFormatters {

public static final DateTimeFormatter DATE_TIME_FORMATTER_VARIABLE_NANOS =
new DateTimeFormatterBuilder()
.appendPattern("yyyy-MM-dd HH:mm:ss")
.appendPattern("uuuu-MM-dd HH:mm:ss")
.appendFraction(
ChronoField.NANO_OF_SECOND,
MIN_FRACTION_SECONDS,
MAX_FRACTION_SECONDS,
true)
.toFormatter(Locale.ROOT);
.toFormatter(Locale.ROOT)
.withResolverStyle(ResolverStyle.STRICT);

public static final DateTimeFormatter DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL =
new DateTimeFormatterBuilder()
.appendPattern("[uuuu-MM-dd HH:mm:ss][HH:mm:ss][uuuu-MM-dd]")
.appendFraction(
ChronoField.NANO_OF_SECOND,
MIN_FRACTION_SECONDS,
MAX_FRACTION_SECONDS,
true)
.toFormatter(Locale.ROOT)
.withResolverStyle(ResolverStyle.STRICT);

public static final DateTimeFormatter TIME_FORMATTER_VARIABLE_NANOS =
new DateTimeFormatterBuilder()
Expand Down
4 changes: 4 additions & 0 deletions core/src/test/java/org/opensearch/sql/data/model/DateTimeValueTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ public void timeValueInterfaceTest() {

assertEquals(TIME, timeValue.type());
assertEquals(LocalTime.parse("01:01:01"), timeValue.timeValue());
assertEquals(LocalDate.now(), timeValue.dateValue());
assertEquals(LocalDate.now().atTime(1, 1, 1), timeValue.datetimeValue());
assertEquals(ZonedDateTime.of(LocalTime.parse("01:01:01").atDate(LocalDate.now()),
ZoneId.systemDefault()).toInstant(), timeValue.timestampValue());
assertEquals("01:01:01", timeValue.value());
assertEquals("TIME '01:01:01'", timeValue.toString());
assertThrows(ExpressionEvaluationException.class, () -> integerValue(1).timeValue(),
Expand Down
10 changes: 10 additions & 0 deletions core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ public void date() {
assertEquals(DATE, expr.type());
assertEquals(new ExprDateValue("2020-08-17"), eval(expr));
assertEquals("date(DATE '2020-08-17')", expr.toString());

expr = dsl.date(DSL.literal(new ExprDateValue("2020-08-17 12:12:00")));
Copy link

Choose a reason for hiding this comment

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

we should test with nano seconds too

assertEquals(DATE, expr.type());
assertEquals(new ExprDateValue("2020-08-17 12:12:00"), eval(expr));
assertEquals("date(DATE '2020-08-17')", expr.toString());
}

@Test
Expand Down Expand Up @@ -795,6 +800,11 @@ public void time() {
assertEquals(TIME, expr.type());
assertEquals(new ExprTimeValue("01:01:01"), eval(expr));
assertEquals("time(TIME '01:01:01')", expr.toString());

expr = dsl.time(DSL.literal(new ExprTimeValue("2020-01-02 01:01:01")));
Copy link

Choose a reason for hiding this comment

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

we should test with nano seconds too (separately)

Copy link
Author

Choose a reason for hiding this comment

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

Added case for times < 1.0 seconds. 2016222

assertEquals(TIME, expr.type());
assertEquals(new ExprTimeValue("2020-01-02 01:01:01"), eval(expr));
assertEquals("time(TIME '01:01:01')", expr.toString());
}

@Test
Expand Down
24 changes: 12 additions & 12 deletions docs/user/dql/functions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1076,13 +1076,13 @@ Return type: DATE

Example::

>od SELECT DATE('2020-08-26'), DATE(TIMESTAMP('2020-08-26 13:49:00'))
os> SELECT DATE('2020-08-26'), DATE(TIMESTAMP('2020-08-26 13:49:00')), DATE('2020-08-26 13:49:00')
fetched rows / total rows = 1/1
+----------------------+------------------------------------------+
| DATE('2020-08-26') | DATE(TIMESTAMP('2020-08-26 13:49:00')) |
|----------------------+------------------------------------------|
| DATE '2020-08-26' | DATE '2020-08-26' |
+----------------------+------------------------------------------+
+----------------------+------------------------------------------+-------------------------------+
| DATE('2020-08-26') | DATE(TIMESTAMP('2020-08-26 13:49:00')) | DATE('2020-08-26 13:49:00') |
|----------------------+------------------------------------------+-------------------------------|
| 2020-08-26 | 2020-08-26 | 2020-08-26 |
+----------------------+------------------------------------------+-------------------------------+


DATETIME
Expand Down Expand Up @@ -1880,13 +1880,13 @@ Return type: TIME

Example::

>od SELECT TIME('13:49:00'), TIME(TIMESTAMP('2020-08-26 13:49:00'))
os> SELECT TIME('13:49:00'), TIME(TIMESTAMP('2020-08-26 13:49:00')), TIME('2020-08-26 13:49:00')
fetched rows / total rows = 1/1
+--------------------+------------------------------------------+
| TIME('13:49:00') | TIME(TIMESTAMP('2020-08-26 13:49:00')) |
|--------------------+------------------------------------------|
| TIME '13:49:00' | TIME '13:49:00' |
+--------------------+------------------------------------------+
+--------------------+------------------------------------------+-------------------------------+
| TIME('13:49:00') | TIME(TIMESTAMP('2020-08-26 13:49:00')) | TIME('2020-08-26 13:49:00') |
|--------------------+------------------------------------------+-------------------------------|
| 13:49:00 | 13:49:00 | 13:49:00 |
+--------------------+------------------------------------------+-------------------------------+


TIME_TO_SEC
Expand Down
24 changes: 12 additions & 12 deletions docs/user/ppl/functions/datetime.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,13 @@ Return type: DATE

Example::

>od source=people | eval `DATE('2020-08-26')` = DATE('2020-08-26'), `DATE(TIMESTAMP('2020-08-26 13:49:00'))` = DATE(TIMESTAMP('2020-08-26 13:49:00')) | fields `DATE('2020-08-26')`, `DATE(TIMESTAMP('2020-08-26 13:49:00'))`
os> source=people | eval `DATE('2020-08-26')` = DATE('2020-08-26'), `DATE(TIMESTAMP('2020-08-26 13:49:00'))` = DATE(TIMESTAMP('2020-08-26 13:49:00')), `DATE('2020-08-26 13:49:00')` = DATE('2020-08-26 13:49:00') | fields `DATE('2020-08-26')`, `DATE(TIMESTAMP('2020-08-26 13:49:00'))`, `DATE('2020-08-26 13:49:00')`
fetched rows / total rows = 1/1
+----------------------+------------------------------------------+
| DATE('2020-08-26') | DATE(TIMESTAMP('2020-08-26 13:49:00')) |
|----------------------+------------------------------------------|
| DATE '2020-08-26' | DATE '2020-08-26' |
+----------------------+------------------------------------------+
+----------------------+------------------------------------------+-------------------------------+
| DATE('2020-08-26') | DATE(TIMESTAMP('2020-08-26 13:49:00')) | DATE('2020-08-26 13:49:00') |
|----------------------+------------------------------------------+-------------------------------|
| 2020-08-26 | 2020-08-26 | 2020-08-26 |
+----------------------+------------------------------------------+-------------------------------+


DATE_ADD
Expand Down Expand Up @@ -1052,13 +1052,13 @@ Return type: TIME

Example::

>od source=people | eval `TIME('13:49:00')` = TIME('13:49:00'), `TIME(TIMESTAMP('2020-08-26 13:49:00'))` = TIME(TIMESTAMP('2020-08-26 13:49:00')) | fields `TIME('13:49:00')`, `TIME(TIMESTAMP('2020-08-26 13:49:00'))`
>od source=people | eval `TIME('13:49:00')` = TIME('13:49:00'), `TIME(TIMESTAMP('2020-08-26 13:49:00'))` = TIME(TIMESTAMP('2020-08-26 13:49:00')), `TIME('2020-08-26 13:49:00')` = TIME('2020-08-26 13:49:00') | fields `TIME('13:49:00')`, `TIME(TIMESTAMP('2020-08-26 13:49:00'))`, `TIME('2020-08-26 13:49:00')`
fetched rows / total rows = 1/1
+--------------------+------------------------------------------+
| TIME('13:49:00') | TIME(TIMESTAMP('2020-08-26 13:49:00')) |
|--------------------+------------------------------------------|
| TIME '13:49:00' | TIME '13:49:00' |
+--------------------+------------------------------------------+
+--------------------+------------------------------------------+-------------------------------+
| TIME('13:49:00') | TIME(TIMESTAMP('2020-08-26 13:49:00')) | TIME('2020-08-26 13:49:00') |
|--------------------+------------------------------------------+-------------------------------|
| 13:49:00 | 13:49:00 | 13:49:00 |
+--------------------+------------------------------------------+-------------------------------+


TIME_TO_SEC
Expand Down