Skip to content

Commit ec7bf56

Browse files
committed
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>
1 parent 27ee33b commit ec7bf56

3 files changed

Lines changed: 2 additions & 15 deletions

File tree

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -279,20 +279,12 @@ 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-
*/
282+
/** DATE check for return-type inference; recognizes the analytics-route {@link DateOnlyType}. */
291283
public static boolean isDateExprType(RelDataType type) {
292284
return type instanceof DateOnlyType || convertRelDataTypeToExprType(type) == ExprCoreType.DATE;
293285
}
294286

295-
/** TIME counterpart of {@link #isDateExprType}recognizes the {@link TimeOnlyType} marker. */
287+
/** TIME counterpart of {@link #isDateExprType}; recognizes {@link TimeOnlyType}. */
296288
public static boolean isTimeExprType(RelDataType type) {
297289
return type instanceof TimeOnlyType || convertRelDataTypeToExprType(type) == ExprCoreType.TIME;
298290
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ private PPLReturnTypes() {}
3535
public static SqlReturnTypeInference TIME_APPLY_RETURN_TYPE =
3636
opBinding -> {
3737
RelDataType temporalType = opBinding.getOperandType(0);
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.
4038
if (OpenSearchTypeFactory.isTimeExprType(temporalType)) {
4139
return UserDefinedFunctionUtils.NULLABLE_TIME_UDT;
4240
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ public SqlReturnTypeInference getReturnTypeInference() {
6363
return opBinding -> {
6464
RelDataType temporalType = opBinding.getOperandType(0);
6565
RelDataType temporalDeltaType = opBinding.getOperandType(1);
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.
6966
if (OpenSearchTypeFactory.isDateExprType(temporalType)
7067
&& SqlTypeFamily.NUMERIC.contains(temporalDeltaType)) {
7168
return NULLABLE_DATE_UDT;

0 commit comments

Comments
 (0)