Skip to content

Commit bfc54ca

Browse files
committed
Remove DatetimeOutputCastRule on the analytics-engine route (#5420)
## Problem On the analytics-engine route, every datetime root column is wrapped in `CAST(<DATE/TIME/TIMESTAMP> AS VARCHAR)` by `DatetimeOutputCastRule`. A matching `DatetimeOutputCastRewriter` on the engine side then translates that cast into DataFusion's `to_char` extension. Whenever the rewriter's format string and the PPL formatter disagree (e.g. trailing `Z`, `T` separator), users see wire-format divergence — issue #5420. ## Solution Drop the cast rule and let the engine return real datetime cells. The PPL response pipeline already handles datetime → string conversion natively at the formatter layer: - `AnalyticsExecutionEngine.convertRows` feeds engine cells (`LocalDateTime` / `LocalDate` / `LocalTime`) through `ExprValueUtils.fromObjectValue`, which produces `ExprTimestampValue` / `ExprDateValue` / `ExprTimeValue`. - Their `value()` methods render the documented PPL space-separated format (`yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]` etc.). The companion change in `opensearch-project/OpenSearch` removes the matching `DatetimeOutputCastRewriter` and its callsites. ## Changes - Delete `DatetimeOutputCastRule`. - Drop the unused `UdtMapping.isDatetimeType` helper. - Rewrite the four cast-shape assertions in `DatetimeExtensionTest` to assert the post-removal RelNode (no `CAST(... VARCHAR)` wrapper, schema reports `DATE` / `TIME` / `TIMESTAMP`). Signed-off-by: Eric Wei <ericwei@amazon.com>
1 parent 817f7f3 commit bfc54ca

3 files changed

Lines changed: 14 additions & 87 deletions

File tree

api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeExtension.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class DatetimeExtension implements LanguageExtension {
2222

2323
@Override
2424
public List<RelShuttle> postAnalysisRules() {
25-
return List.of(DatetimeUdtNormalizeRule.INSTANCE, DatetimeOutputCastRule.INSTANCE);
25+
return List.of(DatetimeUdtNormalizeRule.INSTANCE);
2626
}
2727

2828
/** Maps datetime UDT types to their standard Calcite equivalents. */
@@ -44,10 +44,5 @@ static Optional<UdtMapping> fromUdtType(RelDataType type) {
4444
ExprUDT udt = e.getUdt();
4545
return Arrays.stream(values()).filter(u -> u.udtType == udt).findFirst();
4646
}
47-
48-
/** Returns true if the given SqlTypeName is a standard datetime type. */
49-
static boolean isDatetimeType(SqlTypeName typeName) {
50-
return Arrays.stream(values()).anyMatch(u -> u.stdType == typeName);
51-
}
5247
}
5348
}

api/src/main/java/org/opensearch/sql/api/spec/datetime/DatetimeOutputCastRule.java

Lines changed: 0 additions & 62 deletions
This file was deleted.

api/src/test/java/org/opensearch/sql/api/spec/datetime/DatetimeExtensionTest.java

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ private Table createEventsTable() {
6363
}
6464

6565
@Test
66-
public void testUdfResultNormalizedAndCastToVarchar() {
66+
public void testUdfResultNormalized() {
6767
givenQuery(
6868
"""
6969
source = catalog.events \
@@ -72,9 +72,8 @@ public void testUdfResultNormalizedAndCastToVarchar() {
7272
""")
7373
.assertPlan(
7474
"""
75-
LogicalProject(d=[CAST($0):VARCHAR], t=[CAST($1):VARCHAR], ts=[CAST($2):VARCHAR])
76-
LogicalProject(d=[DATE($1)], t=[TIME($1)], ts=[TIMESTAMP($1)])
77-
LogicalTableScan(table=[[catalog, events]])
75+
LogicalProject(d=[DATE($1)], t=[TIME($1)], ts=[TIMESTAMP($1)])
76+
LogicalTableScan(table=[[catalog, events]])
7877
""")
7978
.assertReturnType("DATE", DATE)
8079
.assertReturnType("TIME", TIME, 9)
@@ -94,13 +93,12 @@ public void testNestedUdfCallsNormalized() {
9493
}
9594

9695
@Test
97-
public void testDateLiteralCastToVarchar() {
96+
public void testDateLiteralNormalized() {
9897
givenQuery("source = catalog.events | eval d = DATE('2024-01-01') | fields d")
9998
.assertPlan(
10099
"""
101-
LogicalProject(d=[CAST($0):VARCHAR])
102-
LogicalProject(d=[DATE('2024-01-01':VARCHAR)])
103-
LogicalTableScan(table=[[catalog, events]])
100+
LogicalProject(d=[DATE('2024-01-01':VARCHAR)])
101+
LogicalTableScan(table=[[catalog, events]])
104102
""")
105103
.assertReturnType("DATE", DATE);
106104
}
@@ -134,13 +132,12 @@ public void testComparisonWithDatetimeUdf() {
134132
}
135133

136134
@Test
137-
public void testAllStandardDatetimeTypesCastToVarchar() {
135+
public void testStandardDatetimeFieldsNotWrapped() {
138136
givenQuery("source = catalog.events | fields hire_date, start_time, created_at")
139137
.assertPlan(
140138
"""
141-
LogicalProject(hire_date=[CAST($0):VARCHAR NOT NULL], start_time=[CAST($1):VARCHAR NOT NULL], created_at=[CAST($2):VARCHAR NOT NULL])
142-
LogicalProject(hire_date=[$2], start_time=[$3], created_at=[$4])
143-
LogicalTableScan(table=[[catalog, events]])
139+
LogicalProject(hire_date=[$2], start_time=[$3], created_at=[$4])
140+
LogicalTableScan(table=[[catalog, events]])
144141
""");
145142
}
146143

@@ -155,19 +152,16 @@ public void testNonDatetimeFieldsNotWrapped() {
155152
}
156153

157154
@Test
158-
public void testOutputCastCanCompileAndExecute() throws Exception {
155+
public void testDatetimeFieldsPreserveStandardTypes() throws Exception {
159156
RelNode plan =
160157
planner.plan("source = catalog.events | fields hire_date, start_time, created_at");
161158
try (PreparedStatement statement = compiler.compile(plan)) {
162159
ResultSet resultSet = statement.executeQuery();
163160
verify(resultSet)
164161
.expectSchema(
165-
col("hire_date", java.sql.Types.VARCHAR),
166-
col("start_time", java.sql.Types.VARCHAR),
167-
col("created_at", java.sql.Types.VARCHAR))
168-
.expectData(
169-
row("2024-01-16", "12:00:00", "2024-01-15 08:00:00"),
170-
row("2024-06-20", "14:00:00", "2024-06-20 00:00:00"));
162+
col("hire_date", java.sql.Types.DATE),
163+
col("start_time", java.sql.Types.TIME),
164+
col("created_at", java.sql.Types.TIMESTAMP));
171165
}
172166
}
173167
}

0 commit comments

Comments
 (0)