diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/PPLOperandTypes.java b/core/src/main/java/org/opensearch/sql/calcite/utils/PPLOperandTypes.java index fcd361ba229..fd233c83dd5 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/PPLOperandTypes.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/PPLOperandTypes.java @@ -5,10 +5,17 @@ package org.opensearch.sql.calcite.utils; +import static org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.TYPE_FACTORY; + +import java.util.ArrayList; +import java.util.List; +import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.sql.type.CompositeOperandTypeChecker; import org.apache.calcite.sql.type.FamilyOperandTypeChecker; import org.apache.calcite.sql.type.OperandTypes; import org.apache.calcite.sql.type.SqlTypeFamily; +import org.apache.calcite.sql.type.SqlTypeName; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.ExprUDT; import org.opensearch.sql.expression.function.UDFOperandMetadata; /** @@ -20,41 +27,51 @@ public class PPLOperandTypes { // This class is not meant to be instantiated. private PPLOperandTypes() {} + // Convenience RelDataType constants used to express UDF signatures via wrapUDT(...). + // UDT-backed scalar types: + public static final RelDataType DATE_UDT = TYPE_FACTORY.createUDT(ExprUDT.EXPR_DATE); + public static final RelDataType TIME_UDT = TYPE_FACTORY.createUDT(ExprUDT.EXPR_TIME); + public static final RelDataType TIMESTAMP_UDT = TYPE_FACTORY.createUDT(ExprUDT.EXPR_TIMESTAMP); + public static final RelDataType IP_UDT = TYPE_FACTORY.createUDT(ExprUDT.EXPR_IP); + public static final RelDataType BINARY_UDT = TYPE_FACTORY.createUDT(ExprUDT.EXPR_BINARY); + // Plain SQL scalar types: + public static final RelDataType BYTE_T = TYPE_FACTORY.createSqlType(SqlTypeName.TINYINT); + public static final RelDataType SHORT_T = TYPE_FACTORY.createSqlType(SqlTypeName.SMALLINT); + public static final RelDataType INTEGER_T = TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER); + public static final RelDataType LONG_T = TYPE_FACTORY.createSqlType(SqlTypeName.BIGINT); + public static final RelDataType FLOAT_T = TYPE_FACTORY.createSqlType(SqlTypeName.REAL); + public static final RelDataType DOUBLE_T = TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE); + public static final RelDataType STRING_T = TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR); + public static final RelDataType BOOLEAN_T = TYPE_FACTORY.createSqlType(SqlTypeName.BOOLEAN); + /** List of all scalar type signatures (single parameter each) */ - private static final java.util.List> - SCALAR_TYPES = - java.util.List.of( - // Numeric types - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.BYTE), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.SHORT), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.INTEGER), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.LONG), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.FLOAT), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.DOUBLE), - // String type - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.STRING), - // Boolean type - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.BOOLEAN), - // Temporal types - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.DATE), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.TIME), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP), - // Special scalar types - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.IP), - java.util.List.of(org.opensearch.sql.data.type.ExprCoreType.BINARY)); + private static final List> SCALAR_TYPES = + List.of( + // Numeric types + List.of(BYTE_T), + List.of(SHORT_T), + List.of(INTEGER_T), + List.of(LONG_T), + List.of(FLOAT_T), + List.of(DOUBLE_T), + // String type + List.of(STRING_T), + // Boolean type + List.of(BOOLEAN_T), + // Temporal types + List.of(DATE_UDT), + List.of(TIME_UDT), + List.of(TIMESTAMP_UDT), + // Special scalar types + List.of(IP_UDT), + List.of(BINARY_UDT)); /** Helper method to create scalar types with optional integer parameter */ - private static java.util.List> - createScalarWithOptionalInteger() { - java.util.List> result = - new java.util.ArrayList<>(SCALAR_TYPES); + private static List> createScalarWithOptionalInteger() { + List> result = new ArrayList<>(SCALAR_TYPES); // Add scalar + integer combinations - SCALAR_TYPES.forEach( - scalarType -> - result.add( - java.util.List.of( - scalarType.get(0), org.opensearch.sql.data.type.ExprCoreType.INTEGER))); + SCALAR_TYPES.forEach(scalarType -> result.add(List.of(scalarType.get(0), INTEGER_T))); return result; } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/CoercionUtils.java b/core/src/main/java/org/opensearch/sql/expression/function/CoercionUtils.java index 7562dac74d8..204abc67b7b 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/CoercionUtils.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/CoercionUtils.java @@ -5,49 +5,61 @@ package org.opensearch.sql.expression.function; -import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; - import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.PriorityQueue; import java.util.Set; import java.util.function.BiPredicate; import java.util.function.BinaryOperator; -import java.util.stream.Collectors; import javax.annotation.Nullable; +import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexNode; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.commons.lang3.tuple.Pair; +import org.opensearch.sql.calcite.type.AbstractExprRelDataType; import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; -import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.ExprUDT; import org.opensearch.sql.exception.ExpressionEvaluationException; +/** + * RelDataType-native widening, casting, and common-type resolution for PPL function dispatch. + * + *

This module models a small widening DAG over a tag space derived from {@link RelDataType}: + * + *

    + *
  • numeric SQL types (BYTE → SHORT → INTEGER → LONG → FLOAT → DOUBLE) + *
  • STRING (VARCHAR/CHAR) + *
  • BOOLEAN + *
  • OpenSearch UDTs (DATE, TIME, TIMESTAMP, IP, BINARY) detected via {@code instanceof} + *
+ * + *

The widening rules mirror prior behavior: {@code STRING + NUMERIC → DOUBLE}, {@code DATE + + * TIME → TIMESTAMP}, {@code STRING + BINARY → BINARY}; STRING widens to BOOLEAN/DATE/TIME/ + * TIMESTAMP/IP; numerics widen along the integer/float chain; etc. + */ public final class CoercionUtils { + private CoercionUtils() {} + /** - * Casts the arguments to the types specified in the typeChecker. Returns null if no combination - * of parameter types matches the arguments or if casting fails. - * - * @param builder RexBuilder to create casts - * @param typeChecker PPLTypeChecker that provides the parameter types - * @param arguments List of RexNode arguments to be cast - * @return List of cast RexNode arguments or null if casting fails + * Casts the arguments to one of the typeChecker's allowed signatures. Returns null if no + * combination of parameter types matches the arguments or if casting fails. */ public static @Nullable List castArguments( RexBuilder builder, PPLTypeChecker typeChecker, List arguments) { - List> paramTypeCombinations = typeChecker.getParameterTypes(); + List> paramTypeCombinations = typeChecker.getParameterTypes(); - List sourceTypes = - arguments.stream() - .map(node -> OpenSearchTypeFactory.convertRelDataTypeToExprType(node.getType())) - .collect(Collectors.toList()); + List sourceTypes = arguments.stream().map(RexNode::getType).toList(); // Candidate parameter signatures ordered by decreasing widening distance - PriorityQueue, Integer>> rankedSignatures = + PriorityQueue, Integer>> rankedSignatures = new PriorityQueue<>((left, right) -> Integer.compare(right.getValue(), left.getValue())); - for (List paramTypes : paramTypeCombinations) { - int distance = distance(sourceTypes, paramTypes); + for (List paramTypes : paramTypeCombinations) { + int distance = signatureDistance(sourceTypes, paramTypes); if (distance == TYPE_EQUAL) { return castArguments(builder, paramTypes, arguments); } @@ -64,38 +76,43 @@ public final class CoercionUtils { /** * Widen the arguments to the widest type found among them. If no widest type can be determined, * returns null. - * - * @param builder RexBuilder to create casts - * @param arguments List of RexNode arguments to be widened - * @return List of widened RexNode arguments or null if no widest type can be determined */ public static @Nullable List widenArguments( RexBuilder builder, List arguments) { - // TODO: Add test on e.g. IP - ExprType widestType = findWidestType(arguments); + RelDataType widestType = findWidestType(arguments); if (widestType == null) { - return null; // No widest type found, return null + return null; } return arguments.stream().map(arg -> cast(builder, widestType, arg)).toList(); } + /** Returns true if any of the operands is a string-like (VARCHAR/CHAR) RexNode. */ + public static boolean hasString(List rexNodeList) { + return rexNodeList.stream().map(RexNode::getType).anyMatch(SqlTypeUtil::isCharacter); + } + /** - * Casts the arguments to the types specified in paramTypes. Returns null if the number of - * parameters does not match or if casting fails. + * Resolves the widening common type for two {@link RelDataType}s using PPL coercion rules. Used + * primarily by tests to verify the rule set. */ + @VisibleForTesting + public static Optional resolveCommonType(RelDataType left, RelDataType right) { + return COMMON_COERCION_RULES.stream() + .map(rule -> rule.apply(left, right)) + .flatMap(Optional::stream) + .findFirst(); + } + + // -------- Internals -------- + private static @Nullable List castArguments( - RexBuilder builder, List paramTypes, List arguments) { + RexBuilder builder, List paramTypes, List arguments) { if (paramTypes.size() != arguments.size()) { - return null; // Skip if the number of parameters does not match + return null; } - List castedArguments = new ArrayList<>(); for (int i = 0; i < paramTypes.size(); i++) { - ExprType toType = paramTypes.get(i); - RexNode arg = arguments.get(i); - - RexNode castedArg = cast(builder, toType, arg); - + RexNode castedArg = cast(builder, paramTypes.get(i), arguments.get(i)); if (castedArg == null) { return null; } @@ -104,180 +121,289 @@ public final class CoercionUtils { return castedArguments; } - private static @Nullable RexNode cast(RexBuilder builder, ExprType targetType, RexNode arg) { - ExprType argType = OpenSearchTypeFactory.convertRelDataTypeToExprType(arg.getType()); - if (!argType.shouldCast(targetType)) { + private static @Nullable RexNode cast(RexBuilder builder, RelDataType targetType, RexNode arg) { + RelDataType argType = arg.getType(); + if (!shouldCast(argType, targetType)) { return arg; } + // Always make the cast target nullable. Safe cast returns null on parse failure or on a null + // source, so a non-nullable declared target defeats null-propagation and leads to primitive + // unboxing NPEs in downstream aggregations. if (distance(argType, targetType) != IMPOSSIBLE_WIDENING) { return builder.makeCast( - OpenSearchTypeFactory.convertExprTypeToRelDataType(targetType), arg, true, true); + builder.getTypeFactory().createTypeWithNullability(targetType, true), arg, true, true); } return resolveCommonType(argType, targetType) .map( - exprType -> + common -> builder.makeCast( - OpenSearchTypeFactory.convertExprTypeToRelDataType(exprType), arg, true, true)) + builder.getTypeFactory().createTypeWithNullability(common, true), + arg, + true, + true)) .orElse(null); } - /** - * Finds the widest type among the given arguments. The widest type is determined by applying the - * widening type rule to each pair of types in the arguments. - * - * @param arguments List of RexNode arguments to find the widest type from - * @return the widest ExprType if found, otherwise null - */ - private static @Nullable ExprType findWidestType(List arguments) { + private static @Nullable RelDataType findWidestType(List arguments) { if (arguments.isEmpty()) { - return null; // No arguments to process + return null; } - ExprType widestType = - OpenSearchTypeFactory.convertRelDataTypeToExprType(arguments.getFirst().getType()); + RelDataType widest = arguments.getFirst().getType(); if (arguments.size() == 1) { - return widestType; + return widest; } - - // Iterate pairwise through the arguments and find the widest type for (int i = 1; i < arguments.size(); i++) { - var type = OpenSearchTypeFactory.convertRelDataTypeToExprType(arguments.get(i).getType()); + RelDataType type = arguments.get(i).getType(); try { - final ExprType tempType = widestType; - widestType = resolveCommonType(widestType, type).orElseGet(() -> max(tempType, type)); + final RelDataType current = widest; + widest = resolveCommonType(widest, type).orElseGet(() -> max(current, type)); } catch (ExpressionEvaluationException e) { - // the two types are not compatible, return null return null; } } - return widestType; + // Normalize standard temporal types to their UDT equivalents so downstream casts route through + // the ExtendedRexBuilder UDT path (producing TIMESTAMP/DATE/TIME function calls at runtime). + return normalizeTemporalToUdt(widest); } - private static boolean areDateAndTime(ExprType type1, ExprType type2) { - return (type1 == ExprCoreType.DATE && type2 == ExprCoreType.TIME) - || (type1 == ExprCoreType.TIME && type2 == ExprCoreType.DATE); + private static RelDataType normalizeTemporalToUdt(RelDataType type) { + if (type instanceof AbstractExprRelDataType) { + return type; + } + SqlTypeName name = type.getSqlTypeName(); + ExprUDT udt = + switch (name) { + case TIMESTAMP -> ExprUDT.EXPR_TIMESTAMP; + case DATE -> ExprUDT.EXPR_DATE; + case TIME -> ExprUDT.EXPR_TIME; + default -> null; + }; + return udt == null + ? type + : OpenSearchTypeFactory.TYPE_FACTORY.createUDT(udt, type.isNullable()); } - @VisibleForTesting - public static Optional resolveCommonType(ExprType left, ExprType right) { - return COMMON_COERCION_RULES.stream() - .map(rule -> rule.apply(left, right)) - .flatMap(Optional::stream) - .findFirst(); - } + // -------- Tag space + widening DAG -------- - public static boolean hasString(List rexNodeList) { - return rexNodeList.stream() - .map(RexNode::getType) - .map(OpenSearchTypeFactory::convertRelDataTypeToExprType) - .anyMatch(t -> t == ExprCoreType.STRING); + /** + * A small enum that tags every {@link RelDataType} the coercion rules need to reason about. Using + * an enum means the widening DAG is closed and self-contained. + */ + private enum CoercionTag { + UNDEFINED, // Calcite NULL — compatible with anything + UNKNOWN, // Anything we can't classify + BYTE, + SHORT, + INTEGER, + LONG, + FLOAT, + DOUBLE, + STRING, + BOOLEAN, + DATE, + TIME, + TIMESTAMP, + IP, + BINARY, + INTERVAL, + ARRAY, + STRUCT, + GEOMETRY; } - private static final Set NUMBER_TYPES = ExprCoreType.numberTypes(); - - private static final List COMMON_COERCION_RULES = - List.of( - CoercionRule.of( - (left, right) -> areDateAndTime(left, right), - (left, right) -> ExprCoreType.TIMESTAMP), - CoercionRule.of( - (left, right) -> hasString(left, right) && hasNumber(left, right), - (left, right) -> ExprCoreType.DOUBLE), - // (BINARY, STRING) → BINARY: ip/binary columns compared with string literals. - // Triggers ExtendedRexBuilder.makeCast which wraps the literal with BINARY. - CoercionRule.of( - (left, right) -> hasString(left, right) && hasBinary(left, right), - (left, right) -> ExprCoreType.BINARY)); - - private static boolean hasString(ExprType left, ExprType right) { - return left == ExprCoreType.STRING || right == ExprCoreType.STRING; + private static CoercionTag tagOf(RelDataType type) { + if (type instanceof AbstractExprRelDataType udt) { + return switch (udt.getUdt()) { + case EXPR_DATE -> CoercionTag.DATE; + case EXPR_TIME -> CoercionTag.TIME; + case EXPR_TIMESTAMP -> CoercionTag.TIMESTAMP; + case EXPR_IP -> CoercionTag.IP; + case EXPR_BINARY -> CoercionTag.BINARY; + }; + } + SqlTypeName name = type.getSqlTypeName(); + if (name == null) return CoercionTag.UNKNOWN; + return switch (name) { + case TINYINT -> CoercionTag.BYTE; + case SMALLINT -> CoercionTag.SHORT; + case INTEGER -> CoercionTag.INTEGER; + case BIGINT -> CoercionTag.LONG; + case REAL, FLOAT -> CoercionTag.FLOAT; + case DOUBLE, DECIMAL -> CoercionTag.DOUBLE; + case CHAR, VARCHAR -> CoercionTag.STRING; + case BOOLEAN -> CoercionTag.BOOLEAN; + case BINARY, VARBINARY -> CoercionTag.BINARY; + case DATE -> CoercionTag.DATE; + case TIME, TIME_WITH_LOCAL_TIME_ZONE, TIME_TZ -> CoercionTag.TIME; + case TIMESTAMP, TIMESTAMP_WITH_LOCAL_TIME_ZONE, TIMESTAMP_TZ -> CoercionTag.TIMESTAMP; + case NULL -> CoercionTag.UNDEFINED; + case ARRAY, MULTISET -> CoercionTag.ARRAY; + case MAP, ROW, STRUCTURED -> CoercionTag.STRUCT; + case GEOMETRY -> CoercionTag.GEOMETRY; + default -> { + if (SqlTypeName.INTERVAL_TYPES.contains(name)) { + yield CoercionTag.INTERVAL; + } + yield CoercionTag.UNKNOWN; + } + }; } - private static boolean hasNumber(ExprType left, ExprType right) { - return NUMBER_TYPES.contains(left) || NUMBER_TYPES.contains(right); + /** + * Parents in the widening DAG. Mirrors {@code ExprCoreType} parent links. A type widens to its + * parent and (transitively) to grandparents. + */ + private static final Map> PARENTS = buildParents(); + + private static Map> buildParents() { + Map> p = new HashMap<>(); + p.put(CoercionTag.BYTE, List.of(CoercionTag.SHORT)); + p.put(CoercionTag.SHORT, List.of(CoercionTag.INTEGER)); + p.put(CoercionTag.INTEGER, List.of(CoercionTag.LONG)); + p.put(CoercionTag.LONG, List.of(CoercionTag.FLOAT)); + p.put(CoercionTag.FLOAT, List.of(CoercionTag.DOUBLE)); + // STRING has TIMESTAMP as a direct parent (in addition to DATE/TIME/BOOLEAN/IP) so the + // STRING→TIMESTAMP widening cost matches STRING→DOUBLE (1). This preserves baseline ranking + // when both NUMERIC and TIMESTAMP signatures are candidates for a STRING input. + p.put( + CoercionTag.STRING, + List.of( + CoercionTag.BOOLEAN, + CoercionTag.DATE, + CoercionTag.TIME, + CoercionTag.TIMESTAMP, + CoercionTag.IP)); + p.put(CoercionTag.DATE, List.of(CoercionTag.TIMESTAMP)); + p.put(CoercionTag.TIME, List.of(CoercionTag.TIMESTAMP)); + return p; } - private static boolean hasBoolean(ExprType left, ExprType right) { - return left == ExprCoreType.BOOLEAN || right == ExprCoreType.BOOLEAN; + private static List parentsOf(CoercionTag tag) { + return PARENTS.getOrDefault(tag, List.of()); } - private static boolean hasBinary(ExprType left, ExprType right) { - return left == ExprCoreType.BINARY || right == ExprCoreType.BINARY; - } + private static final Set NUMBER_TAGS = + Set.of( + CoercionTag.BYTE, + CoercionTag.SHORT, + CoercionTag.INTEGER, + CoercionTag.LONG, + CoercionTag.FLOAT, + CoercionTag.DOUBLE); - private record CoercionRule( - BiPredicate predicate, BinaryOperator resolver) { - - Optional apply(ExprType left, ExprType right) { - return predicate.test(left, right) - ? Optional.of(resolver.apply(left, right)) - : Optional.empty(); - } - - static CoercionRule of( - BiPredicate predicate, BinaryOperator resolver) { - return new CoercionRule(predicate, resolver); - } - } + // -------- Distance and "shouldCast" -------- private static final int IMPOSSIBLE_WIDENING = Integer.MAX_VALUE; private static final int TYPE_EQUAL = 0; - private static int distance(ExprType type1, ExprType type2) { - return distance(type1, type2, TYPE_EQUAL); + private static int distance(RelDataType from, RelDataType to) { + return distance(tagOf(from), tagOf(to), TYPE_EQUAL); } - private static int distance(ExprType type1, ExprType type2, int distance) { - if (type1 == type2) { - return distance; - } else if (type1 == UNKNOWN) { - return IMPOSSIBLE_WIDENING; - } else if (type1 == ExprCoreType.STRING && type2 == ExprCoreType.DOUBLE) { - return 1; - } else { - return type1.getParent().stream() - .map(parentOfType1 -> distance(parentOfType1, type2, distance + 1)) - .reduce(Math::min) - .get(); + private static int distance(CoercionTag from, CoercionTag to) { + return distance(from, to, TYPE_EQUAL); + } + + private static int distance(CoercionTag from, CoercionTag to, int acc) { + if (from == to) return acc; + if (from == CoercionTag.UNKNOWN) return IMPOSSIBLE_WIDENING; + // UNDEFINED (NULL literal) widens to any concrete type at unit cost — mirrors v2 where every + // concrete ExprCoreType has UNDEFINED as a parent. + if (from == CoercionTag.UNDEFINED && to != CoercionTag.UNKNOWN) return acc + 1; + // STRING widens directly to DOUBLE (custom rule mirroring v2 behavior). + if (from == CoercionTag.STRING && to == CoercionTag.DOUBLE) return acc + 1; + List parents = parentsOf(from); + if (parents.isEmpty()) return IMPOSSIBLE_WIDENING; + int best = IMPOSSIBLE_WIDENING; + for (CoercionTag parent : parents) { + int d = distance(parent, to, acc + 1); + if (d < best) best = d; } + return best; + } + + /** The argument should be cast to the target type when their CoercionTags differ. */ + private static boolean shouldCast(RelDataType source, RelDataType target) { + return tagOf(source) != tagOf(target); } /** - * The max type among two types. The max is defined as follow if type1 could widen to type2, then - * max is type2, vice versa if type1 couldn't widen to type2 and type2 could't widen to type1, - * then throw {@link ExpressionEvaluationException}. - * - * @param type1 type1 - * @param type2 type2 - * @return the max type among two types. + * The widest of two types: the one that the other can widen to. Throws if neither widens to the + * other. */ - public static ExprType max(ExprType type1, ExprType type2) { - int type1To2 = distance(type1, type2); - int type2To1 = distance(type2, type1); - - if (type1To2 == Integer.MAX_VALUE && type2To1 == Integer.MAX_VALUE) { + static RelDataType max(RelDataType type1, RelDataType type2) { + int t1To2 = distance(type1, type2); + int t2To1 = distance(type2, type1); + if (t1To2 == IMPOSSIBLE_WIDENING && t2To1 == IMPOSSIBLE_WIDENING) { throw new ExpressionEvaluationException( String.format("no max type of %s and %s ", type1, type2)); - } else { - return type1To2 == Integer.MAX_VALUE ? type1 : type2; } + return t1To2 == IMPOSSIBLE_WIDENING ? type1 : type2; } - public static int distance(List sourceTypes, List targetTypes) { + private static int signatureDistance( + List sourceTypes, List targetTypes) { if (sourceTypes.size() != targetTypes.size()) { return IMPOSSIBLE_WIDENING; } - - int totalDistance = 0; + int total = 0; for (int i = 0; i < sourceTypes.size(); i++) { - ExprType source = sourceTypes.get(i); - ExprType target = targetTypes.get(i); - int distance = distance(source, target); - if (distance == IMPOSSIBLE_WIDENING) { + int d = distance(sourceTypes.get(i), targetTypes.get(i)); + if (d == IMPOSSIBLE_WIDENING) { return IMPOSSIBLE_WIDENING; - } else { - totalDistance += distance; } + total += d; + } + return total; + } + + // -------- Common-type rules -------- + + private static boolean hasString(RelDataType l, RelDataType r) { + return tagOf(l) == CoercionTag.STRING || tagOf(r) == CoercionTag.STRING; + } + + private static boolean hasNumber(RelDataType l, RelDataType r) { + return NUMBER_TAGS.contains(tagOf(l)) || NUMBER_TAGS.contains(tagOf(r)); + } + + private static boolean hasBinary(RelDataType l, RelDataType r) { + return tagOf(l) == CoercionTag.BINARY || tagOf(r) == CoercionTag.BINARY; + } + + private static boolean areDateAndTime(RelDataType l, RelDataType r) { + CoercionTag lt = tagOf(l); + CoercionTag rt = tagOf(r); + return (lt == CoercionTag.DATE && rt == CoercionTag.TIME) + || (lt == CoercionTag.TIME && rt == CoercionTag.DATE); + } + + private static final List COMMON_COERCION_RULES = + List.of( + CoercionRule.of( + CoercionUtils::areDateAndTime, + (l, r) -> OpenSearchTypeFactory.TYPE_FACTORY.createUDT(ExprUDT.EXPR_TIMESTAMP)), + CoercionRule.of( + (l, r) -> hasString(l, r) && hasNumber(l, r), + (l, r) -> OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE)), + // (BINARY, STRING) → BINARY: ip/binary columns compared with string literals. + // ExtendedRexBuilder.makeCast wraps the literal into VARBINARY when it sees this. + CoercionRule.of( + (l, r) -> hasString(l, r) && hasBinary(l, r), + (l, r) -> OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.VARBINARY))); + + private record CoercionRule( + BiPredicate predicate, BinaryOperator resolver) { + + Optional apply(RelDataType left, RelDataType right) { + return predicate.test(left, right) + ? Optional.of(resolver.apply(left, right)) + : Optional.empty(); + } + + static CoercionRule of( + BiPredicate predicate, BinaryOperator resolver) { + return new CoercionRule(predicate, resolver); } - return totalDistance; } } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/PPLBuiltinOperators.java b/core/src/main/java/org/opensearch/sql/expression/function/PPLBuiltinOperators.java index d64f04bb9ad..2a670af3fee 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/PPLBuiltinOperators.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/PPLBuiltinOperators.java @@ -246,11 +246,12 @@ public class PPLBuiltinOperators extends ReflectiveSqlOperatorTable { public static final SqlOperator SECOND = new DatePartFunction(TimeUnit.SECOND).toUDF("SECOND"); public static final SqlOperator MICROSECOND = new DatePartFunction(TimeUnit.MICROSECOND).toUDF("MICROSECOND"); - public static final SqlOperator NOW = new CurrentFunction(ExprCoreType.TIMESTAMP).toUDF("NOW"); + public static final SqlOperator NOW = + new CurrentFunction(CurrentFunction.Kind.TIMESTAMP).toUDF("NOW"); public static final SqlOperator CURRENT_TIME = - new CurrentFunction(ExprCoreType.TIME).toUDF("CURRENT_TIME"); + new CurrentFunction(CurrentFunction.Kind.TIME).toUDF("CURRENT_TIME"); public static final SqlOperator CURRENT_DATE = - new CurrentFunction(ExprCoreType.DATE).toUDF("CURRENT_DATE"); + new CurrentFunction(CurrentFunction.Kind.DATE).toUDF("CURRENT_DATE"); public static final SqlOperator DATE_FORMAT = new FormatFunction(ExprCoreType.DATE).toUDF("DATE_FORMAT"); public static final SqlOperator TIME_FORMAT = diff --git a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java index 151c4a96655..64f29906829 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java @@ -309,6 +309,7 @@ import org.apache.calcite.sql.type.SqlOperandTypeChecker; import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.calcite.sql.validate.SqlUserDefinedAggFunction; import org.apache.calcite.sql.validate.SqlUserDefinedFunction; import org.apache.calcite.tools.RelBuilder; @@ -319,8 +320,6 @@ import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.calcite.utils.PlanUtils; import org.opensearch.sql.calcite.utils.UserDefinedFunctionUtils; -import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.executor.QueryType; import org.opensearch.sql.expression.function.CollectionUDF.MVIndexFunctionImp; @@ -450,7 +449,7 @@ public boolean requiresNumericArgument(String functionName, int argumentIndex) { return false; } try { - List> signatures = + List> signatures = checker.getParameterTypes().stream() .filter(parameters -> argumentIndex < parameters.size()) .toList(); @@ -458,12 +457,12 @@ public boolean requiresNumericArgument(String functionName, int argumentIndex) { return false; } foundArgument = true; - List acceptedTypes = + List acceptedTypes = signatures.stream().map(parameters -> parameters.get(argumentIndex)).toList(); - if (acceptedTypes.stream().allMatch(ExprCoreType.numberTypes()::contains)) { + if (acceptedTypes.stream().allMatch(SqlTypeUtil::isNumeric)) { continue; } - if (acceptedTypes.stream().anyMatch(type -> type != ExprCoreType.UNKNOWN) + if (acceptedTypes.stream().anyMatch(type -> type.getSqlTypeName() != SqlTypeName.ANY) || !requiresNumericByValidation(checker, signatures, argumentIndex)) { return false; } @@ -475,7 +474,7 @@ public boolean requiresNumericArgument(String functionName, int argumentIndex) { } private boolean requiresNumericByValidation( - PPLTypeChecker checker, List> signatures, int argumentIndex) { + PPLTypeChecker checker, List> signatures, int argumentIndex) { RelDataType numericType = TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE); RelDataType stringType = TYPE_FACTORY.createSqlType(SqlTypeName.VARCHAR); return signatures.stream() 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 521764ba7bb..b14f157b97f 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 @@ -27,11 +27,12 @@ import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.calcite.util.Pair; +import org.opensearch.sql.calcite.type.AbstractExprRelDataType; import org.opensearch.sql.calcite.type.ExprIPType; import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.ExprUDT; import org.opensearch.sql.calcite.utils.UserDefinedFunctionUtils; import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.data.type.ExprType; /** * A custom type checker interface for PPL (Piped Processing Language) functions. @@ -61,12 +62,9 @@ public interface PPLTypeChecker { /** * Get a list of all possible parameter type combinations for the function. * - *

This method is used to generate the allowed signatures for the function based on the - * parameter types. - * * @return a list of lists, where each inner list represents an allowed parameter type combination */ - List> getParameterTypes(); + List> getParameterTypes(); private static boolean validateOperands( List funcTypeFamilies, List operandTypes) { @@ -111,8 +109,8 @@ public String getAllowedSignatures() { } @Override - public List> getParameterTypes() { - return PPLTypeChecker.getExprSignatures(families); + public List> getParameterTypes() { + return PPLTypeChecker.getRelDataTypeSignatures(families); } @Override @@ -150,17 +148,17 @@ public boolean checkOperandTypes(List types) { @Override public String getAllowedSignatures() { if (innerTypeChecker instanceof FamilyOperandTypeChecker familyOperandTypeChecker) { - var allowedExprSignatures = getExprSignatures(familyOperandTypeChecker); - return PPLTypeChecker.formatExprSignatures(allowedExprSignatures); + var allowedSignatures = getRelDataTypeSignatures(familyOperandTypeChecker); + return PPLTypeChecker.formatSignatures(allowedSignatures); } else { return ""; } } @Override - public List> getParameterTypes() { + public List> getParameterTypes() { if (innerTypeChecker instanceof FamilyOperandTypeChecker familyOperandTypeChecker) { - return getExprSignatures(familyOperandTypeChecker); + return getRelDataTypeSignatures(familyOperandTypeChecker); } else { // If the inner type checker is not a FamilyOperandTypeChecker, we cannot provide // parameter types. @@ -232,11 +230,11 @@ public String getAllowedSignatures() { } @Override - public List> getParameterTypes() { - List> parameterTypes = new ArrayList<>(); + public List> getParameterTypes() { + List> parameterTypes = new ArrayList<>(); for (SqlOperandTypeChecker rule : allowedRules) { if (rule instanceof FamilyOperandTypeChecker familyOperandTypeChecker) { - parameterTypes.addAll(getExprSignatures(familyOperandTypeChecker)); + parameterTypes.addAll(getRelDataTypeSignatures(familyOperandTypeChecker)); } else { throw new IllegalArgumentException( "Currently only compositions of FamilyOperandTypeChecker are supported"); @@ -274,7 +272,7 @@ public boolean checkOperandTypes(List types) { } /** - * Modified from {@link SqlTypeUtil#isComparable(RelDataType, RelDataType)} to + * Modified from {@link SqlTypeUtil#isComparable(RelDataType, RelDataType)} for PPL semantics. * * @param type1 first type * @param type2 second type @@ -299,18 +297,38 @@ private static boolean isComparable(RelDataType type1, RelDataType type2) { return true; } - // Numeric types are comparable without the need to cast + // Numeric types are comparable without the need to cast. if (SqlTypeUtil.isNumeric(type1) && SqlTypeUtil.isNumeric(type2)) { return true; } - ExprType exprType1 = OpenSearchTypeFactory.convertRelDataTypeToExprType(type1); - ExprType exprType2 = OpenSearchTypeFactory.convertRelDataTypeToExprType(type2); + // Same UDT kind is comparable. Compare via the ExprUDT tag (not the concrete Java class), + // because createTypeWithNullability on an AbstractExprRelDataType returns a plain ExprSqlType + // that loses the concrete subclass identity. + if (type1 instanceof AbstractExprRelDataType udt1 + && type2 instanceof AbstractExprRelDataType udt2 + && udt1.getUdt() == udt2.getUdt()) { + return true; + } - if (!exprType1.shouldCast(exprType2)) { + // Temporal UDT vs standard temporal of the same kind is comparable (widening produced by + // findWidestType may leave a standard TIMESTAMP field alongside a UDT temporal literal). + if (temporalKind(type1) != null && temporalKind(type1) == temporalKind(type2)) { return true; } + // Plain types are comparable when their SqlTypeFamily matches (CHAR vs VARCHAR, INTEGER vs + // BIGINT, BOOLEAN vs BOOLEAN, etc.). UDTs are excluded since they share the underlying + // VARCHAR family but should not be transparently comparable to plain strings here. + if (!(type1 instanceof AbstractExprRelDataType) + && !(type2 instanceof AbstractExprRelDataType)) { + SqlTypeFamily family1 = type1.getSqlTypeName().getFamily(); + SqlTypeFamily family2 = type2.getSqlTypeName().getFamily(); + if (family1 != null && family1 == family2) { + return true; + } + } + // If one of the arguments is of type 'ANY', return true. return type1.getFamily() == SqlTypeFamily.ANY || type2.getFamily() == SqlTypeFamily.ANY; } @@ -337,9 +355,10 @@ public String getAllowedSignatures() { } @Override - public List> getParameterTypes() { - // Should not be used - return List.of(List.of(ExprCoreType.UNKNOWN, ExprCoreType.UNKNOWN)); + public List> getParameterTypes() { + // Should not be used by coercion since comparable operators don't drive type widening here. + RelDataType anyType = OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.ANY); + return List.of(List.of(anyType, anyType)); } } @@ -397,23 +416,23 @@ public String getAllowedSignatures() { } @Override - public List> getParameterTypes() { + public List> getParameterTypes() { if (internal instanceof FamilyOperandTypeChecker familyChecker) { - return getExprSignatures(familyChecker); + return getRelDataTypeSignatures(familyChecker); } else { - // For unknown type checkers, return UNKNOWN types + // For unknown type checkers, return ANY-typed signatures. int min = internal.getOperandCountRange().getMin(); int max = internal.getOperandCountRange().getMax(); + RelDataType anyType = OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.ANY); if (min == -1 || max == -1) { - // Variable arguments - return a single signature with UNKNOWN - return List.of(List.of(ExprCoreType.UNKNOWN)); + return List.of(List.of(anyType)); } else { - List> parameterTypes = new ArrayList<>(); + List> parameterTypes = new ArrayList<>(); final int MAX_ARGS = 10; max = Math.min(MAX_ARGS, max); for (int i = min; i <= max; i++) { - parameterTypes.add(Collections.nCopies(i, ExprCoreType.UNKNOWN)); + parameterTypes.add(Collections.nCopies(i, anyType)); } return parameterTypes; } @@ -513,28 +532,27 @@ static PPLDefaultTypeChecker wrapDefault(SqlOperandTypeChecker typeChecker) { } /** - * Create a {@link PPLTypeChecker} from a list of allowed signatures consisted of {@link - * ExprType}. This is useful to validate arguments against user-defined types (UDT) that does not - * match any Calcite {@link SqlTypeFamily}. + * Create a {@link PPLTypeChecker} from a list of allowed signatures composed of {@link + * RelDataType}. This is used for functions whose argument types include user-defined types (UDTs) + * that don't fit any standard {@link SqlTypeFamily}. * * @param allowedSignatures a list of allowed signatures, where each signature is a list of {@link - * ExprType} representing the expected types of the function arguments. + * RelDataType} representing the expected types of the function arguments. * @return a {@link PPLTypeChecker} that checks if the operand types match any of the allowed * signatures */ - static PPLTypeChecker wrapUDT(List> allowedSignatures) { + static PPLTypeChecker wrapUDT(List> allowedSignatures) { return new PPLTypeChecker() { @Override public boolean checkOperandTypes(List types) { - List argExprTypes = - types.stream().map(OpenSearchTypeFactory::convertRelDataTypeToExprType).toList(); for (var allowedSignature : allowedSignatures) { if (allowedSignature.size() != types.size()) { continue; // Skip signatures that do not match the operand count } - // Check if the argument types match the allowed signature + // Match each operand against the allowed signature using nullability-insensitive + // equality with UDT-class-aware comparison. if (IntStream.range(0, allowedSignature.size()) - .allMatch(i -> allowedSignature.get(i).equals(argExprTypes.get(i)))) { + .allMatch(i -> typesMatch(allowedSignature.get(i), types.get(i)))) { return true; } } @@ -543,17 +561,53 @@ public boolean checkOperandTypes(List types) { @Override public String getAllowedSignatures() { - return PPLTypeChecker.formatExprSignatures(allowedSignatures); + return PPLTypeChecker.formatSignatures(allowedSignatures); } @Override - public List> getParameterTypes() { + public List> getParameterTypes() { return allowedSignatures; } }; } + /** + * 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. + */ + 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; + } + return expected.getSqlTypeName() == actual.getSqlTypeName(); + } + + /** Returns DATE / TIME / TIMESTAMP for either UDT or standard temporal types; null otherwise. */ + private static SqlTypeName temporalKind(RelDataType type) { + if (type instanceof AbstractExprRelDataType udt) { + return switch (udt.getUdt()) { + case EXPR_DATE -> SqlTypeName.DATE; + case EXPR_TIME -> SqlTypeName.TIME; + case EXPR_TIMESTAMP -> SqlTypeName.TIMESTAMP; + default -> null; + }; + } + SqlTypeName name = type.getSqlTypeName(); + if (name == SqlTypeName.DATE || name == SqlTypeName.TIME || name == SqlTypeName.TIMESTAMP) { + return name; + } + return null; + } + // Util Functions + /** * Generates a list of allowed function signatures based on the provided {@link * FamilyOperandTypeChecker}. The signatures are generated by iterating through the operand count @@ -563,14 +617,15 @@ public List> getParameterTypes() { * to 10 to avoid excessive enumeration. * * @param typeChecker the {@link FamilyOperandTypeChecker} to use for generating signatures - * @return a list of allowed function signatures + * @return a string representation of allowed function signatures */ private static String getFamilySignatures(FamilyOperandTypeChecker typeChecker) { - var allowedExprSignatures = getExprSignatures(typeChecker); - return formatExprSignatures(allowedExprSignatures); + var allowedSignatures = getRelDataTypeSignatures(typeChecker); + return formatSignatures(allowedSignatures); } - private static List> getExprSignatures(FamilyOperandTypeChecker typeChecker) { + private static List> getRelDataTypeSignatures( + FamilyOperandTypeChecker typeChecker) { var operandCountRange = typeChecker.getOperandCountRange(); int min = operandCountRange.getMin(); int max = operandCountRange.getMax(); @@ -578,91 +633,79 @@ private static List> getExprSignatures(FamilyOperandTypeChecker t for (int i = 0; i < min; i++) { families.add(typeChecker.getOperandSqlTypeFamily(i)); } - List> allowedSignatures = new ArrayList<>(getExprSignatures(families)); + List> allowedSignatures = new ArrayList<>(getRelDataTypeSignatures(families)); // Avoid enumerating signatures for infinite args final int MAX_ARGS = 10; max = Math.min(max, MAX_ARGS); for (int i = min; i < max; i++) { families.add(typeChecker.getOperandSqlTypeFamily(i)); - allowedSignatures.addAll(getExprSignatures(families)); + allowedSignatures.addAll(getRelDataTypeSignatures(families)); } return allowedSignatures; } /** - * Converts a {@link SqlTypeFamily} to a list of {@link ExprType}. This method is used to display - * the allowed signatures for functions based on their type families. - * - * @param family the {@link SqlTypeFamily} to convert - * @return a list of {@link ExprType} corresponding to the concrete types of the family + * Converts a {@link SqlTypeFamily} to a list of concrete {@link RelDataType} representatives. + * Used to enumerate allowed signatures and to drive widening in PPL coercion. */ - private static List getExprTypes(SqlTypeFamily family) { - List concreteTypes = - switch (family) { - case DATETIME -> - List.of( - OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.TIMESTAMP), - OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.DATE), - OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.TIME)); - case NUMERIC -> - List.of( - OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER), - OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.DOUBLE)); - // Integer is mapped to BIGINT in family.getDefaultConcreteType - case INTEGER -> - List.of(OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.INTEGER)); - case ANY, IGNORE -> - List.of(OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.ANY)); - case DATETIME_INTERVAL -> - SqlTypeName.INTERVAL_TYPES.stream() - .map( - type -> - OpenSearchTypeFactory.TYPE_FACTORY.createSqlIntervalType( + private static List getRelDataTypes(SqlTypeFamily family) { + OpenSearchTypeFactory tf = OpenSearchTypeFactory.TYPE_FACTORY; + return switch (family) { + case DATETIME -> + List.of( + tf.createUDT(ExprUDT.EXPR_TIMESTAMP), + tf.createUDT(ExprUDT.EXPR_DATE), + tf.createUDT(ExprUDT.EXPR_TIME)); + case TIMESTAMP -> List.of(tf.createUDT(ExprUDT.EXPR_TIMESTAMP)); + case DATE -> List.of(tf.createUDT(ExprUDT.EXPR_DATE)); + case TIME -> List.of(tf.createUDT(ExprUDT.EXPR_TIME)); + case NUMERIC -> + List.of(tf.createSqlType(SqlTypeName.INTEGER), tf.createSqlType(SqlTypeName.DOUBLE)); + // Integer is mapped to BIGINT in family.getDefaultConcreteType + case INTEGER -> List.of(tf.createSqlType(SqlTypeName.INTEGER)); + case ANY, IGNORE -> List.of(tf.createSqlType(SqlTypeName.ANY)); + // ARRAY of nullable ANY, matching convertExprTypeToRelDataType(ARRAY, nullable=true). + // Calcite's default concrete type for ARRAY has a NOT NULL element, which diverges from + // PPL semantics (see #5175: null literals must remain nullable inside an array). + case ARRAY -> List.of(tf.createArrayType(tf.createSqlType(SqlTypeName.ANY, true), -1)); + case BINARY -> List.of(tf.createUDT(ExprUDT.EXPR_BINARY)); + case DATETIME_INTERVAL -> + SqlTypeName.INTERVAL_TYPES.stream() + .map( + type -> + (RelDataType) + tf.createSqlIntervalType( new SqlIntervalQualifier( type.getStartUnit(), type.getEndUnit(), SqlParserPos.ZERO))) - .collect(Collectors.toList()); - default -> { - RelDataType type = family.getDefaultConcreteType(OpenSearchTypeFactory.TYPE_FACTORY); - if (type == null) { - yield List.of(OpenSearchTypeFactory.TYPE_FACTORY.createSqlType(SqlTypeName.OTHER)); - } - yield List.of(type); - } - }; - return concreteTypes.stream() - .map(OpenSearchTypeFactory::convertRelDataTypeToExprType) - .distinct() - .collect(Collectors.toList()); + .collect(Collectors.toList()); + default -> { + RelDataType type = family.getDefaultConcreteType(tf); + if (type == null) { + yield List.of(tf.createSqlType(SqlTypeName.OTHER)); + } + yield List.of(type); + } + }; } /** - * Generates a list of all possible {@link ExprType} signatures based on the provided {@link - * SqlTypeFamily} list. - * - * @param families the list of {@link SqlTypeFamily} to generate signatures for - * @return a list of lists, where each inner list contains {@link ExprType} signatures + * Generates a list of all possible {@link RelDataType} signatures based on the provided {@link + * SqlTypeFamily} list (cartesian product per-position). */ - private static List> getExprSignatures(List families) { - List> exprTypes = - families.stream().map(PPLTypeChecker::getExprTypes).collect(Collectors.toList()); - - // Do a cartesian product of all ExprTypes in the family - return Lists.cartesianProduct(exprTypes); + private static List> getRelDataTypeSignatures(List families) { + List> perPosition = + families.stream().map(PPLTypeChecker::getRelDataTypes).collect(Collectors.toList()); + return Lists.cartesianProduct(perPosition); } /** * Generates a string representation of the function signature based on the provided type * families. The format is a list of type families enclosed in square brackets, e.g.: "[INTEGER, * STRING]". - * - * @param families the list of type families to include in the signature - * @return a string representation of the function signature */ private static String getFamilySignature(List families) { - List> signatures = getExprSignatures(families); - // Convert each signature to a string representation and then concatenate them - return formatExprSignatures(signatures); + return formatSignatures(getRelDataTypeSignatures(families)); } /** @@ -686,13 +729,20 @@ private static boolean isCompositionOr(CompositeOperandTypeChecker typeChecker) return composition == CompositeOperandTypeChecker.Composition.OR; } - private static String formatExprSignatures(List> signatures) { + /** + * Renders a list of {@link RelDataType} signatures as a pipe-separated string of bracketed + * signatures, e.g. {@code [INTEGER,STRING]|[DOUBLE,STRING]}. Each type is rendered through {@link + * OpenSearchTypeFactory#convertRelDataTypeToExprType} so plain SQL types come out as their PPL + * names ({@code STRING}, {@code LONG}, ...) and UDTs as their {@code ExprCoreType} names; {@code + * UNDEFINED} (Calcite {@code NULL}/{@code ANY}) is displayed as {@code ANY}. + */ + static String formatSignatures(List> signatures) { return signatures.stream() .map( types -> "[" + types.stream() - // Display ExprCoreType.UNDEFINED as "ANY" for better interpretability + .map(OpenSearchTypeFactory::convertRelDataTypeToExprType) .map(t -> t == ExprCoreType.UNDEFINED ? "ANY" : t.toString()) .collect(Collectors.joining(",")) + "]") diff --git a/core/src/main/java/org/opensearch/sql/expression/function/UDFOperandMetadata.java b/core/src/main/java/org/opensearch/sql/expression/function/UDFOperandMetadata.java index dc4761b26e7..d67b612e0ea 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/UDFOperandMetadata.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/UDFOperandMetadata.java @@ -17,7 +17,6 @@ import org.apache.calcite.sql.type.SqlOperandMetadata; import org.apache.calcite.sql.type.SqlOperandTypeChecker; import org.apache.calcite.sql.validate.SqlUserDefinedFunction; -import org.opensearch.sql.data.type.ExprType; /** * This class is created for the compatibility with {@link SqlUserDefinedFunction} constructors when @@ -106,11 +105,12 @@ public String getAllowedSignatures(SqlOperator op, String opName) { }; } - static UDFOperandMetadata wrapUDT(List> allowSignatures) { + static UDFOperandMetadata wrapUDT(List> allowSignatures) { return new UDTOperandMetadata(allowSignatures); } - record UDTOperandMetadata(List> allowedParamTypes) implements UDFOperandMetadata { + record UDTOperandMetadata(List> allowedParamTypes) + implements UDFOperandMetadata { @Override public SqlOperandTypeChecker getInnerTypeChecker() { return this; diff --git a/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/CurrentFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/CurrentFunction.java index 49e06afa3d6..f620ffd21c5 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/CurrentFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/CurrentFunction.java @@ -20,8 +20,6 @@ import org.opensearch.sql.data.model.ExprDateValue; import org.opensearch.sql.data.model.ExprTimeValue; import org.opensearch.sql.data.model.ExprTimestampValue; -import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.expression.datetime.DateTimeFunctions; import org.opensearch.sql.expression.function.FunctionProperties; import org.opensearch.sql.expression.function.ImplementorUDF; @@ -39,21 +37,28 @@ *

It returns the current date, time, or timestamp based on the specified return type. */ public class CurrentFunction extends ImplementorUDF { - private final ExprType returnType; - public CurrentFunction(ExprType returnType) { - super(new CurrentFunctionImplementor(returnType), NullPolicy.NONE); - this.returnType = returnType; + /** Discriminates the temporal flavour at registration time, decoupled from {@code ExprType}. */ + public enum Kind { + DATE, + TIME, + TIMESTAMP + } + + private final Kind kind; + + public CurrentFunction(Kind kind) { + super(new CurrentFunctionImplementor(kind), NullPolicy.NONE); + this.kind = kind; } @Override public SqlReturnTypeInference getReturnTypeInference() { return opBinding -> - switch (returnType) { - case ExprCoreType.DATE -> UserDefinedFunctionUtils.NULLABLE_DATE_UDT; - case ExprCoreType.TIME -> UserDefinedFunctionUtils.NULLABLE_TIME_UDT; - case ExprCoreType.TIMESTAMP -> UserDefinedFunctionUtils.NULLABLE_TIMESTAMP_UDT; - default -> throw new IllegalArgumentException("Unsupported return type: " + returnType); + switch (kind) { + case DATE -> UserDefinedFunctionUtils.NULLABLE_DATE_UDT; + case TIME -> UserDefinedFunctionUtils.NULLABLE_TIME_UDT; + case TIMESTAMP -> UserDefinedFunctionUtils.NULLABLE_TIMESTAMP_UDT; }; } @@ -64,18 +69,17 @@ public UDFOperandMetadata getOperandMetadata() { @RequiredArgsConstructor public static class CurrentFunctionImplementor implements NotNullImplementor { - private final ExprType returnType; + private final Kind kind; @Override public Expression implement( RexToLixTranslator translator, RexCall call, List translatedOperands) { String functionName = - switch (returnType) { - case ExprCoreType.DATE -> "currentDate"; - case ExprCoreType.TIME -> "currentTime"; - case ExprCoreType.TIMESTAMP -> "currentTimestamp"; - default -> throw new IllegalArgumentException("Unsupported return type: " + returnType); + switch (kind) { + case DATE -> "currentDate"; + case TIME -> "currentTime"; + case TIMESTAMP -> "currentTimestamp"; }; Expression properties = diff --git a/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/PeriodNameFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/PeriodNameFunction.java index 109bad16bf1..a53c733de66 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/PeriodNameFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/udf/datetime/PeriodNameFunction.java @@ -17,11 +17,9 @@ import org.apache.calcite.linq4j.tree.Expressions; import org.apache.calcite.rex.RexCall; import org.apache.calcite.sql.type.SqlReturnTypeInference; -import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.calcite.utils.PPLReturnTypes; import org.opensearch.sql.data.model.ExprDateValue; -import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.expression.function.ImplementorUDF; import org.opensearch.sql.expression.function.UDFOperandMetadata; @@ -65,9 +63,6 @@ public PeriodNameFunctionImplementor(TimeUnit periodUnit) { @Override public Expression implement( RexToLixTranslator translator, RexCall call, List translatedOperands) { - ExprType dateType = - OpenSearchTypeFactory.convertRelDataTypeToExprType( - call.getOperands().getFirst().getType()); return Expressions.call( PeriodNameFunctionImplementor.class, "name", diff --git a/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CidrMatchFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CidrMatchFunction.java index 11fdd7947af..b213181038b 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CidrMatchFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CidrMatchFunction.java @@ -14,10 +14,10 @@ import org.apache.calcite.rex.RexCall; import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlReturnTypeInference; +import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.data.model.ExprIpValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; -import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.expression.function.ImplementorUDF; import org.opensearch.sql.expression.function.UDFOperandMetadata; import org.opensearch.sql.expression.ip.IPFunctions; @@ -51,9 +51,9 @@ public UDFOperandMetadata getOperandMetadata() { // We use a specific type checker to serve return UDFOperandMetadata.wrapUDT( List.of( - List.of(ExprCoreType.IP, ExprCoreType.STRING), - List.of(ExprCoreType.STRING, ExprCoreType.STRING), - List.of(ExprCoreType.BINARY, ExprCoreType.STRING))); + List.of(PPLOperandTypes.IP_UDT, PPLOperandTypes.STRING_T), + List.of(PPLOperandTypes.STRING_T, PPLOperandTypes.STRING_T), + List.of(PPLOperandTypes.BINARY_UDT, PPLOperandTypes.STRING_T))); } public static class CidrMatchImplementor implements NotNullImplementor { diff --git a/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CompareIpFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CompareIpFunction.java index ce200323f60..34d5fec2f3e 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CompareIpFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/CompareIpFunction.java @@ -25,8 +25,8 @@ import org.apache.calcite.sql.type.SqlReturnTypeInference; import org.apache.calcite.sql.validate.SqlUserDefinedFunction; import org.checkerframework.checker.nullness.qual.Nullable; +import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.data.model.ExprIpValue; -import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.expression.function.ImplementorUDF; import org.opensearch.sql.expression.function.PPLBuiltinOperators; import org.opensearch.sql.expression.function.UDFOperandMetadata; @@ -120,7 +120,8 @@ public SqlReturnTypeInference getReturnTypeInference() { @Override public UDFOperandMetadata getOperandMetadata() { - return UDFOperandMetadata.wrapUDT(List.of(List.of(ExprCoreType.IP, ExprCoreType.IP))); + return UDFOperandMetadata.wrapUDT( + List.of(List.of(PPLOperandTypes.IP_UDT, PPLOperandTypes.IP_UDT))); } public static class CompareImplementor implements NotNullImplementor { diff --git a/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/IPFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/IPFunction.java index baf6b8a37e1..13dccfee6ae 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/IPFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/udf/ip/IPFunction.java @@ -11,13 +11,14 @@ import org.apache.calcite.adapter.enumerable.RexToLixTranslator; import org.apache.calcite.linq4j.tree.Expression; import org.apache.calcite.linq4j.tree.Expressions; +import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rex.RexCall; import org.apache.calcite.sql.type.SqlReturnTypeInference; -import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; +import org.apache.calcite.sql.type.SqlTypeUtil; +import org.opensearch.sql.calcite.type.ExprIPType; +import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.calcite.utils.PPLReturnTypes; import org.opensearch.sql.data.model.ExprIpValue; -import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.expression.function.ImplementorUDF; import org.opensearch.sql.expression.function.UDFOperandMetadata; @@ -41,7 +42,7 @@ public IPFunction() { @Override public UDFOperandMetadata getOperandMetadata() { return UDFOperandMetadata.wrapUDT( - List.of(List.of(ExprCoreType.IP), List.of(ExprCoreType.STRING))); + List.of(List.of(PPLOperandTypes.IP_UDT), List.of(PPLOperandTypes.STRING_T))); } @Override @@ -57,12 +58,10 @@ public Expression implement( if (call.getOperands().size() != 1) { throw new IllegalArgumentException("IP function requires exactly one operand"); } - ExprType argType = - OpenSearchTypeFactory.convertRelDataTypeToExprType( - call.getOperands().getFirst().getType()); - if (argType == ExprCoreType.IP) { + RelDataType argType = call.getOperands().getFirst().getType(); + if (argType instanceof ExprIPType) { return translatedOperands.getFirst(); - } else if (argType == ExprCoreType.STRING) { + } else if (SqlTypeUtil.isCharacter(argType)) { return Expressions.new_(ExprIpValue.class, translatedOperands); } else { throw new ExpressionEvaluationException( diff --git a/core/src/test/java/org/opensearch/sql/expression/function/CoercionUtilsTest.java b/core/src/test/java/org/opensearch/sql/expression/function/CoercionUtilsTest.java index 5533fd90915..bff5f5bdb53 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/CoercionUtilsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/CoercionUtilsTest.java @@ -6,11 +6,6 @@ package org.opensearch.sql.expression.function; import static org.junit.jupiter.api.Assertions.*; -import static org.opensearch.sql.data.type.ExprCoreType.BINARY; -import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; -import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; -import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; import java.util.List; import java.util.stream.Stream; @@ -23,33 +18,51 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; +import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory.ExprUDT; +import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.data.type.ExprCoreType; -import org.opensearch.sql.data.type.ExprType; class CoercionUtilsTest { - private static final RexBuilder REX_BUILDER = new RexBuilder(OpenSearchTypeFactory.TYPE_FACTORY); + private static final OpenSearchTypeFactory TF = OpenSearchTypeFactory.TYPE_FACTORY; + private static final RexBuilder REX_BUILDER = new RexBuilder(TF); - private static RexNode nullLiteral(ExprCoreType type) { - return REX_BUILDER.makeNullLiteral(OpenSearchTypeFactory.convertExprTypeToRelDataType(type)); + private static RelDataType type(SqlTypeName name) { + return TF.createSqlType(name); + } + + private static RexNode nullLiteral(RelDataType type) { + return REX_BUILDER.makeNullLiteral(type); } private static Stream commonWidestTypeArguments() { + RelDataType string = type(SqlTypeName.VARCHAR); + RelDataType integer = type(SqlTypeName.INTEGER); + RelDataType doubleT = type(SqlTypeName.DOUBLE); + RelDataType bool = type(SqlTypeName.BOOLEAN); + RelDataType binary = TF.createUDT(ExprUDT.EXPR_BINARY); + RelDataType varbinary = type(SqlTypeName.VARBINARY); return Stream.of( - Arguments.of(STRING, INTEGER, DOUBLE), - Arguments.of(INTEGER, STRING, DOUBLE), - Arguments.of(STRING, DOUBLE, DOUBLE), - Arguments.of(INTEGER, BOOLEAN, null), - Arguments.of(BINARY, STRING, BINARY), - Arguments.of(STRING, BINARY, BINARY)); + Arguments.of(string, integer, doubleT), + Arguments.of(integer, string, doubleT), + Arguments.of(string, doubleT, doubleT), + Arguments.of(integer, bool, null), + Arguments.of(binary, string, varbinary), + Arguments.of(string, binary, varbinary)); } @ParameterizedTest @MethodSource("commonWidestTypeArguments") - public void findCommonWidestType( - ExprCoreType left, ExprCoreType right, ExprCoreType expectedCommonType) { - assertEquals( - expectedCommonType, CoercionUtils.resolveCommonType(left, right).orElseGet(() -> null)); + public void findCommonWidestType(RelDataType left, RelDataType right, RelDataType expected) { + if (expected == null) { + assertTrue(CoercionUtils.resolveCommonType(left, right).isEmpty()); + } else { + assertEquals( + expected.getSqlTypeName(), + CoercionUtils.resolveCommonType(left, right) + .map(RelDataType::getSqlTypeName) + .orElse(null)); + } } @Test @@ -88,42 +101,54 @@ void widenArgumentsUnifiesPlainTimestampWithDateUdtBounds() { @Test void castArgumentsReturnsExactMatchWhenAvailable() { - PPLTypeChecker typeChecker = new StubTypeChecker(List.of(List.of(INTEGER), List.of(DOUBLE))); - List arguments = List.of(nullLiteral(INTEGER)); + PPLTypeChecker typeChecker = + new StubTypeChecker( + List.of(List.of(type(SqlTypeName.INTEGER)), List.of(type(SqlTypeName.DOUBLE)))); + List arguments = List.of(nullLiteral(type(SqlTypeName.INTEGER))); List result = CoercionUtils.castArguments(REX_BUILDER, typeChecker, arguments); assertNotNull(result); assertEquals(1, result.size()); - assertEquals( - INTEGER, OpenSearchTypeFactory.convertRelDataTypeToExprType(result.getFirst().getType())); + assertEquals(SqlTypeName.INTEGER, result.getFirst().getType().getSqlTypeName()); } @Test void castArgumentsFallsBackToWidestCandidate() { PPLTypeChecker typeChecker = - new StubTypeChecker(List.of(List.of(ExprCoreType.LONG), List.of(DOUBLE))); - List arguments = List.of(nullLiteral(STRING)); + new StubTypeChecker( + List.of(List.of(type(SqlTypeName.BIGINT)), List.of(type(SqlTypeName.DOUBLE)))); + List arguments = List.of(nullLiteral(type(SqlTypeName.VARCHAR))); List result = CoercionUtils.castArguments(REX_BUILDER, typeChecker, arguments); assertNotNull(result); - assertEquals( - DOUBLE, OpenSearchTypeFactory.convertRelDataTypeToExprType(result.getFirst().getType())); + assertEquals(SqlTypeName.DOUBLE, result.getFirst().getType().getSqlTypeName()); } @Test void castArgumentsReturnsNullWhenNoCompatibleSignatureExists() { - PPLTypeChecker typeChecker = new StubTypeChecker(List.of(List.of(ExprCoreType.GEO_POINT))); - List arguments = List.of(nullLiteral(INTEGER)); + PPLTypeChecker typeChecker = new StubTypeChecker(List.of(List.of(type(SqlTypeName.GEOMETRY)))); + List arguments = List.of(nullLiteral(type(SqlTypeName.INTEGER))); assertNull(CoercionUtils.castArguments(REX_BUILDER, typeChecker, arguments)); } + @Test + void hasStringDetectsCharacterArguments() { + assertTrue( + CoercionUtils.hasString( + List.of( + nullLiteral(type(SqlTypeName.VARCHAR)), nullLiteral(type(SqlTypeName.INTEGER))))); + assertFalse( + CoercionUtils.hasString( + List.of(nullLiteral(type(SqlTypeName.INTEGER)), nullLiteral(PPLOperandTypes.IP_UDT)))); + } + private static class StubTypeChecker implements PPLTypeChecker { - private final List> signatures; + private final List> signatures; - private StubTypeChecker(List> signatures) { + private StubTypeChecker(List> signatures) { this.signatures = signatures; } @@ -138,7 +163,7 @@ public String getAllowedSignatures() { } @Override - public List> getParameterTypes() { + public List> getParameterTypes() { return signatures; } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/functions/GeoIpFunction.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/functions/GeoIpFunction.java index 83b1915f6b5..d2e3c52bb6b 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/functions/GeoIpFunction.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/functions/GeoIpFunction.java @@ -19,12 +19,12 @@ import org.apache.calcite.sql.type.SqlReturnTypeInference; import org.apache.calcite.sql.type.SqlTypeName; import org.opensearch.geospatial.action.IpEnrichmentActionClient; +import org.opensearch.sql.calcite.utils.PPLOperandTypes; import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.data.model.ExprIpValue; import org.opensearch.sql.data.model.ExprStringValue; import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.expression.function.ImplementorUDF; import org.opensearch.sql.expression.function.UDFOperandMetadata; import org.opensearch.transport.client.node.NodeClient; @@ -60,8 +60,8 @@ public SqlReturnTypeInference getReturnTypeInference() { public UDFOperandMetadata getOperandMetadata() { return UDFOperandMetadata.wrapUDT( List.of( - List.of(ExprCoreType.STRING, ExprCoreType.IP), - List.of(ExprCoreType.STRING, ExprCoreType.IP, ExprCoreType.STRING))); + List.of(PPLOperandTypes.STRING_T, PPLOperandTypes.IP_UDT), + List.of(PPLOperandTypes.STRING_T, PPLOperandTypes.IP_UDT, PPLOperandTypes.STRING_T))); } public static class GeoIPImplementor implements NotNullImplementor {