Skip to content

Commit

Permalink
More fixes for god of fixes.
Browse files Browse the repository at this point in the history 
Signed-off-by: Yury-Fridlyand <[email protected]>
  • Loading branch information
@Yury-Fridlyand
Yury-Fridlyand committed Jul 7, 2023
1 parent 6d214a5 commit c086dad
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 36 deletions.
53 changes: 24 additions & 29 deletions opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDateType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ public class OpenSearchDateType extends OpenSearchDataType {
private static final String CUSTOM_FORMAT_DATE_SYMBOLS = "FecEWwYqQgdMLDyuG";

@EqualsAndHashCode.Exclude
String formatString;
private final List<String> formats;

private OpenSearchDateType() {
super(MappingType.Date);
this.formatString = "";
this.formats = List.of();
}

private OpenSearchDateType(ExprCoreType exprCoreType) {
Expand All @@ -156,16 +156,20 @@ private OpenSearchDateType(ExprType exprType) {

private OpenSearchDateType(String format) {
super(MappingType.Date);
this.formatString = format;
this.formats = getFormatList(format);
this.exprCoreType = getExprTypeFromFormatString(format);
}

public boolean hasFormats() {
return !formats.isEmpty();
}

/**
* Retrieves and splits a user defined format string from the mapping into a list of formats.
* @return A list of format names and user defined formats.
*/
private List<String> getFormatList() {
String format = strip8Prefix(formatString);
private List<String> getFormatList(String format) {
format = strip8Prefix(format);
return splitCombinedPatterns(format).stream().map(String::trim).collect(Collectors.toList());
}

Expand All @@ -174,7 +178,7 @@ private List<String> getFormatList() {
* @return a list of DateFormatters that can be used to parse a Date/Time/Timestamp.
*/
public List<DateFormatter> getAllNamedFormatters() {
return getFormatList().stream()
return formats.stream()
.filter(formatString -> FormatNames.forName(formatString) != null)
.map(DateFormatter::forPattern).collect(Collectors.toList());
}
Expand All @@ -184,7 +188,7 @@ public List<DateFormatter> getAllNamedFormatters() {
* @return a list of DateFormatters that can be used to parse a Date.
*/
public List<DateFormatter> getNumericNamedFormatters() {
return getFormatList().stream()
return formats.stream()
.filter(formatString -> {
FormatNames namedFormat = FormatNames.forName(formatString);
return namedFormat != null && SUPPORTED_NAMED_NUMERIC_FORMATS.contains(namedFormat);
Expand All @@ -197,7 +201,7 @@ public List<DateFormatter> getNumericNamedFormatters() {
* @return a list of formats as strings that can be used to parse a Date/Time/Timestamp.
*/
public List<String> getAllCustomFormats() {
return getFormatList().stream()
return formats.stream()
.filter(format -> FormatNames.forName(format) == null)
.map(format -> {
try {
Expand All @@ -217,17 +221,8 @@ public List<String> getAllCustomFormats() {
* @return a list of DateFormatters that can be used to parse a Date/Time/Timestamp.
*/
public List<DateFormatter> getAllCustomFormatters() {
return getFormatList().stream()
.filter(format -> FormatNames.forName(format) == null)
.map(format -> {
try {
return DateFormatter.forPattern(format);
} catch (Exception ignored) {
// parsing failed
return null;
}
})
.filter(Objects::nonNull)
return getAllCustomFormats().stream()
.map(DateFormatter::forPattern)
.collect(Collectors.toList());
}

Expand All @@ -236,7 +231,7 @@ public List<DateFormatter> getAllCustomFormatters() {
* @return a list of DateFormatters that can be used to parse a Date.
*/
public List<DateFormatter> getDateNamedFormatters() {
return getFormatList().stream()
return formats.stream()
.filter(formatString -> {
FormatNames namedFormat = FormatNames.forName(formatString);
return namedFormat != null && SUPPORTED_NAMED_DATE_FORMATS.contains(namedFormat);
Expand All @@ -249,7 +244,7 @@ public List<DateFormatter> getDateNamedFormatters() {
* @return a list of DateFormatters that can be used to parse a Time.
*/
public List<DateFormatter> getTimeNamedFormatters() {
return getFormatList().stream()
return formats.stream()
.filter(formatString -> {
FormatNames namedFormat = FormatNames.forName(formatString);
return namedFormat != null && SUPPORTED_NAMED_TIME_FORMATS.contains(namedFormat);
Expand All @@ -262,7 +257,7 @@ public List<DateFormatter> getTimeNamedFormatters() {
* @return a list of DateFormatters that can be used to parse a DateTime.
*/
public List<DateFormatter> getDateTimeNamedFormatters() {
return getFormatList().stream()
return formats.stream()
.filter(formatString -> {
FormatNames namedFormat = FormatNames.forName(formatString);
return namedFormat != null && SUPPORTED_NAMED_DATETIME_FORMATS.contains(namedFormat);
Expand All @@ -274,17 +269,17 @@ private ExprCoreType getExprTypeFromCustomFormats(List<String> formats) {
boolean isDate = false;
boolean isTime = false;

for (var format : formats) {
for (String format : formats) {
if (!isTime) {
for (var symbol : CUSTOM_FORMAT_TIME_SYMBOLS.toCharArray()) {
for (char symbol : CUSTOM_FORMAT_TIME_SYMBOLS.toCharArray()) {
if (format.contains(String.valueOf(symbol))) {
isTime = true;
break;
}
}
}
if (!isDate) {
for (var symbol : CUSTOM_FORMAT_DATE_SYMBOLS.toCharArray()) {
for (char symbol : CUSTOM_FORMAT_DATE_SYMBOLS.toCharArray()) {
if (format.contains(String.valueOf(symbol))) {
isDate = true;
break;
Expand Down Expand Up @@ -393,7 +388,7 @@ public static OpenSearchDateType of() {

@Override
public List<ExprType> getParent() {
return List.of(this.exprCoreType);
return List.of(exprCoreType);
}

@Override
Expand All @@ -403,9 +398,9 @@ public boolean shouldCast(ExprType other) {

@Override
protected OpenSearchDataType cloneEmpty() {
if (this.formatString.isEmpty()) {
return OpenSearchDateType.of(this.exprCoreType);
if (formats.isEmpty()) {
return OpenSearchDateType.of(exprCoreType);
}
return OpenSearchDateType.of(this.formatString);
return OpenSearchDateType.of(String.join(" || ", formats));
}
}
20 changes: 14 additions & 6 deletions ...ch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ private ExprValue parseTimestampString(String value, OpenSearchDateType dateType
parsed = DateFormatters.from(DATE_TIME_FORMATTER.parse(value)).toInstant();
return new ExprTimestampValue(parsed);
} catch (DateTimeParseException ignored) {
// ignored
}

// otherwise, throw an error that no formatters worked
Expand Down Expand Up @@ -279,6 +280,7 @@ private ExprValue parseTimeString(String value, OpenSearchDateType dateType) {
return new ExprTimeValue(
DateFormatters.from(STRICT_HOUR_MINUTE_SECOND_FORMATTER.parse(value)).toLocalTime());
} catch (DateTimeParseException ignored) {
// ignored
}

throw new IllegalArgumentException("Construct ExprTimeValue from \"" + value
Expand Down Expand Up @@ -313,6 +315,7 @@ private ExprValue parseDateString(String value, OpenSearchDateType dateType) {
return new ExprDateValue(
DateFormatters.from(STRICT_YEAR_MONTH_DAY_FORMATTER.parse(value)).toLocalDate());
} catch (DateTimeParseException ignored) {
// ignored
}

throw new IllegalArgumentException("Construct ExprDateValue from \"" + value
Expand All @@ -325,19 +328,24 @@ private ExprValue createOpenSearchDateType(Content value, ExprType type) {

if (value.isNumber()) { // isNumber
var numFormatters = dt.getNumericNamedFormatters();
if (numFormatters.size() > 0) {
if (numFormatters.size() > 0 || !dt.hasFormats()) {
long epochMillis = 0;
if (numFormatters.contains(DateFormatter.forPattern(
FormatNames.EPOCH_MILLIS.getSnakeCaseName()))) {
FormatNames.EPOCH_SECOND.getSnakeCaseName()))) {
// no CamelCase for `EPOCH_*` formats
epochMillis = value.longValue();
} else /* EPOCH_SECOND */ {
epochMillis = value.longValue() * 1000;
} else /* EPOCH_MILLIS */ {
epochMillis = value.longValue();
}
Instant instant = Instant.ofEpochMilli(epochMillis);
switch ((ExprCoreType) returnFormat) {
case TIME: return new ExprTimeValue(LocalTime.from(instant.atZone(UTC_ZONE_ID)));
case DATE: return new ExprDateValue(LocalDate.ofInstant(instant, UTC_ZONE_ID));
default: return new ExprTimestampValue(instant);
}
return new ExprTimestampValue(Instant.ofEpochMilli(epochMillis));
} else {
// custom format
switch ((ExprCoreType) dt.getExprType()) {
switch ((ExprCoreType) returnFormat) {
case TIME: return parseTimeString(value.stringValue(), dt);
case DATE: return parseDateString(value.stringValue(), dt);
default: return parseTimestampString(value.stringValue(), dt);
Expand Down
15 changes: 14 additions & 1 deletion ...rc/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@
import static org.opensearch.sql.data.type.ExprCoreType.STRUCT;
import static org.opensearch.sql.data.type.ExprCoreType.TIME;
import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP;
import static org.opensearch.sql.utils.DateTimeUtils.UTC_ZONE_ID;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableMap;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalTime;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -212,7 +215,8 @@ public void constructDouble() {
@Test
public void constructString() {
assertAll(
() -> assertEquals(stringValue("text"), tupleValue("{\"stringV\":\"text\"}").get("stringV")),
() -> assertEquals(stringValue("text"),
tupleValue("{\"stringV\":\"text\"}").get("stringV")),
() -> assertEquals(stringValue("text"), constructFromObject("stringV", "text"))
);
}
Expand Down Expand Up @@ -248,6 +252,9 @@ public void constructDates() {
ExprValue dateStringV = constructFromObject("dateStringV", "1984-04-12");
assertAll(
() -> assertEquals(new ExprDateValue("1984-04-12"), dateStringV),
() -> assertEquals(new ExprDateValue(
LocalDate.ofInstant(Instant.ofEpochMilli(450576000000L), UTC_ZONE_ID)),
constructFromObject("dateV", 450576000000L)),
() -> assertEquals(new ExprDateValue("1984-04-12"),
constructFromObject("dateOrOrdinalDateV", "1984-103")),
() -> assertEquals(new ExprDateValue("2015-01-01"),
Expand All @@ -262,6 +269,9 @@ public void constructTimes() {
() -> assertTrue(timeStringV.isDateTime()),
() -> assertTrue(timeStringV instanceof ExprTimeValue),
() -> assertEquals(new ExprTimeValue("12:10:30"), timeStringV),
() -> assertEquals(new ExprTimeValue(LocalTime.from(
Instant.ofEpochMilli(1420070400001L).atZone(UTC_ZONE_ID))),
constructFromObject("timeV", 1420070400001L)),
() -> assertEquals(new ExprTimeValue("09:07:42.000"),
constructFromObject("timeNoMillisOrTimeV", "09:07:42.000Z")),
() -> assertEquals(new ExprTimeValue("09:07:42"),
Expand All @@ -284,6 +294,9 @@ public void constructDatetime() {
() -> assertEquals(
new ExprTimestampValue("2015-01-01 12:10:30"),
tupleValue("{\"timestampV\":\"2015-01-01 12:10:30\"}").get("timestampV")),
() -> assertEquals(
new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)),
constructFromObject("timestampV", 1420070400001L)),
() -> assertEquals(
new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)),
constructFromObject("timestampV", Instant.ofEpochMilli(1420070400001L))),
Expand Down

0 comments on commit c086dad

Please sign in to comment.