Skip to content

Commit f861d02

Browse files
vinaykpudclaude
andauthored
DATE/TIME label and rendering for date-only / time-only fields (#5521)
* fix(core): cluster D — TIME-typed list elements format as HH:mm:ss Companion to OpenSearch fix/ai/datetime-clusters @ 9009736c2dc. list(<TIME-typed field>) now returns "HH:mm:ss[.fraction]" without the 1970-01-01 epoch-date prefix. The analytics-engine path rewrites PPL list() to DataFusion's list_merge, so the legacy ListAggFunction never fires. Instead, AnalyticsExecutionEngine now post-processes List-typed cells in the result conversion: when an element string matches "1970-01-01[ T]HH:mm:ss[.fraction]", only the time portion is kept. Scalar cells are untouched, preserving the wider timestamp-stringification regression baseline. Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com> * fix(core): cluster A — span() output column type for date/time UDT Recognize the new sandbox DateOnlyType / TimeOnlyType UDT markers in: - OpenSearchTypeFactory.convertAnalyticsEngineRelDataTypeToExprType: DateOnlyType → ExprCoreType.DATE, TimeOnlyType → ExprCoreType.TIME so the user-visible response schema labels span() bucket columns as `date` / `time` instead of `timestamp`. - AnalyticsExecutionEngine.toExprValue: when the column carries a DateOnlyType marker, strip the trailing ` HH:MM:SS` from the Timestamp(ms)-formatted wire value so dates render as `YYYY-MM-DD`. Symmetric handling for TimeOnlyType strips the `1970-01-01 ` prefix. Pairs with the sandbox schema-builder change in opensearch-project/OpenSearch@b69c5ff8888. Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com> * Preserve DATE/TIME return type for ADDDATE/SUBDATE/ADDTIME/SUBTIME on date/time columns On the analytics-engine route a 'date'/'time' column is a TIMESTAMP-backed DateOnlyType/TimeOnlyType marker, so operand-conditional return-type inference mis-read it as TIMESTAMP — ADDDATE(date_col, N) reported (and rendered) TIMESTAMP instead of DATE. Add isDateExprType/isTimeExprType helpers recognizing those markers (off the general conversion path, no substrait round-trip risk) and use them in AddSubDateFunction and PPLReturnTypes.TIME_APPLY_RETURN_TYPE. Fixes the 6 *Null column-operand cases end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com> * Tighten ADDDATE/SUBDATE return-type helper comments Comment trims on top of d12407b (Marc Handalian's cherry-pick) — collapse the multi-line Javadoc on isDateExprType/isTimeExprType to one-line case names, and drop the inline narration at the two call sites (PPLReturnTypes.TIME_APPLY_RETURN_TYPE, AddSubDateFunction#getReturnTypeInference) that restated the helper contract. Behavior unchanged. Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com> * Fix tests Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com> --------- Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f12e4c3 commit f861d02

5 files changed

Lines changed: 127 additions & 19 deletions

File tree

core/src/main/java/org/opensearch/sql/calcite/utils/OpenSearchTypeFactory.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@
4747
import org.apache.calcite.sql.type.SqlTypeUtil;
4848
import org.checkerframework.checker.nullness.qual.Nullable;
4949
import org.opensearch.analytics.schema.BinaryType;
50+
import org.opensearch.analytics.schema.DateOnlyType;
5051
import org.opensearch.analytics.schema.IpType;
52+
import org.opensearch.analytics.schema.TimeOnlyType;
5153
import org.opensearch.sql.calcite.type.AbstractExprRelDataType;
5254
import org.opensearch.sql.calcite.type.ExprBinaryType;
5355
import org.opensearch.sql.calcite.type.ExprDateType;
@@ -277,6 +279,16 @@ public static ExprType convertRelDataTypeToExprType(RelDataType type) {
277279
return exprType;
278280
}
279281

282+
/** DATE check for return-type inference; recognizes the analytics-route {@link DateOnlyType}. */
283+
public static boolean isDateExprType(RelDataType type) {
284+
return type instanceof DateOnlyType || convertRelDataTypeToExprType(type) == ExprCoreType.DATE;
285+
}
286+
287+
/** TIME counterpart of {@link #isDateExprType}; recognizes {@link TimeOnlyType}. */
288+
public static boolean isTimeExprType(RelDataType type) {
289+
return type instanceof TimeOnlyType || convertRelDataTypeToExprType(type) == ExprCoreType.TIME;
290+
}
291+
280292
/**
281293
* Result-schema-only variant of {@link #convertRelDataTypeToExprType} that recognizes the
282294
* analytics-engine {@link IpType} / {@link BinaryType} markers as {@link ExprCoreType#IP} /
@@ -293,6 +305,13 @@ public static ExprType convertAnalyticsEngineRelDataTypeToExprType(RelDataType t
293305
if (type instanceof BinaryType) {
294306
return BINARY;
295307
}
308+
// span() over date / time UDT — schema label is DATE / TIME, not TIMESTAMP.
309+
if (type instanceof DateOnlyType) {
310+
return DATE;
311+
}
312+
if (type instanceof TimeOnlyType) {
313+
return TIME;
314+
}
296315
return convertRelDataTypeToExprType(type);
297316
}
298317

core/src/main/java/org/opensearch/sql/calcite/utils/PPLReturnTypes.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.apache.calcite.sql.type.SqlReturnTypeInference;
1313
import org.apache.calcite.sql.type.SqlTypeTransforms;
1414
import org.apache.calcite.sql.type.SqlTypeUtil;
15-
import org.opensearch.sql.data.type.ExprCoreType;
1615

1716
/**
1817
* Return types used in PPL. This class complements the {@link
@@ -36,8 +35,7 @@ private PPLReturnTypes() {}
3635
public static SqlReturnTypeInference TIME_APPLY_RETURN_TYPE =
3736
opBinding -> {
3837
RelDataType temporalType = opBinding.getOperandType(0);
39-
if (ExprCoreType.TIME.equals(
40-
OpenSearchTypeFactory.convertRelDataTypeToExprType(temporalType))) {
38+
if (OpenSearchTypeFactory.isTimeExprType(temporalType)) {
4139
return UserDefinedFunctionUtils.NULLABLE_TIME_UDT;
4240
}
4341
return UserDefinedFunctionUtils.NULLABLE_TIMESTAMP_UDT;

core/src/main/java/org/opensearch/sql/executor/analytics/AnalyticsExecutionEngine.java

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,17 @@
1212
import java.util.LinkedHashMap;
1313
import java.util.List;
1414
import java.util.Map;
15+
import java.util.regex.Pattern;
1516
import org.apache.calcite.plan.RelOptUtil;
1617
import org.apache.calcite.rel.RelNode;
1718
import org.apache.calcite.rel.type.RelDataType;
1819
import org.apache.calcite.rel.type.RelDataTypeField;
1920
import org.opensearch.analytics.exec.QueryPlanExecutor;
2021
import org.opensearch.analytics.exec.profile.ProfiledResult;
2122
import org.opensearch.analytics.schema.BinaryType;
23+
import org.opensearch.analytics.schema.DateOnlyType;
2224
import org.opensearch.analytics.schema.IpType;
25+
import org.opensearch.analytics.schema.TimeOnlyType;
2326
import org.opensearch.common.network.InetAddresses;
2427
import org.opensearch.core.action.ActionListener;
2528
import org.opensearch.sql.ast.statement.ExplainMode;
@@ -47,6 +50,15 @@
4750
*/
4851
public class AnalyticsExecutionEngine implements ExecutionEngine {
4952

53+
// TIME-typed list elements round-trip via Timestamp and bypass ArrowValues' scalar
54+
// post-processing (see DataFusion list_merge), arriving as "1970-01-01[ T]HH:mm:ss[.frac]".
55+
private static final Pattern EPOCH_DATE_TIME_PREFIX =
56+
Pattern.compile("^1970-01-01[ T](\\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?)$");
57+
58+
// DateOnlyType wire is Timestamp(ms) at midnight — keep the date, drop the time.
59+
private static final Pattern DATE_WITH_MIDNIGHT_TIME =
60+
Pattern.compile("^(\\d{4}-\\d{2}-\\d{2})[ T]\\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?$");
61+
5062
private final QueryPlanExecutor<RelNode, Iterable<Object[]>> planExecutor;
5163

5264
public AnalyticsExecutionEngine(QueryPlanExecutor<RelNode, Iterable<Object[]>> planExecutor) {
@@ -206,21 +218,7 @@ private List<ExprValue> convertRows(Iterable<Object[]> rows, List<RelDataTypeFie
206218
return results;
207219
}
208220

209-
/**
210-
* Converts a single result cell to an {@link ExprValue}, dispatching on the column's UDT when
211-
* present so {@code byte[]} payloads are rendered correctly:
212-
*
213-
* <ul>
214-
* <li>{@link IpType} + {@code byte[]} &rarr; canonical address string (matches {@code
215-
* IpFieldMapper}'s {@code valueFetcher} output).
216-
* <li>{@link BinaryType} + {@code byte[]} &rarr; base64-encoded string (matches the OpenSearch
217-
* {@code binary} field wire format).
218-
* <li>Anything else &rarr; existing {@link ExprValueUtils#fromObjectValue} path.
219-
* </ul>
220-
*
221-
* <p>Without this dispatch, {@code fromObjectValue} throws {@code unsupported object class [B} on
222-
* byte[] cells, and IP buffers leak through as raw 16-byte ipv4-mapped-ipv6 garbage.
223-
*/
221+
/** Renders UDT cells (IP/binary byte[]; date / time string) and strips TIME prefixes in lists. */
224222
private static ExprValue toExprValue(Object value, RelDataType type) {
225223
if (value instanceof byte[] bytes) {
226224
if (type instanceof IpType) {
@@ -234,9 +232,39 @@ private static ExprValue toExprValue(Object value, RelDataType type) {
234232
return ExprValueUtils.stringValue(Base64.getEncoder().encodeToString(bytes));
235233
}
236234
}
235+
// DateOnlyType scalar — strip midnight time suffix off the Timestamp(ms) wire.
236+
if (type instanceof DateOnlyType && value instanceof String s) {
237+
var m = DATE_WITH_MIDNIGHT_TIME.matcher(s);
238+
if (m.matches()) {
239+
return ExprValueUtils.stringValue(m.group(1));
240+
}
241+
}
242+
// TimeOnlyType scalar — strip 1970-01-01 prefix off the Timestamp(ms) wire.
243+
if (type instanceof TimeOnlyType && value instanceof String s) {
244+
var m = EPOCH_DATE_TIME_PREFIX.matcher(s);
245+
if (m.matches()) {
246+
return ExprValueUtils.stringValue(m.group(1));
247+
}
248+
}
249+
if (value instanceof List<?> list) {
250+
return ExprValueUtils.collectionValue(stripEpochDatePrefixInList(list));
251+
}
237252
return ExprValueUtils.fromObjectValue(value);
238253
}
239254

255+
private static List<Object> stripEpochDatePrefixInList(List<?> list) {
256+
List<Object> out = new ArrayList<>(list.size());
257+
for (Object element : list) {
258+
if (element instanceof String s) {
259+
var m = EPOCH_DATE_TIME_PREFIX.matcher(s);
260+
out.add(m.matches() ? m.group(1) : s);
261+
} else {
262+
out.add(element);
263+
}
264+
}
265+
return out;
266+
}
267+
240268
private Schema buildSchema(List<RelDataTypeField> fields) {
241269
List<Schema.Column> columns = new ArrayList<>();
242270
for (RelDataTypeField field : fields) {

core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/AddSubDateFunction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public SqlReturnTypeInference getReturnTypeInference() {
6363
return opBinding -> {
6464
RelDataType temporalType = opBinding.getOperandType(0);
6565
RelDataType temporalDeltaType = opBinding.getOperandType(1);
66-
if (OpenSearchTypeFactory.convertRelDataTypeToExprType(temporalType) == ExprCoreType.DATE
66+
if (OpenSearchTypeFactory.isDateExprType(temporalType)
6767
&& SqlTypeFamily.NUMERIC.contains(temporalDeltaType)) {
6868
return NULLABLE_DATE_UDT;
6969
} else {

core/src/test/java/org/opensearch/sql/executor/analytics/AnalyticsExecutionEngineTest.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
import org.junit.jupiter.api.Test;
3131
import org.opensearch.analytics.exec.QueryPlanExecutor;
3232
import org.opensearch.analytics.schema.BinaryType;
33+
import org.opensearch.analytics.schema.DateOnlyType;
3334
import org.opensearch.analytics.schema.IpType;
35+
import org.opensearch.analytics.schema.TimeOnlyType;
3436
import org.opensearch.core.action.ActionListener;
3537
import org.opensearch.sql.calcite.CalcitePlanContext;
3638
import org.opensearch.sql.calcite.SysLimit;
@@ -237,6 +239,67 @@ void executeRelNode_binaryColumnRendersAsBase64() {
237239
"byte[] should base64-encode to match OpenSearch binary wire format. " + dump);
238240
}
239241

242+
/** DateOnlyType — schema reports DATE, value strips midnight suffix. */
243+
@Test
244+
void executeRelNode_dateOnlyTypeStripsTimeSuffix() {
245+
RelNode relNode =
246+
mockRelNodeWithType("d", new DateOnlyType(RelDataTypeSystem.DEFAULT, true, 3));
247+
Iterable<Object[]> rows = Collections.singletonList(new Object[] {"1984-04-12 00:00:00"});
248+
stubExecutorWith(relNode, rows);
249+
250+
QueryResponse response = executeAndCapture(relNode);
251+
String dump = dumpResponse(response);
252+
253+
assertEquals(ExprCoreType.DATE, response.getSchema().getColumns().get(0).getExprType(), dump);
254+
assertEquals(
255+
"1984-04-12", response.getResults().get(0).tupleValue().get("d").stringValue(), dump);
256+
}
257+
258+
/** TimeOnlyType — schema reports TIME, value strips 1970-01-01 prefix. */
259+
@Test
260+
void executeRelNode_timeOnlyTypeStripsEpochDatePrefix() {
261+
RelNode relNode =
262+
mockRelNodeWithType("t", new TimeOnlyType(RelDataTypeSystem.DEFAULT, true, 3));
263+
Iterable<Object[]> rows = Collections.singletonList(new Object[] {"1970-01-01 09:00:00"});
264+
stubExecutorWith(relNode, rows);
265+
266+
QueryResponse response = executeAndCapture(relNode);
267+
String dump = dumpResponse(response);
268+
269+
assertEquals(ExprCoreType.TIME, response.getSchema().getColumns().get(0).getExprType(), dump);
270+
assertEquals(
271+
"09:00:00", response.getResults().get(0).tupleValue().get("t").stringValue(), dump);
272+
}
273+
274+
/** TIME-typed list elements arrive as "1970-01-01[ T]HH:mm:ss[.frac]" — strip the prefix. */
275+
@Test
276+
void executeRelNode_listOfStringStripsEpochDatePrefix() {
277+
SqlTypeFactoryImpl typeFactory = new SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
278+
RelDataType arrayOfVarchar =
279+
typeFactory.createArrayType(typeFactory.createSqlType(SqlTypeName.VARCHAR), -1);
280+
RelNode relNode = mockRelNodeWithType("time_list", arrayOfVarchar);
281+
java.util.List<String> input =
282+
Arrays.asList(
283+
"1970-01-01 19:36:22",
284+
"1970-01-01T02:05:25",
285+
"1970-01-01 12:34:56.123456789",
286+
"2020-10-13 13:00:00",
287+
"hello");
288+
stubExecutorWith(relNode, Collections.singletonList(new Object[] {input}));
289+
290+
QueryResponse response = executeAndCapture(relNode);
291+
String dump = dumpResponse(response);
292+
293+
java.util.List<String> result =
294+
response.getResults().get(0).tupleValue().get("time_list").collectionValue().stream()
295+
.map(org.opensearch.sql.data.model.ExprValue::stringValue)
296+
.toList();
297+
assertEquals(
298+
Arrays.asList("19:36:22", "02:05:25", "12:34:56.123456789", "2020-10-13 13:00:00", "hello"),
299+
result,
300+
dump);
301+
}
302+
240303
@Test
241304
void executeRelNode_emptyResults() {
242305
RelNode relNode = mockRelNode("name", SqlTypeName.VARCHAR);

0 commit comments

Comments
 (0)