Skip to content

Commit

Permalink
Fix to support different date formats in the sql query without date c…
Browse files Browse the repository at this point in the history
…asting

Github Issue - #2700

Signed-off-by: Manasvini B S <manasvis@amazon.com>
  • Loading branch information
manasvinibs committed Jul 8, 2024
1 parent 4326396 commit a2c9f68
Show file tree
Hide file tree
Showing 20 changed files with 495 additions and 79 deletions.
1 change: 1 addition & 0 deletions core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,22 @@
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.util.ArrayList;
import java.util.List;
import lombok.RequiredArgsConstructor;
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.common.time.DateFormatter;

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

private final LocalDate date;
private String dateFormat;

/** Constructor of ExprDateValue. */
public ExprDateValue(String date) {
Expand All @@ -36,9 +42,44 @@ public ExprDateValue(String date) {
}
}

/** Constructor of ExprDateValue to support custom/OpenSearch date formats in mappings. */
public ExprDateValue(String date, List<DateFormatter> dateFormatters) {
LocalDate localDate = null;
String dateFormat = "";
// check if dateFormatters are empty, then set default ones
if (dateFormatters == null || dateFormatters.isEmpty()) {
String defaultPatterns = "strict_date_time_no_millis||strict_date_optional_time||epoch_millis";
String[] patterns = defaultPatterns.split("\\|\\|");
dateFormatters = new ArrayList<>();
for (String pattern : patterns) {
dateFormatters.add(DateFormatter.forPattern(pattern));
}
}
for (DateFormatter formatter : dateFormatters) {
try {
TemporalAccessor accessor = formatter.parse(date);
ZonedDateTime zonedDateTime = DateFormatters.from(accessor);
localDate = zonedDateTime.withZoneSameLocal(ZoneOffset.UTC).toLocalDate();

dateFormat = formatter.format(accessor);
break;
} catch (IllegalArgumentException ignored) {
// nothing to do, try another format
}
}
if (localDate == null) {
localDate = new ExprDateValue(date).dateValue();
}
this.date = localDate;
this.dateFormat = dateFormat;
}

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

@Override
Expand Down Expand Up @@ -68,7 +109,7 @@ public boolean isDateTime() {

@Override
public String toString() {
return String.format("DATE '%s'", value());
return String.format("DATE '%s'", DateTimeFormatter.ISO_LOCAL_DATE.format(date));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
import static java.time.format.DateTimeFormatter.ISO_LOCAL_TIME;
import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL;

import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.*;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.util.ArrayList;
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;
Expand All @@ -26,6 +27,7 @@
public class ExprTimeValue extends AbstractExprValue {

private final LocalTime time;
private String timeFormat;

/** Constructor of ExprTimeValue. */
public ExprTimeValue(String time) {
Expand All @@ -37,9 +39,43 @@ public ExprTimeValue(String time) {
}
}

/** Constructor of ExprTimeValue to support custom/OpenSearch date formats in mappings. */
public ExprTimeValue(String time, List<DateFormatter> dateFormatters) {
LocalTime localTime = null;
String timeFormat = "";
// check if dateFormatters are empty, then set default ones
if (dateFormatters == null || dateFormatters.isEmpty()) {
String defaultPatterns = "strict_date_time_no_millis||strict_date_optional_time||epoch_millis";
String[] patterns = defaultPatterns.split("\\|\\|");
dateFormatters = new ArrayList<>();
for (String pattern : patterns) {
dateFormatters.add(DateFormatter.forPattern(pattern));
}
}
for (DateFormatter formatter : dateFormatters) {
try {
TemporalAccessor accessor = formatter.parse(time);
ZonedDateTime zonedDateTime = DateFormatters.from(accessor);
localTime = zonedDateTime.withZoneSameLocal(ZoneOffset.UTC).toLocalTime();
timeFormat = formatter.format(accessor);
break;
} catch (IllegalArgumentException ignored) {
// nothing to do, try another format
}
}
if (localTime == null) {
localTime = new ExprTimeValue(time).timeValue();
}
this.time = localTime;
this.timeFormat = timeFormat;
}

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

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@

package org.opensearch.sql.data.model;

import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_VARIABLE_NANOS;
import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_WITHOUT_NANO;

import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZoneOffset;
import static org.opensearch.sql.utils.DateTimeFormatters.*;

import java.time.*;
import java.time.format.DateTimeParseException;
import java.time.temporal.ChronoUnit;
import java.time.temporal.TemporalAccessor;
import java.util.ArrayList;
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;
Expand All @@ -26,6 +26,7 @@
public class ExprTimestampValue extends AbstractExprValue {

private final Instant timestamp;
private String dateTimeFormat;

/** Constructor. */
public ExprTimestampValue(String timestamp) {
Expand All @@ -42,18 +43,52 @@ public ExprTimestampValue(String timestamp) {
}
}

/** Constructor of ExprTimestampValue to support custom/OpenSearch dateTime formats in mappings. */
public ExprTimestampValue(String timestamp, List<DateFormatter> dateFormatters) {
Instant localDateTime = null;
String dateTimeFormat = "";
// check if dateFormatters are empty, then set default ones
if (dateFormatters == null || dateFormatters.isEmpty()) {
String defaultPatterns = "strict_date_time_no_millis||strict_date_optional_time||epoch_millis";
String[] patterns = defaultPatterns.split("\\|\\|");
dateFormatters = new ArrayList<>();
for (String pattern : patterns) {
dateFormatters.add(DateFormatter.forPattern(pattern));
}
}
for (DateFormatter formatter : dateFormatters) {
try {
TemporalAccessor accessor = formatter.parse(timestamp);
ZonedDateTime zonedDateTime = DateFormatters.from(accessor);
localDateTime = zonedDateTime.withZoneSameLocal(ZoneOffset.UTC).toInstant();
dateTimeFormat = formatter.format(accessor);
break;
} catch (IllegalArgumentException ignored) {
// nothing to do, try another format
}
}
if (localDateTime == null) {
localDateTime = new ExprTimestampValue(timestamp).timestampValue();
}
this.timestamp = localDateTime;
this.dateTimeFormat = dateTimeFormat;
}

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

@Override
public String value() {
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);
if (this.dateTimeFormat == null || this.dateTimeFormat.isEmpty()) {
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.dateTimeFormat;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ public class DateTimeFormatters {

public static final DateTimeFormatter DATE_TIME_FORMATTER_VARIABLE_NANOS_OPTIONAL =
new DateTimeFormatterBuilder()
.appendPattern("[uuuu-MM-dd HH:mm:ss][uuuu-MM-dd HH:mm][HH:mm:ss][HH:mm][uuuu-MM-dd]")
.appendPattern(
"[uuuu-MM-dd HH:mm:ss][uuuu-MM-dd HH:mm][HH:mm:ss][HH:mm][uuuu-MM-dd][dd-MMM-uu]")
.appendFraction(
ChronoField.NANO_OF_SECOND, MIN_FRACTION_SECONDS, MAX_FRACTION_SECONDS, true)
.toFormatter(Locale.ROOT)
Expand Down Expand Up @@ -206,4 +207,5 @@ public class DateTimeFormatters {
.appendFraction(
ChronoField.NANO_OF_SECOND, MIN_FRACTION_SECONDS, MAX_FRACTION_SECONDS, true)
.toFormatter();

}
Loading

0 comments on commit a2c9f68

Please sign in to comment.