Skip to content

Commit

Permalink
Move formatting logic inside opensearch module and address minor PR c…
Browse files Browse the repository at this point in the history 
…omments

Signed-off-by: Manasvini B S <[email protected]>
  • Loading branch information
@manasvinibs
manasvinibs committed Jul 16, 2024
1 parent 2952233 commit f725346
Show file tree
Hide file tree
Showing 15 changed files with 337 additions and 606 deletions.
1 change: 0 additions & 1 deletion core/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ dependencies {
api "com.fasterxml.jackson.core:jackson-annotations:${versions.jackson}"
api group: 'com.google.code.gson', name: 'gson', version: '2.8.9'
api group: 'com.tdunning', name: 't-digest', version: '3.3'
api group: 'org.opensearch', name: 'opensearch', version: "${opensearch_version}"
api project(':common')

testImplementation('org.junit.jupiter:junit-jupiter:5.9.3')
Expand Down
43 changes: 2 additions & 41 deletions core/src/main/java/org/opensearch/sql/data/model/ExprDateValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,57 +15,21 @@
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.opensearch.common.time.DateFormatter;
import org.opensearch.common.time.DateFormatters;
import org.opensearch.sql.data.type.ExprCoreType;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.utils.DateTimeFormatters;

/** Expression Date Value. */
@RequiredArgsConstructor
public class ExprDateValue extends AbstractExprValue {

private final LocalDate date;

/** Formatted date using user defined/default format */
private String formattedDate;

/** Constructor of ExprDateValue. */
public ExprDateValue(String date) {
this(date, List.of());
}

/** Constructor of ExprDateValue to support custom/OpenSearch date formats in mappings. */
public ExprDateValue(String date, List<DateFormatter> dateFormatters) {
try {
LocalDate localDate = null;
// check if dateFormatters are empty, then set default ones
if (dateFormatters == null || dateFormatters.isEmpty()) {
dateFormatters = DateTimeFormatters.initializeDateFormatters();
}
// parse using OpenSearch DateFormatters
for (DateFormatter formatter : dateFormatters) {
try {
TemporalAccessor accessor = formatter.parse(date);
ZonedDateTime zonedDateTime = DateFormatters.from(accessor);
localDate = zonedDateTime.withZoneSameLocal(ZoneOffset.UTC).toLocalDate();

this.formattedDate = formatter.format(accessor);
break;
} catch (IllegalArgumentException ignored) {
// nothing to do, try another format
}
}
if (localDate == null) {
// Default constructor behavior, parse using java DateTimeFormatter
localDate = LocalDate.parse(date, DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL);
}
this.date = localDate;

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 All @@ -74,10 +38,7 @@ public ExprDateValue(String date, List<DateFormatter> dateFormatters) {

@Override
public String value() {
if (this.formattedDate == null) {
return DateTimeFormatter.ISO_LOCAL_DATE.format(date);
}
return this.formattedDate;
return DateTimeFormatter.ISO_LOCAL_DATE.format(date);
}

@Override
Expand Down
41 changes: 2 additions & 39 deletions core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,57 +14,23 @@
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.util.List;
import java.util.Objects;
import lombok.RequiredArgsConstructor;
import org.opensearch.common.time.DateFormatter;
import org.opensearch.common.time.DateFormatters;
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;
import org.opensearch.sql.utils.DateTimeFormatters;

/** Expression Time Value. */
@RequiredArgsConstructor
public class ExprTimeValue extends AbstractExprValue {

private final LocalTime time;

/** Formatted time using user defined/default format */
private String formattedTime;

/** Constructor of ExprTimeValue. */
public ExprTimeValue(String time) {
this(time, List.of());
}

/** Constructor of ExprTimeValue to support custom/OpenSearch date formats in mappings. */
public ExprTimeValue(String time, List<DateFormatter> dateFormatters) {
try {
LocalTime localTime = null;
// check if dateFormatters are empty, then set default ones
if (dateFormatters == null || dateFormatters.isEmpty()) {
dateFormatters = DateTimeFormatters.initializeDateFormatters();
}
// parse using OpenSearch DateFormatters
for (DateFormatter formatter : dateFormatters) {
try {
TemporalAccessor accessor = formatter.parse(time);
ZonedDateTime zonedDateTime = DateFormatters.from(accessor);
localTime = zonedDateTime.withZoneSameLocal(ZoneOffset.UTC).toLocalTime();
this.formattedTime = formatter.format(accessor);
break;
} catch (IllegalArgumentException ignored) {
// nothing to do, try another format
}
}
if (localTime == null) {
// Default constructor behavior, parse using java DateTimeFormatter
localTime = LocalTime.parse(time, DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL);
}
this.time = localTime;
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 @@ -73,10 +39,7 @@ public ExprTimeValue(String time, List<DateFormatter> dateFormatters) {

@Override
public String value() {
if (this.formattedTime == null) {
return ISO_LOCAL_TIME.format(time);
}
return this.formattedTime;
return ISO_LOCAL_TIME.format(time);
}

@Override
Expand Down
63 changes: 9 additions & 54 deletions core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,64 +12,27 @@
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeParseException;
import java.time.temporal.ChronoUnit;
import java.time.temporal.TemporalAccessor;
import java.util.List;
import java.util.Objects;
import lombok.RequiredArgsConstructor;
import org.opensearch.common.time.DateFormatter;
import org.opensearch.common.time.DateFormatters;
import org.opensearch.sql.data.type.ExprCoreType;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.utils.DateTimeFormatters;

/** Expression Timestamp Value. */
@RequiredArgsConstructor
public class ExprTimestampValue extends AbstractExprValue {

private final Instant timestamp;

/** Formatted dateTime using user defined/default format */
private String formattedDateTime;

/** Constructor. */
public ExprTimestampValue(String timestamp) {
this(timestamp, List.of());
}

/**
* Constructor of ExprTimestampValue to support custom/OpenSearch dateTime formats in mappings.
*/
public ExprTimestampValue(String timestamp, List<DateFormatter> dateFormatters) {
try {
Instant localDateTime = null;
// check if dateFormatters are empty, then set default ones
if (dateFormatters == null || dateFormatters.isEmpty()) {
dateFormatters = DateTimeFormatters.initializeDateFormatters();
}
// parse using OpenSearch DateFormatters
for (DateFormatter formatter : dateFormatters) {
try {
TemporalAccessor accessor = formatter.parse(timestamp);
ZonedDateTime zonedDateTime = DateFormatters.from(accessor);
localDateTime = zonedDateTime.withZoneSameLocal(ZoneOffset.UTC).toInstant();
this.formattedDateTime = formatter.format(accessor);
break;
} catch (IllegalArgumentException ignored) {
// nothing to do, try another format
}
}
if (localDateTime == null) {
// Default constructor behavior, parse using java DateTimeFormatter
localDateTime =
LocalDateTime.parse(timestamp, DATE_TIME_FORMATTER_VARIABLE_NANOS)
.atZone(ZoneOffset.UTC)
.toInstant();
}
this.timestamp = localDateTime;
this.timestamp =
LocalDateTime.parse(timestamp, DATE_TIME_FORMATTER_VARIABLE_NANOS)
.atZone(ZoneOffset.UTC)
.toInstant();
} catch (DateTimeParseException e) {
throw new SemanticCheckException(
String.format(
Expand All @@ -81,23 +44,15 @@ public ExprTimestampValue(String timestamp, List<DateFormatter> dateFormatters)
/** localDateTime Constructor. */
public ExprTimestampValue(LocalDateTime localDateTime) {
this.timestamp = localDateTime.atZone(ZoneOffset.UTC).toInstant();
this.formattedDateTime = null;
}

public boolean hasNoFormatter() {
return this.formattedDateTime == null;
}

@Override
public String value() {
if (this.formattedDateTime == null) {
return timestamp.getNano() == 0
? DATE_TIME_FORMATTER_WITHOUT_NANO
.withZone(ZoneOffset.UTC)
.format(timestamp.truncatedTo(ChronoUnit.SECONDS))
: DATE_TIME_FORMATTER_VARIABLE_NANOS.withZone(ZoneOffset.UTC).format(timestamp);
}
return this.formattedDateTime;
return timestamp.getNano() == 0
? DATE_TIME_FORMATTER_WITHOUT_NANO
.withZone(ZoneOffset.UTC)
.format(timestamp.truncatedTo(ChronoUnit.SECONDS))
: DATE_TIME_FORMATTER_VARIABLE_NANOS.withZone(ZoneOffset.UTC).format(timestamp);
}

@Override
Expand Down
18 changes: 0 additions & 18 deletions core/src/main/java/org/opensearch/sql/utils/DateTimeFormatters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,8 @@
import java.time.format.ResolverStyle;
import java.time.format.SignStyle;
import java.time.temporal.ChronoField;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import lombok.experimental.UtilityClass;
import org.opensearch.common.time.DateFormatter;

/** DateTimeFormatter. Reference org.opensearch.common.time.DateFormatters. */
@UtilityClass
Expand All @@ -47,9 +43,6 @@ public class DateTimeFormatters {
private static final int MIN_FRACTION_SECONDS = 0;
private static final int MAX_FRACTION_SECONDS = 9;

public static final List<String> OPENSEARCH_DEFAULT_FORMATS =
Arrays.asList("strict_date_time_no_millis", "strict_date_optional_time", "epoch_millis");

public static final DateTimeFormatter TIME_ZONE_FORMATTER_NO_COLON =
new DateTimeFormatterBuilder()
.appendOffset("+HHmm", "Z")
Expand Down Expand Up @@ -116,9 +109,6 @@ public class DateTimeFormatters {
public static final DateTimeFormatter SQL_LITERAL_DATE_TIME_FORMAT =
DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");

public static final DateTimeFormatter SQL_LITERAL_DEFAULT_DATE_TIME_FORMAT =
DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ");

public static final DateTimeFormatter DATE_TIME_FORMATTER =
new DateTimeFormatterBuilder()
.appendOptional(SQL_LITERAL_DATE_TIME_FORMAT)
Expand Down Expand Up @@ -216,12 +206,4 @@ public class DateTimeFormatters {
.appendFraction(
ChronoField.NANO_OF_SECOND, MIN_FRACTION_SECONDS, MAX_FRACTION_SECONDS, true)
.toFormatter();

public static List<DateFormatter> initializeDateFormatters() {
List<DateFormatter> dateFormatters = new ArrayList<>();
for (String pattern : OPENSEARCH_DEFAULT_FORMATS) {
dateFormatters.add(DateFormatter.forPattern(pattern));
}
return dateFormatters;
}
}
Loading

0 comments on commit f725346

Please sign in to comment.