diff --git a/core/src/main/java/org/opensearch/sql/expression/function/PPLTypeChecker.java b/core/src/main/java/org/opensearch/sql/expression/function/PPLTypeChecker.java index c3de664443d..19875775c24 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/PPLTypeChecker.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/PPLTypeChecker.java @@ -552,24 +552,52 @@ public List> getParameterTypes() { }; } - /** - * Compares two RelDataTypes for signature matching. Two UDTs match if they share the same {@link - * ExprUDT} tag — comparing {@code getClass()} is unsafe because addCharsetAndCollation collapses - * ExprDateType/ExprTimeType/ExprTimeStampType/ExprBinaryType down to ExprSqlType, so different - * UDTs would appear equal. Plain types match by SqlTypeName. - */ + /** Compares SQL-private UDTs and analytics schema markers by their semantic PPL type. */ private static boolean typesMatch(RelDataType expected, RelDataType actual) { - if (expected instanceof AbstractExprRelDataType expUdt - && actual instanceof AbstractExprRelDataType actUdt) { - return expUdt.getUdt() == actUdt.getUdt(); - } - if (expected instanceof AbstractExprRelDataType - || actual instanceof AbstractExprRelDataType) { - return false; + ExprUDT expectedUdt = semanticUdt(expected); + ExprUDT actualUdt = semanticUdt(actual); + if (expectedUdt != null || actualUdt != null) { + return expectedUdt == actualUdt; } return expected.getSqlTypeName() == actual.getSqlTypeName(); } + private static ExprUDT semanticUdt(RelDataType type) { + if (type instanceof AbstractExprRelDataType udt) { + return udt.getUdt(); + } + + // Analytics marker types retain these semantic names while using TIMESTAMP or VARBINARY + // internally. Inspecting the Calcite digest avoids loading the optional analytics-api classes. + String digest = type.getFullTypeString(); + if (hasTypeName(digest, "DATE")) { + return ExprUDT.EXPR_DATE; + } + if (hasTypeName(digest, "TIME")) { + return ExprUDT.EXPR_TIME; + } + if (hasTypeName(digest, "IP")) { + return ExprUDT.EXPR_IP; + } + if (hasTypeName(digest, "BINARY")) { + return ExprUDT.EXPR_BINARY; + } + + return switch (type.getSqlTypeName()) { + case DATE -> ExprUDT.EXPR_DATE; + case TIME, TIME_WITH_LOCAL_TIME_ZONE -> ExprUDT.EXPR_TIME; + case TIMESTAMP, TIMESTAMP_WITH_LOCAL_TIME_ZONE -> ExprUDT.EXPR_TIMESTAMP; + case BINARY, VARBINARY -> ExprUDT.EXPR_BINARY; + default -> null; + }; + } + + private static boolean hasTypeName(String digest, String typeName) { + return digest.equals(typeName) + || digest.startsWith(typeName + "(") + || digest.startsWith(typeName + " "); + } + // Util Functions /** diff --git a/core/src/test/java/org/opensearch/sql/expression/function/PPLTypeCheckerTest.java b/core/src/test/java/org/opensearch/sql/expression/function/PPLTypeCheckerTest.java new file mode 100644 index 00000000000..5be90f16f61 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/function/PPLTypeCheckerTest.java @@ -0,0 +1,62 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.expression.function; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeSystem; +import org.apache.calcite.sql.type.SqlTypeName; +import org.junit.jupiter.api.Test; +import org.opensearch.analytics.schema.BinaryType; +import org.opensearch.analytics.schema.DateOnlyType; +import org.opensearch.analytics.schema.IpType; +import org.opensearch.analytics.schema.TimeOnlyType; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.ExprUDT; + +class PPLTypeCheckerTest { + + private static final OpenSearchTypeFactory TYPE_FACTORY = OpenSearchTypeFactory.TYPE_FACTORY; + + @Test + void matchesAnalyticsTypesToEquivalentPplUdts() { + assertMatches(ExprUDT.EXPR_TIMESTAMP, TYPE_FACTORY.createSqlType(SqlTypeName.TIMESTAMP)); + assertMatches(ExprUDT.EXPR_DATE, new DateOnlyType(RelDataTypeSystem.DEFAULT, true, 3)); + assertMatches(ExprUDT.EXPR_TIME, new TimeOnlyType(RelDataTypeSystem.DEFAULT, true, 3)); + assertMatches(ExprUDT.EXPR_IP, new IpType(true)); + assertMatches(ExprUDT.EXPR_BINARY, new BinaryType(true)); + } + + @Test + void doesNotConflateDifferentSemanticTypes() { + assertDoesNotMatch( + ExprUDT.EXPR_TIMESTAMP, new DateOnlyType(RelDataTypeSystem.DEFAULT, true, 3)); + assertDoesNotMatch(ExprUDT.EXPR_IP, new BinaryType(true)); + assertDoesNotMatch( + TYPE_FACTORY.createSqlType(SqlTypeName.TIMESTAMP), + new DateOnlyType(RelDataTypeSystem.DEFAULT, true, 3)); + assertDoesNotMatch(TYPE_FACTORY.createSqlType(SqlTypeName.VARBINARY), new IpType(true)); + } + + private static void assertMatches(ExprUDT expected, RelDataType actual) { + assertTrue(checker(TYPE_FACTORY.createUDT(expected)).checkOperandTypes(List.of(actual))); + } + + private static void assertDoesNotMatch(ExprUDT expected, RelDataType actual) { + assertDoesNotMatch(TYPE_FACTORY.createUDT(expected), actual); + } + + private static void assertDoesNotMatch(RelDataType expected, RelDataType actual) { + assertFalse(checker(expected).checkOperandTypes(List.of(actual))); + } + + private static PPLTypeChecker checker(RelDataType expected) { + return PPLTypeChecker.wrapUDT(List.of(List.of(expected))); + } +}