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.

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

Add Minute_Of_Hour Function As An Alias Of Minute Function (#196) #1230

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 0 deletions core/src/main/java/org/opensearch/sql/expression/DSL.java
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ public static FunctionExpression minute_of_day(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.MINUTE_OF_DAY, expressions);
}

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

public static FunctionExpression month(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.MONTH, expressions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ public void register(BuiltinFunctionRepository repository) {
repository.register(makedate());
repository.register(maketime());
repository.register(microsecond());
repository.register(minute());
repository.register(minute(BuiltinFunctionName.MINUTE));
repository.register(minute_of_day());
repository.register(minute(BuiltinFunctionName.MINUTE_OF_HOUR));
repository.register(month(BuiltinFunctionName.MONTH));
repository.register(month(BuiltinFunctionName.MONTH_OF_YEAR));
repository.register(monthName());
Expand Down Expand Up @@ -429,11 +430,12 @@ private DefaultFunctionResolver microsecond() {
/**
* MINUTE(STRING/TIME/DATETIME/TIMESTAMP). return the minute value for time.
*/
private DefaultFunctionResolver minute() {
return define(BuiltinFunctionName.MINUTE.getName(),
private DefaultFunctionResolver minute(BuiltinFunctionName name) {
return define(name.getName(),
impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, STRING),
impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, TIME),
impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, DATETIME),
impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, DATE),
impl(nullMissingHandling(DateTimeFunction::exprMinute), INTEGER, TIMESTAMP)
);
}
Expand Down Expand Up @@ -945,7 +947,8 @@ private ExprValue exprMicrosecond(ExprValue time) {
* @return ExprValue.
*/
private ExprValue exprMinute(ExprValue time) {
return new ExprIntegerValue(time.timeValue().getMinute());
return new ExprIntegerValue(
(MINUTES.between(LocalTime.MIN, time.timeValue()) % 60));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public enum BuiltinFunctionName {
MICROSECOND(FunctionName.of("microsecond")),
MINUTE(FunctionName.of("minute")),
MINUTE_OF_DAY(FunctionName.of("minute_of_day")),
MINUTE_OF_HOUR(FunctionName.of("minute_of_hour")),
MONTH(FunctionName.of("month")),
MONTH_OF_YEAR(FunctionName.of("month_of_year")),
MONTHNAME(FunctionName.of("monthname")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@
import com.google.common.collect.ImmutableList;
import java.time.LocalDate;
import java.util.List;
import java.util.stream.Stream;
import lombok.AllArgsConstructor;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.opensearch.sql.data.model.ExprDateValue;
Expand All @@ -46,6 +50,7 @@
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.ExpressionTestBase;
import org.opensearch.sql.expression.FunctionExpression;
import org.opensearch.sql.expression.LiteralExpression;
import org.opensearch.sql.expression.env.Environment;

@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -727,7 +732,7 @@ private void testMinuteOfDay(String date, int value) {
assertEquals(INTEGER, expression.type());
assertEquals(integerValue(value), eval(expression));
}

@Test
public void minuteOfDay() {
when(nullRef.type()).thenReturn(TIME);
Expand Down Expand Up @@ -764,6 +769,85 @@ public void minuteOfDay() {
testMinuteOfDay("2020-08-17 00:00:01", 0);
}

private void minuteOfHourQuery(FunctionExpression dateExpression, int minute, String testExpr) {
assertAll(
() -> assertEquals(INTEGER, dateExpression.type()),
() -> assertEquals(integerValue(minute), eval(dateExpression)),
() -> assertEquals(testExpr, dateExpression.toString())
);
}

private static Stream<Arguments> getTestDataForMinuteOfHour() {
return Stream.of(
Arguments.of(
DSL.literal(new ExprTimeValue("01:02:03")),
2,
"minute_of_hour(TIME '01:02:03')"),
Arguments.of(
DSL.literal("01:02:03"),
2,
"minute_of_hour(\"01:02:03\")"),
Arguments.of(
DSL.literal(new ExprTimestampValue("2020-08-17 01:02:03")),
2,
"minute_of_hour(TIMESTAMP '2020-08-17 01:02:03')"),
Arguments.of(
DSL.literal(new ExprDatetimeValue("2020-08-17 01:02:03")),
2,
"minute_of_hour(DATETIME '2020-08-17 01:02:03')"),
Arguments.of(
DSL.literal("2020-08-17 01:02:03"),
2,
"minute_of_hour(\"2020-08-17 01:02:03\")")
);
}

@ParameterizedTest(name = "{2}")
@MethodSource("getTestDataForMinuteOfHour")
public void minuteOfHour(LiteralExpression arg, int expectedResult, String expectedString) {
lenient().when(nullRef.valueOf(env)).thenReturn(nullValue());
lenient().when(missingRef.valueOf(env)).thenReturn(missingValue());

minuteOfHourQuery(DSL.minute_of_hour(arg), expectedResult, expectedString);
}

private void invalidMinuteOfHourQuery(String time) {
FunctionExpression expression = DSL.minute_of_hour(DSL.literal(new ExprTimeValue(time)));
eval(expression);
}

@Test
public void minuteOfHourInvalidArguments() {
when(nullRef.type()).thenReturn(TIME);
when(missingRef.type()).thenReturn(TIME);

assertAll(
() -> assertEquals(nullValue(), eval(DSL.minute_of_hour(nullRef))),
() -> assertEquals(missingValue(), eval(DSL.minute_of_hour(missingRef))),

//Invalid Seconds
() -> assertThrows(
SemanticCheckException.class,
() -> invalidMinuteOfHourQuery("12:23:61")),

//Invalid Minutes
() -> assertThrows(
SemanticCheckException.class,
() -> invalidMinuteOfHourQuery("12:61:34")),

//Invalid Hours
() -> assertThrows(
SemanticCheckException.class,
() -> invalidMinuteOfHourQuery("25:23:34")),

//incorrect format
() -> assertThrows(
SemanticCheckException.class,
() -> invalidMinuteOfHourQuery("asdfasdf"))
);
}


@Test
public void month() {
when(nullRef.type()).thenReturn(DATE);
Expand Down
13 changes: 7 additions & 6 deletions docs/user/dql/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1747,20 +1747,21 @@ Description
>>>>>>>>>>>

Usage: minute(time) returns the minute for time, in the range 0 to 59.
The `minute_of_hour` function is provided as an alias.

Argument type: STRING/TIME/DATETIME/TIMESTAMP

Return type: INTEGER

Example::

os> SELECT MINUTE((TIME '01:02:03'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

why change from TIME to time()?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I asked for this, but I recognized later that it is not needed. My mistake.

os> SELECT MINUTE(time('01:02:03')), MINUTE_OF_HOUR(time('01:02:03'))
fetched rows / total rows = 1/1
+-----------------------------+
| MINUTE((TIME '01:02:03')) |
|-----------------------------|
| 2 |
+-----------------------------+
+----------------------------+------------------------------------+
| MINUTE(time('01:02:03')) | MINUTE_OF_HOUR(time('01:02:03')) |
|----------------------------+------------------------------------|
| 2 | 2 |
+----------------------------+------------------------------------+

MINUTE_OF_DAY
------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,52 @@ public void testMinuteOfDay() throws IOException {
verifyDataRows(result, rows(1050));
}

@Test
public void testMinuteOfHour() throws IOException {
JSONObject result = executeQuery("select minute_of_hour(timestamp('2020-09-16 17:30:00'))");
verifySchema(result, schema(
"minute_of_hour(timestamp('2020-09-16 17:30:00'))", null, "integer"));
verifyDataRows(result, rows(30));

result = executeQuery("select minute_of_hour(time('17:30:00'))");
verifySchema(result, schema("minute_of_hour(time('17:30:00'))", null, "integer"));
verifyDataRows(result, rows(30));

result = executeQuery("select minute_of_hour('2020-09-16 17:30:00')");
verifySchema(result, schema("minute_of_hour('2020-09-16 17:30:00')", null, "integer"));
verifyDataRows(result, rows(30));

result = executeQuery("select minute_of_hour('17:30:00')");
verifySchema(result, schema("minute_of_hour('17:30:00')", null, "integer"));
verifyDataRows(result, rows(30));
}

@Test
public void testMinuteFunctionAliasesReturnTheSameResults() throws IOException {
JSONObject result1 = executeQuery("SELECT minute('11:30:00')");
JSONObject result2 = executeQuery("SELECT minute_of_hour('11:30:00')");
verifyDataRows(result1, rows(30));
result1.getJSONArray("datarows").similar(result2.getJSONArray("datarows"));

result1 = executeQuery(String.format(
"SELECT minute(datetime(CAST(time0 AS STRING))) FROM %s", TEST_INDEX_CALCS));
result2 = executeQuery(String.format(
"SELECT minute_of_hour(datetime(CAST(time0 AS STRING))) FROM %s", TEST_INDEX_CALCS));
result1.getJSONArray("datarows").similar(result2.getJSONArray("datarows"));

result1 = executeQuery(String.format(
"SELECT minute(CAST(time0 AS STRING)) FROM %s", TEST_INDEX_CALCS));
result2 = executeQuery(String.format(
"SELECT minute_of_hour(CAST(time0 AS STRING)) FROM %s", TEST_INDEX_CALCS));
result1.getJSONArray("datarows").similar(result2.getJSONArray("datarows"));

result1 = executeQuery(String.format(
"SELECT minute(CAST(datetime0 AS timestamp)) FROM %s", TEST_INDEX_CALCS));
result2 = executeQuery(String.format(
"SELECT minute_of_hour(CAST(datetime0 AS timestamp)) FROM %s", TEST_INDEX_CALCS));
result1.getJSONArray("datarows").similar(result2.getJSONArray("datarows"));
}

@Test
public void testMonth() throws IOException {
JSONObject result = executeQuery("select month(date('2020-09-16'))");
Expand Down
1 change: 1 addition & 0 deletions sql/src/main/antlr/OpenSearchSQLParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ dateTimeFunctionName
| MICROSECOND
| MINUTE
| MINUTE_OF_DAY
| MINUTE_OF_HOUR
| MONTH
| MONTHNAME
| NOW
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,15 @@ public void can_parse_dayofyear_functions() {
assertNotNull(parser.parse("SELECT day_of_year('2022-11-18')"));
}

@Test
public void can_parse_minute_functions() {
assertNotNull(parser.parse("SELECT minute('12:23:34')"));
assertNotNull(parser.parse("SELECT minute_of_hour('12:23:34')"));

assertNotNull(parser.parse("SELECT minute('2022-12-20 12:23:34')"));
assertNotNull(parser.parse("SELECT minute_of_hour('2022-12-20 12:23:34')"));
}

@Test
public void can_parse_month_of_year_function() {
assertNotNull(parser.parse("SELECT month('2022-11-18')"));
Expand Down