From eb88e3cbd5c5902b3c48f3151c62d0c9b3ff0076 Mon Sep 17 00:00:00 2001 From: Andy Kwok Date: Mon, 2 Dec 2024 09:22:51 -0800 Subject: [PATCH] Fix: Date field format parsing for legacy query engine (#3160) * Test cases Signed-off-by: Andy Kwok * Minimise code changes Signed-off-by: Andy Kwok * Format Signed-off-by: Andy Kwok * Update integration test Signed-off-by: Andy Kwok * Update unit test Signed-off-by: Andy Kwok --------- Signed-off-by: Andy Kwok --- .../org/opensearch/sql/legacy/CursorIT.java | 4 ++- .../executor/format/DateFieldFormatter.java | 32 +++++++++++++------ .../format/DateFieldFormatterTest.java | 20 +++++++++++- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java index e4ba593844..565c40b121 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/CursorIT.java @@ -280,7 +280,9 @@ public void testRegressionOnDateFormatChange() throws IOException { Arrays.asList( "2015-01-01 00:00:00.000", "2015-01-01 12:10:30.000", - "1585882955", // by existing design, this is not formatted in MySQL standard format + // Conversion will be applied when dateTime is stored on unix timestamp, + // https://github.com/opensearch-project/sql/pull/3160 + "2020-04-03 03:02:35.000", "2020-04-08 06:10:30.000"); assertThat(actualDateList, equalTo(expectedDateList)); diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatter.java b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatter.java index dc239abd84..feba13d139 100644 --- a/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatter.java +++ b/legacy/src/main/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatter.java @@ -21,6 +21,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.sql.legacy.esdomain.LocalClusterState; import org.opensearch.sql.legacy.esdomain.mapping.FieldMappings; +import org.opensearch.sql.legacy.utils.StringUtils; /** Formatter to transform date fields into a consistent format for consumption by clients. */ public class DateFieldFormatter { @@ -83,7 +84,6 @@ public void applyJDBCDateFormat(Map rowSource) { Date date = parseDateString(formats, columnOriginalDate.toString()); if (date != null) { rowSource.put(columnName, DateFormat.getFormattedDate(date, FORMAT_JDBC)); - break; } else { LOG.warn("Could not parse date value; returning original value"); } @@ -152,15 +152,27 @@ private Date parseDateString(List formats, String columnOriginalDate) { switch (columnFormat) { case "date_optional_time": case "strict_date_optional_time": - parsedDate = - DateUtils.parseDate( - columnOriginalDate, - FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_LOGS_EXCEPTION, - FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_FLIGHTS_EXCEPTION, - FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_FLIGHTS_EXCEPTION_NO_TIME, - FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_ECOMMERCE_EXCEPTION, - FORMAT_DOT_DATE_AND_TIME, - FORMAT_DOT_DATE); + // It's possible to have date stored in second / millisecond form without explicit + // format hint. + // Parse it on a best-effort basis. + if (StringUtils.isNumeric(columnOriginalDate)) { + long timestamp = Long.parseLong(columnOriginalDate); + if (timestamp > Integer.MAX_VALUE) { + parsedDate = new Date(timestamp); + } else { + parsedDate = new Date(timestamp * 1000); + } + } else { + parsedDate = + DateUtils.parseDate( + columnOriginalDate, + FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_LOGS_EXCEPTION, + FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_FLIGHTS_EXCEPTION, + FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_FLIGHTS_EXCEPTION_NO_TIME, + FORMAT_DOT_OPENSEARCH_DASHBOARDS_SAMPLE_DATA_ECOMMERCE_EXCEPTION, + FORMAT_DOT_DATE_AND_TIME, + FORMAT_DOT_DATE); + } break; case "epoch_millis": parsedDate = new Date(Long.parseLong(columnOriginalDate)); diff --git a/legacy/src/test/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatterTest.java b/legacy/src/test/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatterTest.java index 1c2d1bae62..7d43ea0383 100644 --- a/legacy/src/test/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatterTest.java +++ b/legacy/src/test/java/org/opensearch/sql/legacy/executor/format/DateFieldFormatterTest.java @@ -575,7 +575,7 @@ public void testIncorrectFormat() { String dateFormat = "date_optional_time"; String originalDateValue = "1581724085"; // Invalid format for date value; should return original value - String expectedDateValue = "1581724085"; + String expectedDateValue = "2020-02-14 23:48:05.000"; verifyFormatting(columnName, dateFormat, originalDateValue, expectedDateValue); } @@ -609,6 +609,24 @@ public void testStrictDateOptionalTimeOrEpochMillsShouldPass() { verifyFormatting(columnName, dateFormat, originalDateValue, expectedDateValue); } + @Test + public void testDateInTimestampFormInSecondWithoutHint() { + String columnName = "date_field"; + String dateFormat = "date_optional_time"; + String originalDateValue = "1732057981"; + String expectedDateValue = "2024-11-19 23:13:01.000"; + verifyFormatting(columnName, dateFormat, originalDateValue, expectedDateValue); + } + + @Test + public void testDateInTimestampFormInMilliSecondWithoutHint() { + String columnName = "date_field"; + String dateFormat = "date_optional_time"; + String originalDateValue = "1732057981000"; + String expectedDateValue = "2024-11-19 23:13:01.000"; + verifyFormatting(columnName, dateFormat, originalDateValue, expectedDateValue); + } + private void verifyFormatting( String columnName, String dateFormatProperty,