Skip to content

Commit 27ee33b

Browse files
mch2claude
authored andcommitted
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>
1 parent f89313e commit 27ee33b

3 files changed

Lines changed: 25 additions & 4 deletions

File tree

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,24 @@ public static ExprType convertRelDataTypeToExprType(RelDataType type) {
279279
return exprType;
280280
}
281281

282+
/**
283+
* Whether {@code type} represents a DATE for the purpose of operand-conditional return-type
284+
* inference (e.g. {@code ADDDATE(date, n)} returns DATE only when the base is a date). Recognizes
285+
* both the SQL-plugin DATE UDT (via {@link #convertRelDataTypeToExprType}) and the
286+
* analytics-route {@link DateOnlyType} column marker, which is {@code TIMESTAMP}-backed and would
287+
* otherwise be misread as TIMESTAMP. Kept separate from {@link #convertRelDataTypeToExprType} so
288+
* it isn't on the general planner path that round-trips through {@link
289+
* #convertExprTypeToRelDataType}.
290+
*/
291+
public static boolean isDateExprType(RelDataType type) {
292+
return type instanceof DateOnlyType || convertRelDataTypeToExprType(type) == ExprCoreType.DATE;
293+
}
294+
295+
/** TIME counterpart of {@link #isDateExprType} — recognizes the {@link TimeOnlyType} marker. */
296+
public static boolean isTimeExprType(RelDataType type) {
297+
return type instanceof TimeOnlyType || convertRelDataTypeToExprType(type) == ExprCoreType.TIME;
298+
}
299+
282300
/**
283301
* Result-schema-only variant of {@link #convertRelDataTypeToExprType} that recognizes the
284302
* analytics-engine {@link IpType} / {@link BinaryType} markers as {@link ExprCoreType#IP} /

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

Lines changed: 3 additions & 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,9 @@ 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+
// isTimeExprType so a TimeOnlyType column (analytics-route TIMESTAMP-backed time marker) is
39+
// recognized as TIME; otherwise ADDTIME/SUBTIME on a time column lose the TIME return type.
40+
if (OpenSearchTypeFactory.isTimeExprType(temporalType)) {
4141
return UserDefinedFunctionUtils.NULLABLE_TIME_UDT;
4242
}
4343
return UserDefinedFunctionUtils.NULLABLE_TIMESTAMP_UDT;

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ 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+
// isDateExprType (not convertRelDataTypeToExprType == DATE) so a DateOnlyType column — the
67+
// analytics-route TIMESTAMP-backed date marker — is recognized as DATE; otherwise ADDDATE/
68+
// SUBDATE on a date column would mis-declare TIMESTAMP and lose the DATE return type.
69+
if (OpenSearchTypeFactory.isDateExprType(temporalType)
6770
&& SqlTypeFamily.NUMERIC.contains(temporalDeltaType)) {
6871
return NULLABLE_DATE_UDT;
6972
} else {

0 commit comments

Comments
 (0)