From f3cefb8076d9085842694dcd69518652efb76e0b Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Mon, 26 Aug 2024 14:45:40 -0400 Subject: [PATCH 1/7] combine 3 commonTypes into one --- .../esql/core/type/DataTypeConverter.java | 80 ---------------- .../core/type/DataTypeConversionTests.java | 20 ---- .../predicate/operator/arithmetic/Add.java | 1 - .../arithmetic/ArithmeticOperation.java | 7 +- .../BinaryComparisonInversible.java | 2 +- .../predicate/operator/arithmetic/Div.java | 1 - .../arithmetic/EsqlArithmeticOperation.java | 5 +- .../predicate/operator/arithmetic/Mul.java | 1 - .../predicate/operator/arithmetic/Sub.java | 1 - .../comparison/EsqlBinaryComparison.java | 4 +- .../predicate/operator/comparison/In.java | 4 +- .../rules/SimplifyComparisonsArithmetics.java | 4 +- .../esql/type/EsqlDataTypeConverter.java | 96 ++++++++++++++++--- .../xpack/esql/type/EsqlDataTypeRegistry.java | 24 ----- .../esql/io/stream/PlanNamedTypesTests.java | 2 +- .../esql/type/EsqlDataTypeConverterTests.java | 33 +++++++ 16 files changed, 129 insertions(+), 156 deletions(-) rename x-pack/plugin/{esql-core/src/main/java/org/elasticsearch/xpack/esql/core => esql/src/main/java/org/elasticsearch/xpack/esql}/expression/predicate/operator/arithmetic/ArithmeticOperation.java (80%) rename x-pack/plugin/{esql-core/src/main/java/org/elasticsearch/xpack/esql/core => esql/src/main/java/org/elasticsearch/xpack/esql}/expression/predicate/operator/arithmetic/BinaryComparisonInversible.java (91%) diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataTypeConverter.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataTypeConverter.java index 1e68d63ef7bb1..78b395503e700 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataTypeConverter.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataTypeConverter.java @@ -38,7 +38,6 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION; import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime; -import static org.elasticsearch.xpack.esql.core.type.DataType.isPrimitiveAndSupported; import static org.elasticsearch.xpack.esql.core.type.DataType.isString; import static org.elasticsearch.xpack.esql.core.util.NumericUtils.UNSIGNED_LONG_MAX; import static org.elasticsearch.xpack.esql.core.util.NumericUtils.inUnsignedLongRange; @@ -51,85 +50,6 @@ public final class DataTypeConverter { private DataTypeConverter() {} - /** - * Returns the type compatible with both left and right types - *

- * If one of the types is null - returns another type - * If both types are numeric - returns type with the highest precision int < long < float < double - * If one of the types is string and another numeric - returns numeric - */ - public static DataType commonType(DataType left, DataType right) { - if (left == right) { - return left; - } - if (left == NULL) { - return right; - } - if (right == NULL) { - return left; - } - if (isString(left) && isString(right)) { - if (left == TEXT || right == TEXT) { - return TEXT; - } - if (left == KEYWORD) { - return KEYWORD; - } - return right; - } - if (left.isNumeric() && right.isNumeric()) { - int lsize = left.estimatedSize().orElseThrow(); - int rsize = right.estimatedSize().orElseThrow(); - // if one is int - if (left.isWholeNumber()) { - // promote the highest int - if (right.isWholeNumber()) { - if (left == UNSIGNED_LONG || right == UNSIGNED_LONG) { - return UNSIGNED_LONG; - } - return lsize > rsize ? left : right; - } - // promote the rational - return right; - } - // try the other side - if (right.isWholeNumber()) { - return left; - } - // promote the highest rational - return lsize > rsize ? left : right; - } - if (isString(left)) { - if (right.isNumeric()) { - return right; - } - } - if (isString(right)) { - if (left.isNumeric()) { - return left; - } - } - - if (isDateTime(left) && isDateTime(right)) { - return DATETIME; - } - - // none found - return null; - } - - /** - * Returns true if the from type can be converted to the to type, false - otherwise - */ - public static boolean canConvert(DataType from, DataType to) { - // Special handling for nulls and if conversion is not requires - if (from == to || from == NULL) { - return true; - } - // only primitives are supported so far - return isPrimitiveAndSupported(from) && isPrimitiveAndSupported(to) && converterFor(from, to) != null; - } - /** * Get the conversion from one type to another. */ diff --git a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/type/DataTypeConversionTests.java b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/type/DataTypeConversionTests.java index 929aa1c0eab49..d1d8436c7b392 100644 --- a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/type/DataTypeConversionTests.java +++ b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/type/DataTypeConversionTests.java @@ -32,7 +32,6 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.UNSUPPORTED; import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION; -import static org.elasticsearch.xpack.esql.core.type.DataTypeConverter.commonType; import static org.elasticsearch.xpack.esql.core.type.DataTypeConverter.converterFor; import static org.elasticsearch.xpack.esql.core.util.DateUtils.asDateTime; @@ -519,25 +518,6 @@ public void testConversionToIdentity() { assertEquals(10, conversion.convert(10)); } - public void testCommonType() { - assertEquals(BOOLEAN, commonType(BOOLEAN, NULL)); - assertEquals(BOOLEAN, commonType(NULL, BOOLEAN)); - assertEquals(BOOLEAN, commonType(BOOLEAN, BOOLEAN)); - assertEquals(NULL, commonType(NULL, NULL)); - assertEquals(INTEGER, commonType(INTEGER, KEYWORD)); - assertEquals(LONG, commonType(TEXT, LONG)); - assertEquals(SHORT, commonType(SHORT, BYTE)); - assertEquals(FLOAT, commonType(BYTE, FLOAT)); - assertEquals(FLOAT, commonType(FLOAT, INTEGER)); - assertEquals(UNSIGNED_LONG, commonType(UNSIGNED_LONG, LONG)); - assertEquals(DOUBLE, commonType(DOUBLE, FLOAT)); - assertEquals(FLOAT, commonType(FLOAT, UNSIGNED_LONG)); - - // strings - assertEquals(TEXT, commonType(TEXT, KEYWORD)); - assertEquals(TEXT, commonType(KEYWORD, TEXT)); - } - public void testEsDataTypes() { for (DataType type : DataType.types()) { assertEquals(type, DataType.fromTypeName(type.typeName())); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Add.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Add.java index b6ec9b6fd0e23..8f8d885ee379b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Add.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Add.java @@ -12,7 +12,6 @@ import org.elasticsearch.compute.ann.Evaluator; import org.elasticsearch.compute.ann.Fixed; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.BinaryComparisonInversible; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.util.NumericUtils; diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/operator/arithmetic/ArithmeticOperation.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/ArithmeticOperation.java similarity index 80% rename from x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/operator/arithmetic/ArithmeticOperation.java rename to x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/ArithmeticOperation.java index 8dc0f58083179..cb7e7c4643fb9 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/operator/arithmetic/ArithmeticOperation.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/ArithmeticOperation.java @@ -4,16 +4,17 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -package org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic; +package org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal; import org.elasticsearch.xpack.esql.core.expression.predicate.BinaryOperator; +import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.BinaryArithmeticOperation; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; -import org.elasticsearch.xpack.esql.core.type.DataTypeConverter; import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isNumeric; +import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType; public abstract class ArithmeticOperation extends BinaryOperator { @@ -36,7 +37,7 @@ public ArithmeticOperation swapLeftAndRight() { @Override public DataType dataType() { if (dataType == null) { - dataType = DataTypeConverter.commonType(left().dataType(), right().dataType()); + dataType = commonType(left().dataType(), right().dataType()); } return dataType; } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/operator/arithmetic/BinaryComparisonInversible.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/BinaryComparisonInversible.java similarity index 91% rename from x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/operator/arithmetic/BinaryComparisonInversible.java rename to x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/BinaryComparisonInversible.java index 358ad59ec6356..b0ab4c48d970e 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/predicate/operator/arithmetic/BinaryComparisonInversible.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/BinaryComparisonInversible.java @@ -5,7 +5,7 @@ * 2.0. */ -package org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic; +package org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.tree.Source; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Div.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Div.java index 0e4c506a90d85..f1e197cf350b6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Div.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Div.java @@ -11,7 +11,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.compute.ann.Evaluator; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.BinaryComparisonInversible; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/EsqlArithmeticOperation.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/EsqlArithmeticOperation.java index 647071c44cfd3..400e70b641111 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/EsqlArithmeticOperation.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/EsqlArithmeticOperation.java @@ -13,14 +13,12 @@ import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator; import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.ArithmeticOperation; import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.BinaryArithmeticOperation; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper; import org.elasticsearch.xpack.esql.expression.function.scalar.math.Cast; import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput; -import org.elasticsearch.xpack.esql.type.EsqlDataTypeRegistry; import java.io.IOException; import java.util.List; @@ -31,6 +29,7 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER; import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; +import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType; public abstract class EsqlArithmeticOperation extends ArithmeticOperation implements EvaluatorMapper { public static List getNamedWriteables() { @@ -133,7 +132,7 @@ public Object fold() { public DataType dataType() { if (dataType == null) { - dataType = EsqlDataTypeRegistry.INSTANCE.commonType(left().dataType(), right().dataType()); + dataType = commonType(left().dataType(), right().dataType()); } return dataType; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Mul.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Mul.java index a73562ff153b2..03981a821f52d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Mul.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Mul.java @@ -11,7 +11,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.compute.ann.Evaluator; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.BinaryComparisonInversible; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.util.NumericUtils; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Sub.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Sub.java index ee2ccc3b7107a..27f5579129cc9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Sub.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/Sub.java @@ -12,7 +12,6 @@ import org.elasticsearch.compute.ann.Evaluator; import org.elasticsearch.compute.ann.Fixed; import org.elasticsearch.xpack.esql.core.expression.Expression; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.BinaryComparisonInversible; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EsqlBinaryComparison.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EsqlBinaryComparison.java index 52d4c111b2eae..b50d70e69819d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EsqlBinaryComparison.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EsqlBinaryComparison.java @@ -22,7 +22,6 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.math.Cast; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput; -import org.elasticsearch.xpack.esql.type.EsqlDataTypeRegistry; import java.io.IOException; import java.time.ZoneId; @@ -32,6 +31,7 @@ import static org.elasticsearch.common.logging.LoggerMessageFormat.format; import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; +import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType; public abstract class EsqlBinaryComparison extends BinaryComparison implements EvaluatorMapper { public static List getNamedWriteables() { @@ -172,7 +172,7 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator( Function toEvaluator ) { // Our type is always boolean, so figure out the evaluator type from the inputs - DataType commonType = EsqlDataTypeRegistry.INSTANCE.commonType(left().dataType(), right().dataType()); + DataType commonType = commonType(left().dataType(), right().dataType()); EvalOperator.ExpressionEvaluator.Factory lhs; EvalOperator.ExpressionEvaluator.Factory rhs; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java index 636b31fcc691b..333f32e82c579 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/In.java @@ -27,7 +27,7 @@ import org.elasticsearch.xpack.esql.expression.function.scalar.EsqlScalarFunction; import org.elasticsearch.xpack.esql.expression.function.scalar.math.Cast; import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput; -import org.elasticsearch.xpack.esql.type.EsqlDataTypeRegistry; +import org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter; import java.io.IOException; import java.util.BitSet; @@ -269,7 +269,7 @@ private DataType commonType() { break; } } - commonType = EsqlDataTypeRegistry.INSTANCE.commonType(commonType, e.dataType()); + commonType = EsqlDataTypeConverter.commonType(commonType, e.dataType()); } return commonType; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/SimplifyComparisonsArithmetics.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/SimplifyComparisonsArithmetics.java index 4ef069ea16d04..fe83aeb647bf9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/SimplifyComparisonsArithmetics.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/SimplifyComparisonsArithmetics.java @@ -9,10 +9,10 @@ import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.expression.Literal; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.ArithmeticOperation; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.BinaryComparisonInversible; import org.elasticsearch.xpack.esql.core.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.esql.core.type.DataType; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.ArithmeticOperation; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.BinaryComparisonInversible; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Neg; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Sub; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java index 1572f8950e0ac..b0e6acda59ff5 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java @@ -58,6 +58,7 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_POINT; import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_SHAPE; import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME; +import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_PERIOD; import static org.elasticsearch.xpack.esql.core.type.DataType.DOUBLE; import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT; import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_SHAPE; @@ -67,9 +68,14 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.NULL; import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; +import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION; import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION; -import static org.elasticsearch.xpack.esql.core.type.DataType.isPrimitiveAndSupported; +import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime; +import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTimeOrTemporal; +import static org.elasticsearch.xpack.esql.core.type.DataType.isNullOrDatePeriod; +import static org.elasticsearch.xpack.esql.core.type.DataType.isNullOrTemporalAmount; +import static org.elasticsearch.xpack.esql.core.type.DataType.isNullOrTimeDuration; import static org.elasticsearch.xpack.esql.core.type.DataType.isString; import static org.elasticsearch.xpack.esql.core.type.DataTypeConverter.safeDoubleToLong; import static org.elasticsearch.xpack.esql.core.type.DataTypeConverter.safeToInt; @@ -107,18 +113,6 @@ public class EsqlDataTypeConverter { entry(VERSION, ToVersion::new) ); - /** - * Returns true if the from type can be converted to the to type, false - otherwise - */ - public static boolean canConvert(DataType from, DataType to) { - // Special handling for nulls and if conversion is not requires - if (from == to || from == NULL) { - return true; - } - // only primitives are supported so far - return isPrimitiveAndSupported(from) && isPrimitiveAndSupported(to) && converterFor(from, to) != null; - } - public static Converter converterFor(DataType from, DataType to) { // TODO move EXPRESSION_TO_LONG here if there is no regression if (isString(from)) { @@ -230,8 +224,82 @@ public static Object convert(Object value, DataType dataType) { return converter.convert(value); } + /** + * Returns the type compatible with both left and right types + *

+ * If one of the types is null - returns another type + * If both types are numeric - returns type with the highest precision int < long < float < double + * If one of the types is string and another numeric - returns numeric + */ public static DataType commonType(DataType left, DataType right) { - return DataTypeConverter.commonType(left, right); + if (left == right) { + return left; + } + if (left == NULL) { + return right; + } + if (right == NULL) { + return left; + } + if (isDateTimeOrTemporal(left) || isDateTimeOrTemporal(right)) { + if ((isDateTime(left) && isNullOrTemporalAmount(right)) || (isNullOrTemporalAmount(left) && isDateTime(right))) { + return DATETIME; + } + if (isNullOrTimeDuration(left) && isNullOrTimeDuration(right)) { + return TIME_DURATION; + } + if (isNullOrDatePeriod(left) && isNullOrDatePeriod(right)) { + return DATE_PERIOD; + } + } + if (isString(left) && isString(right)) { + if (left == TEXT || right == TEXT) { + return TEXT; + } + if (left == KEYWORD) { + return KEYWORD; + } + return right; + } + if (left.isNumeric() && right.isNumeric()) { + int lsize = left.estimatedSize().orElseThrow(); + int rsize = right.estimatedSize().orElseThrow(); + // if one is int + if (left.isWholeNumber()) { + // promote the highest int + if (right.isWholeNumber()) { + if (left == UNSIGNED_LONG || right == UNSIGNED_LONG) { + return UNSIGNED_LONG; + } + return lsize > rsize ? left : right; + } + // promote the rational + return right; + } + // try the other side + if (right.isWholeNumber()) { + return left; + } + // promote the highest rational + return lsize > rsize ? left : right; + } + if (isString(left)) { + if (right.isNumeric()) { + return right; + } + } + if (isString(right)) { + if (left.isNumeric()) { + return left; + } + } + + if (isDateTime(left) && isDateTime(right)) { + return DATETIME; + } + + // none found + return null; } // generally supporting abbreviations from https://en.wikipedia.org/wiki/Unit_of_time diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeRegistry.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeRegistry.java index 96e206b82cf0c..f8e8cd37dc8b2 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeRegistry.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeRegistry.java @@ -10,15 +10,6 @@ import org.elasticsearch.index.mapper.TimeSeriesParams; import org.elasticsearch.xpack.esql.core.type.DataType; -import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME; -import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_PERIOD; -import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION; -import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime; -import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTimeOrTemporal; -import static org.elasticsearch.xpack.esql.core.type.DataType.isNullOrDatePeriod; -import static org.elasticsearch.xpack.esql.core.type.DataType.isNullOrTemporalAmount; -import static org.elasticsearch.xpack.esql.core.type.DataType.isNullOrTimeDuration; - public class EsqlDataTypeRegistry { public static final EsqlDataTypeRegistry INSTANCE = new EsqlDataTypeRegistry(); @@ -35,19 +26,4 @@ public DataType fromEs(String typeName, TimeSeriesParams.MetricType metricType) */ return metricType == TimeSeriesParams.MetricType.COUNTER ? type.widenSmallNumeric().counter() : type; } - - public DataType commonType(DataType left, DataType right) { - if (isDateTimeOrTemporal(left) || isDateTimeOrTemporal(right)) { - if ((isDateTime(left) && isNullOrTemporalAmount(right)) || (isNullOrTemporalAmount(left) && isDateTime(right))) { - return DATETIME; - } - if (isNullOrTimeDuration(left) && isNullOrTimeDuration(right)) { - return TIME_DURATION; - } - if (isNullOrDatePeriod(left) && isNullOrDatePeriod(right)) { - return DATE_PERIOD; - } - } - return EsqlDataTypeConverter.commonType(left, right); - } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanNamedTypesTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanNamedTypesTests.java index a5f2adbc1fc29..b796ba84b1fc5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanNamedTypesTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/io/stream/PlanNamedTypesTests.java @@ -20,13 +20,13 @@ import org.elasticsearch.xpack.esql.core.expression.FieldAttribute; import org.elasticsearch.xpack.esql.core.expression.NameId; import org.elasticsearch.xpack.esql.core.expression.Nullability; -import org.elasticsearch.xpack.esql.core.expression.predicate.operator.arithmetic.ArithmeticOperation; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.core.type.KeywordEsField; import org.elasticsearch.xpack.esql.expression.Order; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Add; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.ArithmeticOperation; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Div; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Mod; import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.Mul; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java index 0997c88aac2b0..bf4f746367fda 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java @@ -9,6 +9,19 @@ import org.elasticsearch.test.ESTestCase; +import static org.elasticsearch.xpack.esql.core.type.DataType.BOOLEAN; +import static org.elasticsearch.xpack.esql.core.type.DataType.BYTE; +import static org.elasticsearch.xpack.esql.core.type.DataType.DOUBLE; +import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT; +import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER; +import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; +import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; +import static org.elasticsearch.xpack.esql.core.type.DataType.NULL; +import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT; +import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; +import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; +import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType; + public class EsqlDataTypeConverterTests extends ESTestCase { public void testNanoTimeToString() { @@ -16,4 +29,24 @@ public void testNanoTimeToString() { long actual = EsqlDataTypeConverter.dateNanosToLong(EsqlDataTypeConverter.nanoTimeToString(expected)); assertEquals(expected, actual); } + + public void testCommonType() { + assertEquals(BOOLEAN, commonType(BOOLEAN, NULL)); + assertEquals(BOOLEAN, commonType(NULL, BOOLEAN)); + assertEquals(BOOLEAN, commonType(BOOLEAN, BOOLEAN)); + assertEquals(NULL, commonType(NULL, NULL)); + assertEquals(INTEGER, commonType(INTEGER, KEYWORD)); + assertEquals(LONG, commonType(TEXT, LONG)); + assertEquals(SHORT, commonType(SHORT, BYTE)); + assertEquals(FLOAT, commonType(BYTE, FLOAT)); + assertEquals(FLOAT, commonType(FLOAT, INTEGER)); + assertEquals(UNSIGNED_LONG, commonType(UNSIGNED_LONG, LONG)); + assertEquals(DOUBLE, commonType(DOUBLE, FLOAT)); + assertEquals(FLOAT, commonType(FLOAT, UNSIGNED_LONG)); + + // strings + assertEquals(TEXT, commonType(TEXT, KEYWORD)); + assertEquals(TEXT, commonType(KEYWORD, TEXT)); + } + } From 82118c320cf30f7ffed963082a9906f961ad71b7 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Mon, 26 Aug 2024 16:28:11 -0400 Subject: [PATCH 2/7] refactor --- .../xpack/esql/type/EsqlDataTypeConverter.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java index b0e6acda59ff5..b2701e7d9c090 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java @@ -283,21 +283,6 @@ public static DataType commonType(DataType left, DataType right) { // promote the highest rational return lsize > rsize ? left : right; } - if (isString(left)) { - if (right.isNumeric()) { - return right; - } - } - if (isString(right)) { - if (left.isNumeric()) { - return left; - } - } - - if (isDateTime(left) && isDateTime(right)) { - return DATETIME; - } - // none found return null; } From db1e093c5b35cf8e8cb613fd167a54e0f0503d91 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Mon, 26 Aug 2024 18:57:42 -0400 Subject: [PATCH 3/7] fix test --- .../xpack/esql/type/EsqlDataTypeConverterTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java index bf4f746367fda..eaa7d4825ef7c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java @@ -35,8 +35,8 @@ public void testCommonType() { assertEquals(BOOLEAN, commonType(NULL, BOOLEAN)); assertEquals(BOOLEAN, commonType(BOOLEAN, BOOLEAN)); assertEquals(NULL, commonType(NULL, NULL)); - assertEquals(INTEGER, commonType(INTEGER, KEYWORD)); - assertEquals(LONG, commonType(TEXT, LONG)); + assertEquals(null, commonType(INTEGER, KEYWORD)); + assertEquals(null, commonType(TEXT, LONG)); assertEquals(SHORT, commonType(SHORT, BYTE)); assertEquals(FLOAT, commonType(BYTE, FLOAT)); assertEquals(FLOAT, commonType(FLOAT, INTEGER)); From 0b0ac507b40c56785d22a4bb17efe01891fde867 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Mon, 26 Aug 2024 19:12:46 -0400 Subject: [PATCH 4/7] clean up --- .../xpack/esql/type/EsqlDataTypeConverterTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java index eaa7d4825ef7c..ca43bd3754f13 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java @@ -35,8 +35,8 @@ public void testCommonType() { assertEquals(BOOLEAN, commonType(NULL, BOOLEAN)); assertEquals(BOOLEAN, commonType(BOOLEAN, BOOLEAN)); assertEquals(NULL, commonType(NULL, NULL)); - assertEquals(null, commonType(INTEGER, KEYWORD)); - assertEquals(null, commonType(TEXT, LONG)); + assertNull(commonType(INTEGER, KEYWORD)); + assertNull(commonType(TEXT, LONG)); assertEquals(SHORT, commonType(SHORT, BYTE)); assertEquals(FLOAT, commonType(BYTE, FLOAT)); assertEquals(FLOAT, commonType(FLOAT, INTEGER)); From 0b97e54ec350c52027342278b31ee94bfc43dd87 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 28 Aug 2024 10:42:08 -0400 Subject: [PATCH 5/7] clean up --- .../elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java index b2701e7d9c090..b090708a64ad3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java @@ -229,7 +229,6 @@ public static Object convert(Object value, DataType dataType) { *

* If one of the types is null - returns another type * If both types are numeric - returns type with the highest precision int < long < float < double - * If one of the types is string and another numeric - returns numeric */ public static DataType commonType(DataType left, DataType right) { if (left == right) { @@ -256,9 +255,6 @@ public static DataType commonType(DataType left, DataType right) { if (left == TEXT || right == TEXT) { return TEXT; } - if (left == KEYWORD) { - return KEYWORD; - } return right; } if (left.isNumeric() && right.isNumeric()) { From 13e46bb32a0a243131be5009c0eda9e702282a23 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Wed, 28 Aug 2024 19:30:25 -0400 Subject: [PATCH 6/7] more test --- .../esql/type/EsqlDataTypeConverterTests.java | 158 ++++++++++++++++-- 1 file changed, 142 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java index ca43bd3754f13..f1162be83691e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java @@ -8,18 +8,44 @@ package org.elasticsearch.xpack.esql.type; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.esql.core.type.DataType; + +import java.util.List; import static org.elasticsearch.xpack.esql.core.type.DataType.BOOLEAN; import static org.elasticsearch.xpack.esql.core.type.DataType.BYTE; +import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_POINT; +import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_SHAPE; +import static org.elasticsearch.xpack.esql.core.type.DataType.COUNTER_DOUBLE; +import static org.elasticsearch.xpack.esql.core.type.DataType.COUNTER_INTEGER; +import static org.elasticsearch.xpack.esql.core.type.DataType.COUNTER_LONG; +import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME; +import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_NANOS; +import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_PERIOD; +import static org.elasticsearch.xpack.esql.core.type.DataType.DOC_DATA_TYPE; import static org.elasticsearch.xpack.esql.core.type.DataType.DOUBLE; import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT; +import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT; +import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_SHAPE; +import static org.elasticsearch.xpack.esql.core.type.DataType.HALF_FLOAT; import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER; +import static org.elasticsearch.xpack.esql.core.type.DataType.IP; import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.NULL; +import static org.elasticsearch.xpack.esql.core.type.DataType.OBJECT; +import static org.elasticsearch.xpack.esql.core.type.DataType.PARTIAL_AGG; +import static org.elasticsearch.xpack.esql.core.type.DataType.SCALED_FLOAT; import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT; +import static org.elasticsearch.xpack.esql.core.type.DataType.SOURCE; import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; +import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION; +import static org.elasticsearch.xpack.esql.core.type.DataType.TSID_DATA_TYPE; import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; +import static org.elasticsearch.xpack.esql.core.type.DataType.UNSUPPORTED; +import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION; +import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime; +import static org.elasticsearch.xpack.esql.core.type.DataType.isString; import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType; public class EsqlDataTypeConverterTests extends ESTestCase { @@ -30,23 +56,123 @@ public void testNanoTimeToString() { assertEquals(expected, actual); } - public void testCommonType() { - assertEquals(BOOLEAN, commonType(BOOLEAN, NULL)); - assertEquals(BOOLEAN, commonType(NULL, BOOLEAN)); - assertEquals(BOOLEAN, commonType(BOOLEAN, BOOLEAN)); - assertEquals(NULL, commonType(NULL, NULL)); - assertNull(commonType(INTEGER, KEYWORD)); - assertNull(commonType(TEXT, LONG)); - assertEquals(SHORT, commonType(SHORT, BYTE)); - assertEquals(FLOAT, commonType(BYTE, FLOAT)); - assertEquals(FLOAT, commonType(FLOAT, INTEGER)); - assertEquals(UNSIGNED_LONG, commonType(UNSIGNED_LONG, LONG)); - assertEquals(DOUBLE, commonType(DOUBLE, FLOAT)); - assertEquals(FLOAT, commonType(FLOAT, UNSIGNED_LONG)); + public void testCommonTypeNull() { + for (DataType dataType : DataType.values()) { + assertEqualsCommonType(dataType, NULL, dataType); + } + } + + public void testCommonTypeStrings() { + List STRINGS = List.of(TEXT, KEYWORD); + for (DataType dataType1 : STRINGS) { + for (DataType dataType2 : DataType.values()) { + if (dataType2 == NULL) { + assertEqualsCommonType(dataType1, NULL, dataType1); + } else if ((isString(dataType1) && isString(dataType2))) { + if (dataType1 == dataType2) { + assertEqualsCommonType(dataType1, dataType2, dataType1); + } else { + assertEqualsCommonType(dataType1, dataType2, TEXT); + } + } else { + assertNullCommonType(dataType1, dataType2); + } + } + } + } - // strings - assertEquals(TEXT, commonType(TEXT, KEYWORD)); - assertEquals(TEXT, commonType(KEYWORD, TEXT)); + public void testCommonTypeDateTimeIntervals() { + List DATE_TIME_INTERVALS = List.of(DATETIME, DATE_PERIOD, TIME_DURATION); + for (DataType dataType1 : DATE_TIME_INTERVALS) { + for (DataType dataType2 : DataType.values()) { + if (dataType2 == NULL) { + assertEqualsCommonType(dataType1, NULL, dataType1); + } else if (DATE_TIME_INTERVALS.containsAll(List.of(dataType1, dataType2))) { + if (isDateTime(dataType1) || isDateTime(dataType2)) { + assertEqualsCommonType(dataType1, dataType2, DATETIME); + } else if (dataType1 == dataType2) { + assertEqualsCommonType(dataType1, dataType2, dataType1); + } else { + assertNullCommonType(dataType1, dataType2); + } + } else { + assertNullCommonType(dataType1, dataType2); + } + } + } } + public void testCommonTypeNumeric() { + // whole numbers + commonNumericType(BYTE, List.of(NULL, BYTE)); + commonNumericType(SHORT, List.of(NULL, BYTE, SHORT)); + commonNumericType(INTEGER, List.of(NULL, BYTE, SHORT, INTEGER)); + commonNumericType(LONG, List.of(NULL, BYTE, SHORT, INTEGER, LONG)); + commonNumericType(UNSIGNED_LONG, List.of(NULL, BYTE, SHORT, INTEGER, LONG, UNSIGNED_LONG)); + // floats + commonNumericType(HALF_FLOAT, List.of(NULL, BYTE, SHORT, INTEGER, LONG, UNSIGNED_LONG, HALF_FLOAT, FLOAT)); + commonNumericType(FLOAT, List.of(NULL, BYTE, SHORT, INTEGER, LONG, UNSIGNED_LONG, FLOAT, HALF_FLOAT)); + commonNumericType(DOUBLE, List.of(NULL, BYTE, SHORT, INTEGER, LONG, UNSIGNED_LONG, HALF_FLOAT, FLOAT, DOUBLE, SCALED_FLOAT)); + commonNumericType(SCALED_FLOAT, List.of(NULL, BYTE, SHORT, INTEGER, LONG, UNSIGNED_LONG, HALF_FLOAT, FLOAT, SCALED_FLOAT, DOUBLE)); + } + + /** + * The first argument and the second argument(s) have the first argument as a common type. + */ + private static void commonNumericType(DataType numericType, List lowerTypes) { + List NUMERICS = List.of(INTEGER, LONG, DOUBLE, UNSIGNED_LONG, BYTE, SHORT, FLOAT, HALF_FLOAT, SCALED_FLOAT); + List DOUBLES = List.of(DOUBLE, FLOAT, HALF_FLOAT, SCALED_FLOAT); + for (DataType dataType : DataType.values()) { + if (DOUBLES.containsAll(List.of(numericType, dataType)) && (dataType.estimatedSize().equals(numericType.estimatedSize()))) { + assertEquals(numericType, commonType(dataType, numericType)); + } else if (lowerTypes.contains(dataType)) { + assertEqualsCommonType(numericType, dataType, numericType); + } else if (NUMERICS.contains(dataType)) { + assertEqualsCommonType(numericType, dataType, dataType); + } else { + assertNullCommonType(numericType, dataType); + } + } + } + + public void testCommonTypeMiscellaneous() { + List MISCELLANEOUS = List.of( + COUNTER_INTEGER, + COUNTER_LONG, + COUNTER_DOUBLE, + UNSUPPORTED, + OBJECT, + SOURCE, + DATE_NANOS, + DOC_DATA_TYPE, + TSID_DATA_TYPE, + PARTIAL_AGG, + IP, + VERSION, + GEO_POINT, + GEO_SHAPE, + CARTESIAN_POINT, + CARTESIAN_SHAPE, + BOOLEAN + ); + for (DataType dataType1 : MISCELLANEOUS) { + for (DataType dataType2 : DataType.values()) { + if (dataType2 == NULL || dataType1 == dataType2) { + assertEqualsCommonType(dataType1, dataType2, dataType1); + } else { + assertNullCommonType(dataType1, dataType2); + } + } + } + } + + private static void assertEqualsCommonType(DataType dataType1, DataType dataType2, DataType commonType) { + assertEquals(commonType, commonType(dataType1, dataType2)); + assertEquals(commonType, commonType(dataType2, dataType1)); + } + + private static void assertNullCommonType(DataType dataType1, DataType dataType2) { + assertNull(commonType(dataType1, dataType2)); + assertNull(commonType(dataType2, dataType1)); + } } From 504323440b45151daf628a525eb04ab2e3d396c0 Mon Sep 17 00:00:00 2001 From: Fang Xing Date: Thu, 29 Aug 2024 11:00:34 -0400 Subject: [PATCH 7/7] clean up --- .../esql/type/EsqlDataTypeConverterTests.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java index f1162be83691e..8ad083683f696 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.esql.core.type.DataType; +import java.util.Arrays; import java.util.List; import static org.elasticsearch.xpack.esql.core.type.DataType.BOOLEAN; @@ -21,7 +22,6 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.COUNTER_LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.DATETIME; import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_NANOS; -import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_PERIOD; import static org.elasticsearch.xpack.esql.core.type.DataType.DOC_DATA_TYPE; import static org.elasticsearch.xpack.esql.core.type.DataType.DOUBLE; import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT; @@ -30,7 +30,6 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.HALF_FLOAT; import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER; import static org.elasticsearch.xpack.esql.core.type.DataType.IP; -import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD; import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.NULL; import static org.elasticsearch.xpack.esql.core.type.DataType.OBJECT; @@ -39,12 +38,12 @@ import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT; import static org.elasticsearch.xpack.esql.core.type.DataType.SOURCE; import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT; -import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION; import static org.elasticsearch.xpack.esql.core.type.DataType.TSID_DATA_TYPE; import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG; import static org.elasticsearch.xpack.esql.core.type.DataType.UNSUPPORTED; import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION; import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime; +import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTimeOrTemporal; import static org.elasticsearch.xpack.esql.core.type.DataType.isString; import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.commonType; @@ -63,7 +62,7 @@ public void testCommonTypeNull() { } public void testCommonTypeStrings() { - List STRINGS = List.of(TEXT, KEYWORD); + List STRINGS = Arrays.stream(DataType.values()).filter(DataType::isString).toList(); for (DataType dataType1 : STRINGS) { for (DataType dataType2 : DataType.values()) { if (dataType2 == NULL) { @@ -82,12 +81,12 @@ public void testCommonTypeStrings() { } public void testCommonTypeDateTimeIntervals() { - List DATE_TIME_INTERVALS = List.of(DATETIME, DATE_PERIOD, TIME_DURATION); + List DATE_TIME_INTERVALS = Arrays.stream(DataType.values()).filter(DataType::isDateTimeOrTemporal).toList(); for (DataType dataType1 : DATE_TIME_INTERVALS) { for (DataType dataType2 : DataType.values()) { if (dataType2 == NULL) { assertEqualsCommonType(dataType1, NULL, dataType1); - } else if (DATE_TIME_INTERVALS.containsAll(List.of(dataType1, dataType2))) { + } else if (isDateTimeOrTemporal(dataType2)) { if (isDateTime(dataType1) || isDateTime(dataType2)) { assertEqualsCommonType(dataType1, dataType2, DATETIME); } else if (dataType1 == dataType2) { @@ -120,8 +119,8 @@ public void testCommonTypeNumeric() { * The first argument and the second argument(s) have the first argument as a common type. */ private static void commonNumericType(DataType numericType, List lowerTypes) { - List NUMERICS = List.of(INTEGER, LONG, DOUBLE, UNSIGNED_LONG, BYTE, SHORT, FLOAT, HALF_FLOAT, SCALED_FLOAT); - List DOUBLES = List.of(DOUBLE, FLOAT, HALF_FLOAT, SCALED_FLOAT); + List NUMERICS = Arrays.stream(DataType.values()).filter(DataType::isNumeric).toList(); + List DOUBLES = Arrays.stream(DataType.values()).filter(DataType::isRationalNumber).toList(); for (DataType dataType : DataType.values()) { if (DOUBLES.containsAll(List.of(numericType, dataType)) && (dataType.estimatedSize().equals(numericType.estimatedSize()))) { assertEquals(numericType, commonType(dataType, numericType));