From 73a17a17fe1cc80a6f436c207e3a8cc45bff9b60 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Tue, 16 Apr 2024 14:25:21 -0400 Subject: [PATCH] [ESQL] Moving argument compatibility checking for Equals (#107546) Fixed the conflicts, and re-submitting. Please see #105217 for full details, history, and discussion. I'll use the commit message from that PR as well. Continuing my work from #104490, this PR moves the parameter compatibility checking for Equals into the type resolution check. This is a somewhat bigger change than for Add, as there was no ES|QL base class for binary comparison operators before this. I've added EsqlBinaryComparison as that base class, and migrated all of the binary comparisons to be based off of that (except for NullEquals, see note below). In order to maintain compatibility with the current behavior, I've kept it so that unsigned longs are only inter-operable with other unsigned longs. We've talked a lot about changing that, and I consider this work a prerequisite for that. I've also added a bunch of test cases to Equals and NotEquals, which should have the side effect of filling out the type support table in the equals docs. As noted in the comments, I'll have follow up PRs for the other binary comparisons to add tests, but this PR is already too long. Note about NullEquals: There is an ES|QL NullEquals class, which inherits from the QL version, but I don't think it works. I didn't see any tests or docs for it, and trying it out in the demo instance gave me a syntax error. I think we need to delve into what's going on there, but this PR isn't the right place for it. This reverts commit 225edaf6076770385b4d091af89a546020ec5c79. --- docs/changelog/107537.yaml | 5 - .../src/main/resources/conditional.csv-spec | 7 + .../predicate/operator/comparison/Equals.java | 47 +++-- .../comparison/EsqlBinaryComparison.java | 164 +++++++++++++++ .../operator/comparison/GreaterThan.java | 33 ++- .../comparison/GreaterThanOrEqual.java | 34 ++- .../operator/comparison/LessThan.java | 36 ++-- .../operator/comparison/LessThanOrEqual.java | 31 ++- .../operator/comparison/NotEquals.java | 82 +++++--- .../DateTimeArithmeticOperation.java | 8 +- .../arithmetic/EsqlArithmeticOperation.java | 23 +- .../function/AbstractFunctionTestCase.java | 49 +++++ .../expression/function/TestCaseSupplier.java | 92 +++++++- .../operator/arithmetic/AddTests.java | 8 +- .../operator/arithmetic/DivTests.java | 18 +- .../operator/arithmetic/ModTests.java | 18 +- .../operator/comparison/EqualsTests.java | 198 +++++++++++++++--- .../comparison/GreaterThanOrEqualTests.java | 21 +- .../operator/comparison/GreaterThanTests.java | 21 +- .../comparison/LessThanOrEqualTests.java | 20 +- .../operator/comparison/LessThanTests.java | 20 +- .../operator/comparison/NotEqualsTests.java | 197 ++++++++++++++--- .../esql/optimizer/OptimizerRulesTests.java | 2 +- 23 files changed, 872 insertions(+), 262 deletions(-) delete mode 100644 docs/changelog/107537.yaml create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/EsqlBinaryComparison.java diff --git a/docs/changelog/107537.yaml b/docs/changelog/107537.yaml deleted file mode 100644 index d6d502b394c3b..0000000000000 --- a/docs/changelog/107537.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 107537 -summary: "Revert \"[ES|QL] Moving argument compatibility checking for Equals\"" -area: ES|QL -type: bug -issues: [] diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/conditional.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/conditional.csv-spec index f574722f691e5..64a8c1d9da316 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/conditional.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/conditional.csv-spec @@ -156,6 +156,9 @@ nullOnMultivaluesComparisonOperation required_feature: esql.disable_nullable_opts ROW a = 5, b = [ 1, 2 ]| EVAL same = a == b| LIMIT 1 | WHERE same IS NULL; +warning:Line 1:38: evaluation of [a == b] failed, treating result as null. Only first 20 failures recorded. +warning:Line 1:38: java.lang.IllegalArgumentException: single-value function encountered multi-value + a:integer | b:integer | same:boolean 5 | [1, 2] | null @@ -166,6 +169,8 @@ notNullOnMultivaluesComparisonOperation required_feature: esql.disable_nullable_opts ROW a = 5, b = [ 1, 2 ]| EVAL same = a == b| LIMIT 1 | WHERE same IS NOT NULL; +warning:Line 1:38: evaluation of [a == b] failed, treating result as null. Only first 20 failures recorded. +warning:Line 1:38: java.lang.IllegalArgumentException: single-value function encountered multi-value a:integer | b:integer | same:boolean ; @@ -175,6 +180,8 @@ notNullOnMultivaluesComparisonOperationWithPartialMatch required_feature: esql.disable_nullable_opts ROW a = 5, b = [ 5, 2 ]| EVAL same = a == b| LIMIT 1 | WHERE same IS NOT NULL; +warning:Line 1:38: evaluation of [a == b] failed, treating result as null. Only first 20 failures recorded. +warning:Line 1:38: java.lang.IllegalArgumentException: single-value function encountered multi-value a:integer | b:integer | same:boolean ; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/Equals.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/Equals.java index 9fb899b8e36df..62eec13af008a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/Equals.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/Equals.java @@ -8,33 +8,48 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.compute.ann.Evaluator; -import org.elasticsearch.xpack.esql.expression.EsqlTypeResolutions; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; +import org.elasticsearch.xpack.esql.type.EsqlDataTypes; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.TypeResolutions; +import org.elasticsearch.xpack.ql.expression.predicate.Negatable; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparisonProcessor; import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; import java.time.ZoneId; +import java.util.Map; + +public class Equals extends EsqlBinaryComparison implements Negatable { + private static final Map evaluatorMap = Map.ofEntries( + Map.entry(DataTypes.BOOLEAN, EqualsBoolsEvaluator.Factory::new), + Map.entry(DataTypes.INTEGER, EqualsIntsEvaluator.Factory::new), + Map.entry(DataTypes.DOUBLE, EqualsDoublesEvaluator.Factory::new), + Map.entry(DataTypes.LONG, EqualsLongsEvaluator.Factory::new), + Map.entry(DataTypes.UNSIGNED_LONG, EqualsLongsEvaluator.Factory::new), + Map.entry(DataTypes.DATETIME, EqualsLongsEvaluator.Factory::new), + Map.entry(EsqlDataTypes.GEO_POINT, EqualsGeometriesEvaluator.Factory::new), + Map.entry(EsqlDataTypes.CARTESIAN_POINT, EqualsGeometriesEvaluator.Factory::new), + Map.entry(EsqlDataTypes.GEO_SHAPE, EqualsGeometriesEvaluator.Factory::new), + Map.entry(EsqlDataTypes.CARTESIAN_SHAPE, EqualsGeometriesEvaluator.Factory::new), + Map.entry(DataTypes.KEYWORD, EqualsKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.TEXT, EqualsKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.VERSION, EqualsKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.IP, EqualsKeywordsEvaluator.Factory::new) + ); -import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT; - -public class Equals extends org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals { public Equals(Source source, Expression left, Expression right) { - super(source, left, right); + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.EQ, evaluatorMap); } public Equals(Source source, Expression left, Expression right, ZoneId zoneId) { - super(source, left, right, zoneId); - } - - @Override - protected TypeResolution resolveInputType(Expression e, TypeResolutions.ParamOrdinal paramOrdinal) { - return EsqlTypeResolutions.isExact(e, sourceText(), DEFAULT); + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.EQ, zoneId, evaluatorMap); } @Override - protected NodeInfo info() { + protected NodeInfo info() { return NodeInfo.create(this, Equals::new, left(), right(), zoneId()); } @@ -48,6 +63,11 @@ public Equals swapLeftAndRight() { return new Equals(source(), right(), left(), zoneId()); } + @Override + public BinaryComparison reverse() { + return this; + } + @Override public BinaryComparison negate() { return new NotEquals(source(), left(), right(), zoneId()); @@ -82,4 +102,5 @@ static boolean processBools(boolean lhs, boolean rhs) { static boolean processGeometries(BytesRef lhs, BytesRef rhs) { return lhs.equals(rhs); } + } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/EsqlBinaryComparison.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/EsqlBinaryComparison.java new file mode 100644 index 0000000000000..58a808893c4c6 --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/EsqlBinaryComparison.java @@ -0,0 +1,164 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison; + +import org.elasticsearch.compute.operator.EvalOperator; +import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; +import org.elasticsearch.xpack.esql.evaluator.mapper.EvaluatorMapper; +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.type.EsqlDataTypeRegistry; +import org.elasticsearch.xpack.ql.expression.Expression; +import org.elasticsearch.xpack.ql.expression.TypeResolutions; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparisonProcessor; +import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; + +import java.time.ZoneId; +import java.util.Map; +import java.util.function.Function; + +import static org.elasticsearch.common.logging.LoggerMessageFormat.format; +import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG; + +public abstract class EsqlBinaryComparison extends BinaryComparison implements EvaluatorMapper { + + private final Map evaluatorMap; + + protected EsqlBinaryComparison( + Source source, + Expression left, + Expression right, + /* TODO: BinaryComparisonOperator is an enum with a bunch of functionality we don't really want. We should extract an interface and + create a symbol only version like we did for BinaryArithmeticOperation. Ideally, they could be the same class. + */ + BinaryComparisonProcessor.BinaryComparisonOperation operation, + Map evaluatorMap + ) { + this(source, left, right, operation, null, evaluatorMap); + } + + protected EsqlBinaryComparison( + Source source, + Expression left, + Expression right, + BinaryComparisonProcessor.BinaryComparisonOperation operation, + // TODO: We are definitely not doing the right thing with this zoneId + ZoneId zoneId, + Map evaluatorMap + ) { + super(source, left, right, operation, zoneId); + this.evaluatorMap = evaluatorMap; + } + + @Override + 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()); + EvalOperator.ExpressionEvaluator.Factory lhs; + EvalOperator.ExpressionEvaluator.Factory rhs; + + if (commonType.isNumeric()) { + lhs = Cast.cast(source(), left().dataType(), commonType, toEvaluator.apply(left())); + rhs = Cast.cast(source(), right().dataType(), commonType, toEvaluator.apply(right())); + } else { + lhs = toEvaluator.apply(left()); + rhs = toEvaluator.apply(right()); + } + + if (evaluatorMap.containsKey(commonType) == false) { + throw new EsqlIllegalArgumentException("Unsupported type " + left().dataType()); + } + return evaluatorMap.get(commonType).apply(source(), lhs, rhs); + } + + @Override + public Boolean fold() { + return (Boolean) EvaluatorMapper.super.fold(); + } + + @Override + protected TypeResolution resolveType() { + TypeResolution typeResolution = super.resolveType(); + if (typeResolution.unresolved()) { + return typeResolution; + } + + return checkCompatibility(); + } + + @Override + protected TypeResolution resolveInputType(Expression e, TypeResolutions.ParamOrdinal paramOrdinal) { + return TypeResolutions.isType( + e, + evaluatorMap::containsKey, + sourceText(), + paramOrdinal, + evaluatorMap.keySet().stream().map(DataType::typeName).toArray(String[]::new) + ); + } + + /** + * Check if the two input types are compatible for this operation + * + * @return TypeResolution.TYPE_RESOLVED iff the types are compatible. Otherwise, an appropriate type resolution error. + */ + protected TypeResolution checkCompatibility() { + DataType leftType = left().dataType(); + DataType rightType = right().dataType(); + + // Unsigned long is only interoperable with other unsigned longs + if ((rightType == UNSIGNED_LONG && (false == (leftType == UNSIGNED_LONG || leftType == DataTypes.NULL))) + || (leftType == UNSIGNED_LONG && (false == (rightType == UNSIGNED_LONG || rightType == DataTypes.NULL)))) { + return new TypeResolution(formatIncompatibleTypesMessage()); + } + + if ((leftType.isNumeric() && rightType.isNumeric()) + || (DataTypes.isString(leftType) && DataTypes.isString(rightType)) + || leftType.equals(rightType) + || DataTypes.isNull(leftType) + || DataTypes.isNull(rightType)) { + return TypeResolution.TYPE_RESOLVED; + } + return new TypeResolution(formatIncompatibleTypesMessage()); + } + + public String formatIncompatibleTypesMessage() { + if (left().dataType().equals(UNSIGNED_LONG)) { + return format( + null, + "first argument of [{}] is [unsigned_long] and second is [{}]. " + + "[unsigned_long] can only be operated on together with another [unsigned_long]", + sourceText(), + right().dataType().typeName() + ); + } + if (right().dataType().equals(UNSIGNED_LONG)) { + return format( + null, + "first argument of [{}] is [{}] and second is [unsigned_long]. " + + "[unsigned_long] can only be operated on together with another [unsigned_long]", + sourceText(), + left().dataType().typeName() + ); + } + return format( + null, + "first argument of [{}] is [{}] so second argument must also be [{}] but was [{}]", + sourceText(), + left().dataType().isNumeric() ? "numeric" : left().dataType().typeName(), + left().dataType().isNumeric() ? "numeric" : left().dataType().typeName(), + right().dataType().typeName() + ); + } + +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThan.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThan.java index 5683a9d0d7e85..3eca0e858acbf 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThan.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThan.java @@ -8,29 +8,42 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.compute.ann.Evaluator; -import org.elasticsearch.xpack.esql.expression.EsqlTypeResolutions; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.TypeResolutions; +import org.elasticsearch.xpack.ql.expression.predicate.Negatable; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparisonProcessor; import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; import java.time.ZoneId; +import java.util.Map; -import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT; +public class GreaterThan extends EsqlBinaryComparison implements Negatable { + private static final Map evaluatorMap = Map.ofEntries( + Map.entry(DataTypes.INTEGER, GreaterThanIntsEvaluator.Factory::new), + Map.entry(DataTypes.DOUBLE, GreaterThanDoublesEvaluator.Factory::new), + Map.entry(DataTypes.LONG, GreaterThanLongsEvaluator.Factory::new), + Map.entry(DataTypes.UNSIGNED_LONG, GreaterThanLongsEvaluator.Factory::new), + Map.entry(DataTypes.DATETIME, GreaterThanLongsEvaluator.Factory::new), + Map.entry(DataTypes.KEYWORD, GreaterThanKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.TEXT, GreaterThanKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.VERSION, GreaterThanKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.IP, GreaterThanKeywordsEvaluator.Factory::new) + ); -public class GreaterThan extends org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThan { - public GreaterThan(Source source, Expression left, Expression right, ZoneId zoneId) { - super(source, left, right, zoneId); + public GreaterThan(Source source, Expression left, Expression right) { + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.GT, evaluatorMap); } - @Override - protected TypeResolution resolveInputType(Expression e, TypeResolutions.ParamOrdinal paramOrdinal) { - return EsqlTypeResolutions.isExact(e, sourceText(), DEFAULT); + public GreaterThan(Source source, Expression left, Expression right, ZoneId zoneId) { + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.GT, zoneId, evaluatorMap); } @Override - protected NodeInfo info() { + protected NodeInfo info() { return NodeInfo.create(this, GreaterThan::new, left(), right(), zoneId()); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThanOrEqual.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThanOrEqual.java index ebb29998fb995..f99a85420870b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThanOrEqual.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/GreaterThanOrEqual.java @@ -8,30 +8,42 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.compute.ann.Evaluator; -import org.elasticsearch.xpack.esql.expression.EsqlTypeResolutions; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.TypeResolutions; +import org.elasticsearch.xpack.ql.expression.predicate.Negatable; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparisonProcessor; import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; import java.time.ZoneId; +import java.util.Map; -import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT; +public class GreaterThanOrEqual extends EsqlBinaryComparison implements Negatable { + private static final Map evaluatorMap = Map.ofEntries( + Map.entry(DataTypes.INTEGER, GreaterThanOrEqualIntsEvaluator.Factory::new), + Map.entry(DataTypes.DOUBLE, GreaterThanOrEqualDoublesEvaluator.Factory::new), + Map.entry(DataTypes.LONG, GreaterThanOrEqualLongsEvaluator.Factory::new), + Map.entry(DataTypes.UNSIGNED_LONG, GreaterThanOrEqualLongsEvaluator.Factory::new), + Map.entry(DataTypes.DATETIME, GreaterThanOrEqualLongsEvaluator.Factory::new), + Map.entry(DataTypes.KEYWORD, GreaterThanOrEqualKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.TEXT, GreaterThanOrEqualKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.VERSION, GreaterThanOrEqualKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.IP, GreaterThanOrEqualKeywordsEvaluator.Factory::new) + ); -public class GreaterThanOrEqual extends org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThanOrEqual { - - public GreaterThanOrEqual(Source source, Expression left, Expression right, ZoneId zoneId) { - super(source, left, right, zoneId); + public GreaterThanOrEqual(Source source, Expression left, Expression right) { + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.GTE, evaluatorMap); } - @Override - protected TypeResolution resolveInputType(Expression e, TypeResolutions.ParamOrdinal paramOrdinal) { - return EsqlTypeResolutions.isExact(e, sourceText(), DEFAULT); + public GreaterThanOrEqual(Source source, Expression left, Expression right, ZoneId zoneId) { + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.GTE, zoneId, evaluatorMap); } @Override - protected NodeInfo info() { + protected NodeInfo info() { return NodeInfo.create(this, GreaterThanOrEqual::new, left(), right(), zoneId()); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThan.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThan.java index 12f54270b65dc..6b82df1d67da6 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThan.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThan.java @@ -8,38 +8,44 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.compute.ann.Evaluator; -import org.elasticsearch.xpack.esql.expression.EsqlTypeResolutions; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.TypeResolutions; +import org.elasticsearch.xpack.ql.expression.predicate.Negatable; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparisonProcessor; import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; import java.time.ZoneId; +import java.util.Map; -import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT; +public class LessThan extends EsqlBinaryComparison implements Negatable { -public class LessThan extends org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThan { + private static final Map evaluatorMap = Map.ofEntries( + Map.entry(DataTypes.INTEGER, LessThanIntsEvaluator.Factory::new), + Map.entry(DataTypes.DOUBLE, LessThanDoublesEvaluator.Factory::new), + Map.entry(DataTypes.LONG, LessThanLongsEvaluator.Factory::new), + Map.entry(DataTypes.UNSIGNED_LONG, LessThanLongsEvaluator.Factory::new), + Map.entry(DataTypes.DATETIME, LessThanLongsEvaluator.Factory::new), + Map.entry(DataTypes.KEYWORD, LessThanKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.TEXT, LessThanKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.VERSION, LessThanKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.IP, LessThanKeywordsEvaluator.Factory::new) + ); public LessThan(Source source, Expression left, Expression right, ZoneId zoneId) { - super(source, left, right, zoneId); + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.LT, zoneId, evaluatorMap); } @Override - protected TypeResolution resolveInputType(Expression e, TypeResolutions.ParamOrdinal paramOrdinal) { - return EsqlTypeResolutions.isExact(e, sourceText(), DEFAULT); - } - - @Override - protected NodeInfo info() { + protected NodeInfo info() { return NodeInfo.create(this, LessThan::new, left(), right(), zoneId()); } @Override - protected org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThan replaceChildren( - Expression newLeft, - Expression newRight - ) { + protected LessThan replaceChildren(Expression newLeft, Expression newRight) { return new LessThan(source(), newLeft, newRight, zoneId()); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThanOrEqual.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThanOrEqual.java index e75733a9e2340..ac6a92aaf097b 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThanOrEqual.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/LessThanOrEqual.java @@ -8,29 +8,38 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.compute.ann.Evaluator; -import org.elasticsearch.xpack.esql.expression.EsqlTypeResolutions; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.TypeResolutions; +import org.elasticsearch.xpack.ql.expression.predicate.Negatable; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparisonProcessor; import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; import java.time.ZoneId; +import java.util.Map; -import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT; +public class LessThanOrEqual extends EsqlBinaryComparison implements Negatable { + private static final Map evaluatorMap = Map.ofEntries( + Map.entry(DataTypes.INTEGER, LessThanOrEqualIntsEvaluator.Factory::new), + Map.entry(DataTypes.DOUBLE, LessThanOrEqualDoublesEvaluator.Factory::new), + Map.entry(DataTypes.LONG, LessThanOrEqualLongsEvaluator.Factory::new), + Map.entry(DataTypes.UNSIGNED_LONG, LessThanOrEqualLongsEvaluator.Factory::new), + Map.entry(DataTypes.DATETIME, LessThanOrEqualLongsEvaluator.Factory::new), + Map.entry(DataTypes.KEYWORD, LessThanOrEqualKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.TEXT, LessThanOrEqualKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.VERSION, LessThanOrEqualKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.IP, LessThanOrEqualKeywordsEvaluator.Factory::new) + ); -public class LessThanOrEqual extends org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThanOrEqual { public LessThanOrEqual(Source source, Expression left, Expression right, ZoneId zoneId) { - super(source, left, right, zoneId); + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.LTE, zoneId, evaluatorMap); } @Override - protected TypeResolution resolveInputType(Expression e, TypeResolutions.ParamOrdinal paramOrdinal) { - return EsqlTypeResolutions.isExact(e, sourceText(), DEFAULT); - } - - @Override - protected NodeInfo info() { + protected NodeInfo info() { return NodeInfo.create(this, LessThanOrEqual::new, left(), right(), zoneId()); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/NotEquals.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/NotEquals.java index 6fbed572cdc01..9c931ec7433eb 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/NotEquals.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/NotEquals.java @@ -8,45 +8,44 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.compute.ann.Evaluator; -import org.elasticsearch.xpack.esql.expression.EsqlTypeResolutions; +import org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.EsqlArithmeticOperation; +import org.elasticsearch.xpack.esql.type.EsqlDataTypes; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.TypeResolutions; +import org.elasticsearch.xpack.ql.expression.predicate.Negatable; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparisonProcessor; import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; import java.time.ZoneId; +import java.util.Map; -import static org.elasticsearch.xpack.ql.expression.TypeResolutions.ParamOrdinal.DEFAULT; +public class NotEquals extends EsqlBinaryComparison implements Negatable { + private static final Map evaluatorMap = Map.ofEntries( + Map.entry(DataTypes.BOOLEAN, NotEqualsBoolsEvaluator.Factory::new), + Map.entry(DataTypes.INTEGER, NotEqualsIntsEvaluator.Factory::new), + Map.entry(DataTypes.DOUBLE, NotEqualsDoublesEvaluator.Factory::new), + Map.entry(DataTypes.LONG, NotEqualsLongsEvaluator.Factory::new), + Map.entry(DataTypes.UNSIGNED_LONG, NotEqualsLongsEvaluator.Factory::new), + Map.entry(DataTypes.DATETIME, NotEqualsLongsEvaluator.Factory::new), + Map.entry(EsqlDataTypes.GEO_POINT, NotEqualsGeometriesEvaluator.Factory::new), + Map.entry(EsqlDataTypes.CARTESIAN_POINT, NotEqualsGeometriesEvaluator.Factory::new), + Map.entry(EsqlDataTypes.GEO_SHAPE, NotEqualsGeometriesEvaluator.Factory::new), + Map.entry(EsqlDataTypes.CARTESIAN_SHAPE, NotEqualsGeometriesEvaluator.Factory::new), + Map.entry(DataTypes.KEYWORD, NotEqualsKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.TEXT, NotEqualsKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.VERSION, NotEqualsKeywordsEvaluator.Factory::new), + Map.entry(DataTypes.IP, NotEqualsKeywordsEvaluator.Factory::new) + ); -public class NotEquals extends org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.NotEquals { - public NotEquals(Source source, Expression left, Expression right, ZoneId zoneId) { - super(source, left, right, zoneId); - } - - @Override - protected TypeResolution resolveInputType(Expression e, TypeResolutions.ParamOrdinal paramOrdinal) { - return EsqlTypeResolutions.isExact(e, sourceText(), DEFAULT); - } - - @Override - protected NodeInfo info() { - return NodeInfo.create(this, NotEquals::new, left(), right(), zoneId()); - } - - @Override - protected NotEquals replaceChildren(Expression newLeft, Expression newRight) { - return new NotEquals(source(), newLeft, newRight, zoneId()); + public NotEquals(Source source, Expression left, Expression right) { + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.NEQ, evaluatorMap); } - @Override - public NotEquals swapLeftAndRight() { - return new NotEquals(source(), right(), left(), zoneId()); - } - - @Override - public BinaryComparison negate() { - return new Equals(source(), left(), right(), zoneId()); + public NotEquals(Source source, Expression left, Expression right, ZoneId zoneId) { + super(source, left, right, BinaryComparisonProcessor.BinaryComparisonOperation.NEQ, zoneId, evaluatorMap); } @Evaluator(extraName = "Ints") @@ -78,4 +77,29 @@ static boolean processBools(boolean lhs, boolean rhs) { static boolean processGeometries(BytesRef lhs, BytesRef rhs) { return false == lhs.equals(rhs); } + + @Override + public BinaryComparison reverse() { + return this; + } + + @Override + protected NodeInfo info() { + return NodeInfo.create(this, NotEquals::new, left(), right(), zoneId()); + } + + @Override + protected NotEquals replaceChildren(Expression newLeft, Expression newRight) { + return new NotEquals(source(), newLeft, newRight, zoneId()); + } + + @Override + public NotEquals swapLeftAndRight() { + return new NotEquals(source(), right(), left(), zoneId()); + } + + @Override + public BinaryComparison negate() { + return new Equals(source(), left(), right(), zoneId()); + } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DateTimeArithmeticOperation.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DateTimeArithmeticOperation.java index 0f550862ed1fa..a45707a0197d5 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DateTimeArithmeticOperation.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DateTimeArithmeticOperation.java @@ -43,10 +43,10 @@ interface DatetimeArithmeticEvaluator { Expression left, Expression right, OperationSymbol op, - ArithmeticEvaluator ints, - ArithmeticEvaluator longs, - ArithmeticEvaluator ulongs, - ArithmeticEvaluator doubles, + BinaryEvaluator ints, + BinaryEvaluator longs, + BinaryEvaluator ulongs, + BinaryEvaluator doubles, DatetimeArithmeticEvaluator datetimes ) { super(source, left, right, op, ints, longs, ulongs, doubles); 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 22f5798e5b1c4..ba283bc4d877b 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 @@ -71,14 +71,15 @@ public String symbol() { } /** Arithmetic (quad) function. */ - interface ArithmeticEvaluator { + @FunctionalInterface + public interface BinaryEvaluator { ExpressionEvaluator.Factory apply(Source source, ExpressionEvaluator.Factory lhs, ExpressionEvaluator.Factory rhs); } - private final ArithmeticEvaluator ints; - private final ArithmeticEvaluator longs; - private final ArithmeticEvaluator ulongs; - private final ArithmeticEvaluator doubles; + private final BinaryEvaluator ints; + private final BinaryEvaluator longs; + private final BinaryEvaluator ulongs; + private final BinaryEvaluator doubles; private DataType dataType; @@ -87,10 +88,10 @@ interface ArithmeticEvaluator { Expression left, Expression right, OperationSymbol op, - ArithmeticEvaluator ints, - ArithmeticEvaluator longs, - ArithmeticEvaluator ulongs, - ArithmeticEvaluator doubles + BinaryEvaluator ints, + BinaryEvaluator longs, + BinaryEvaluator ulongs, + BinaryEvaluator doubles ) { super(source, left, right, op); this.ints = ints; @@ -139,7 +140,7 @@ protected TypeResolution checkCompatibility() { return TypeResolution.TYPE_RESOLVED; } - static String formatIncompatibleTypesMessage(String symbol, DataType leftType, DataType rightType) { + public static String formatIncompatibleTypesMessage(String symbol, DataType leftType, DataType rightType) { return format(null, "[{}] has arguments with incompatible types [{}] and [{}]", symbol, leftType.typeName(), rightType.typeName()); } @@ -152,7 +153,7 @@ public ExpressionEvaluator.Factory toEvaluator(Function errorsForCasesWithoutExamples( return suppliers; } + public static String errorMessageStringForBinaryOperators( + boolean includeOrdinal, + List> validPerPosition, + List types + ) { + try { + return typeErrorMessage(includeOrdinal, validPerPosition, types); + } catch (IllegalStateException e) { + // This means all the positional args were okay, so the expected error is from the combination + if (types.get(0).equals(DataTypes.UNSIGNED_LONG)) { + return "first argument of [] is [unsigned_long] and second is [" + + types.get(1).typeName() + + "]. [unsigned_long] can only be operated on together with another [unsigned_long]"; + + } + if (types.get(1).equals(DataTypes.UNSIGNED_LONG)) { + return "first argument of [] is [" + + types.get(0).typeName() + + "] and second is [unsigned_long]. [unsigned_long] can only be operated on together with another [unsigned_long]"; + } + return "first argument of [] is [" + + (types.get(0).isNumeric() ? "numeric" : types.get(0).typeName()) + + "] so second argument must also be [" + + (types.get(0).isNumeric() ? "numeric" : types.get(0).typeName()) + + "] but was [" + + types.get(1).typeName() + + "]"; + + } + } + /** * Adds test cases containing unsupported parameter types that immediately fail. */ @@ -931,6 +962,24 @@ protected static String typeErrorMessage(boolean includeOrdinal, List types) { return types.stream().map(t -> "<" + t.typeName() + ">").collect(Collectors.joining(", ")); } + public static List stringCases( + BinaryOperator expected, + BiFunction evaluatorToString, + List warnings, + DataType expectedType + ) { + List lhsSuppliers = new ArrayList<>(); + List rhsSuppliers = new ArrayList<>(); + List suppliers = new ArrayList<>(); + for (DataType type : AbstractConvertFunction.STRING_TYPES) { + lhsSuppliers.addAll(stringCases(type)); + rhsSuppliers.addAll(stringCases(type)); + casesCrossProduct( + expected, + lhsSuppliers, + rhsSuppliers, + evaluatorToString, + (lhs, rhs) -> warnings, + suppliers, + expectedType, + true + ); + } + return suppliers; + } + @Override public TestCase get() { TestCase supplied = supplier.get(); @@ -258,14 +284,14 @@ public static List castToDoubleSuppliersFromRange(Double Min, return suppliers; } - public record NumericTypeTestConfig(Number min, Number max, BinaryOperator expected, String evaluatorName) {} + public record NumericTypeTestConfig(Number min, Number max, BiFunction expected, String evaluatorName) {} - public record NumericTypeTestConfigs( - NumericTypeTestConfig intStuff, - NumericTypeTestConfig longStuff, - NumericTypeTestConfig doubleStuff + public record NumericTypeTestConfigs( + NumericTypeTestConfig intStuff, + NumericTypeTestConfig longStuff, + NumericTypeTestConfig doubleStuff ) { - public NumericTypeTestConfig get(DataType type) { + public NumericTypeTestConfig get(DataType type) { if (type == DataTypes.INTEGER) { return intStuff; } @@ -312,8 +338,47 @@ public static List getSuppliersForNumericType(DataType type, throw new IllegalArgumentException("bogus numeric type [" + type + "]"); } + public static List forBinaryComparisonWithWidening( + NumericTypeTestConfigs typeStuff, + String lhsName, + String rhsName, + BiFunction> warnings, + boolean allowRhsZero + ) { + List suppliers = new ArrayList<>(); + List numericTypes = List.of(DataTypes.INTEGER, DataTypes.LONG, DataTypes.DOUBLE); + + for (DataType lhsType : numericTypes) { + for (DataType rhsType : numericTypes) { + DataType expected = widen(lhsType, rhsType); + NumericTypeTestConfig expectedTypeStuff = typeStuff.get(expected); + BiFunction evaluatorToString = (lhs, rhs) -> expectedTypeStuff.evaluatorName() + + "[" + + lhsName + + "=" + + getCastEvaluator("Attribute[channel=0]", lhs, expected) + + ", " + + rhsName + + "=" + + getCastEvaluator("Attribute[channel=1]", rhs, expected) + + "]"; + casesCrossProduct( + (l, r) -> expectedTypeStuff.expected().apply((Number) l, (Number) r), + getSuppliersForNumericType(lhsType, expectedTypeStuff.min(), expectedTypeStuff.max(), allowRhsZero), + getSuppliersForNumericType(rhsType, expectedTypeStuff.min(), expectedTypeStuff.max(), allowRhsZero), + evaluatorToString, + warnings, + suppliers, + DataTypes.BOOLEAN, + true + ); + } + } + return suppliers; + } + public static List forBinaryWithWidening( - NumericTypeTestConfigs typeStuff, + NumericTypeTestConfigs typeStuff, String lhsName, String rhsName, BiFunction> warnings, @@ -325,7 +390,7 @@ public static List forBinaryWithWidening( for (DataType lhsType : numericTypes) { for (DataType rhsType : numericTypes) { DataType expected = widen(lhsType, rhsType); - NumericTypeTestConfig expectedTypeStuff = typeStuff.get(expected); + NumericTypeTestConfig expectedTypeStuff = typeStuff.get(expected); BiFunction evaluatorToString = (lhs, rhs) -> expectedTypeStuff.evaluatorName() + "[" + lhsName @@ -885,7 +950,7 @@ public static List doubleCases(double min, double max, boolea return cases; } - private static List booleanCases() { + public static List booleanCases() { return List.of( new TypedDataSupplier("", () -> true, DataTypes.BOOLEAN), new TypedDataSupplier("", () -> false, DataTypes.BOOLEAN) @@ -1267,9 +1332,14 @@ public Matcher evaluatorToString() { * exists because we can't generate random values from the test parameter generation functions, and instead need to return * suppliers which generate the random values at test execution time. */ - public record TypedDataSupplier(String name, Supplier supplier, DataType type) { + public record TypedDataSupplier(String name, Supplier supplier, DataType type, boolean forceLiteral) { + + public TypedDataSupplier(String name, Supplier supplier, DataType type) { + this(name, supplier, type, false); + } + public TypedData get() { - return new TypedData(supplier.get(), type, name); + return new TypedData(supplier.get(), type, name, forceLiteral); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/AddTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/AddTests.java index c40d037890d53..2596959c449db 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/AddTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/AddTests.java @@ -43,20 +43,20 @@ public static Iterable parameters() { List suppliers = new ArrayList<>(); suppliers.addAll( TestCaseSupplier.forBinaryWithWidening( - new TestCaseSupplier.NumericTypeTestConfigs( - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfigs( + new TestCaseSupplier.NumericTypeTestConfig<>( (Integer.MIN_VALUE >> 1) - 1, (Integer.MAX_VALUE >> 1) - 1, (l, r) -> l.intValue() + r.intValue(), "AddIntsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( (Long.MIN_VALUE >> 1) - 1, (Long.MAX_VALUE >> 1) - 1, (l, r) -> l.longValue() + r.longValue(), "AddLongsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, (l, r) -> l.doubleValue() + r.doubleValue(), diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DivTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DivTests.java index 528324f07a086..f3348ab2dcba5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DivTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/DivTests.java @@ -34,20 +34,20 @@ public static Iterable parameters() { List suppliers = new ArrayList<>(); suppliers.addAll( TestCaseSupplier.forBinaryWithWidening( - new TestCaseSupplier.NumericTypeTestConfigs( - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfigs( + new TestCaseSupplier.NumericTypeTestConfig<>( (Integer.MIN_VALUE >> 1) - 1, (Integer.MAX_VALUE >> 1) - 1, (l, r) -> l.intValue() / r.intValue(), "DivIntsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( (Long.MIN_VALUE >> 1) - 1, (Long.MAX_VALUE >> 1) - 1, (l, r) -> l.longValue() / r.longValue(), "DivLongsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig(Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, (l, r) -> { + new TestCaseSupplier.NumericTypeTestConfig<>(Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, (l, r) -> { double v = l.doubleValue() / r.doubleValue(); if (Double.isFinite(v)) { return v; @@ -90,20 +90,20 @@ public static Iterable parameters() { suppliers = errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers), DivTests::divErrorMessageString); // Divide by zero cases - all of these should warn and return null - TestCaseSupplier.NumericTypeTestConfigs typeStuff = new TestCaseSupplier.NumericTypeTestConfigs( - new TestCaseSupplier.NumericTypeTestConfig( + TestCaseSupplier.NumericTypeTestConfigs typeStuff = new TestCaseSupplier.NumericTypeTestConfigs<>( + new TestCaseSupplier.NumericTypeTestConfig<>( (Integer.MIN_VALUE >> 1) - 1, (Integer.MAX_VALUE >> 1) - 1, (l, r) -> null, "DivIntsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( (Long.MIN_VALUE >> 1) - 1, (Long.MAX_VALUE >> 1) - 1, (l, r) -> null, "DivLongsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, (l, r) -> null, @@ -115,7 +115,7 @@ public static Iterable parameters() { for (DataType lhsType : numericTypes) { for (DataType rhsType : numericTypes) { DataType expected = TestCaseSupplier.widen(lhsType, rhsType); - TestCaseSupplier.NumericTypeTestConfig expectedTypeStuff = typeStuff.get(expected); + TestCaseSupplier.NumericTypeTestConfig expectedTypeStuff = typeStuff.get(expected); BiFunction evaluatorToString = (lhs, rhs) -> expectedTypeStuff.evaluatorName() + "[" + "lhs" diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/ModTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/ModTests.java index d2af83e91ec64..a70f2c7885257 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/ModTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/ModTests.java @@ -34,20 +34,20 @@ public static Iterable parameters() { List suppliers = new ArrayList<>(); suppliers.addAll( TestCaseSupplier.forBinaryWithWidening( - new TestCaseSupplier.NumericTypeTestConfigs( - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfigs( + new TestCaseSupplier.NumericTypeTestConfig<>( (Integer.MIN_VALUE >> 1) - 1, (Integer.MAX_VALUE >> 1) - 1, (l, r) -> l.intValue() % r.intValue(), "ModIntsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( (Long.MIN_VALUE >> 1) - 1, (Long.MAX_VALUE >> 1) - 1, (l, r) -> l.longValue() % r.longValue(), "ModLongsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, (l, r) -> l.doubleValue() % r.doubleValue(), @@ -77,20 +77,20 @@ public static Iterable parameters() { suppliers = errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers), ModTests::modErrorMessageString); // Divide by zero cases - all of these should warn and return null - TestCaseSupplier.NumericTypeTestConfigs typeStuff = new TestCaseSupplier.NumericTypeTestConfigs( - new TestCaseSupplier.NumericTypeTestConfig( + TestCaseSupplier.NumericTypeTestConfigs typeStuff = new TestCaseSupplier.NumericTypeTestConfigs<>( + new TestCaseSupplier.NumericTypeTestConfig<>( (Integer.MIN_VALUE >> 1) - 1, (Integer.MAX_VALUE >> 1) - 1, (l, r) -> null, "ModIntsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( (Long.MIN_VALUE >> 1) - 1, (Long.MAX_VALUE >> 1) - 1, (l, r) -> null, "ModLongsEvaluator" ), - new TestCaseSupplier.NumericTypeTestConfig( + new TestCaseSupplier.NumericTypeTestConfig<>( Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, (l, r) -> null, @@ -102,7 +102,7 @@ public static Iterable parameters() { for (DataType lhsType : numericTypes) { for (DataType rhsType : numericTypes) { DataType expected = TestCaseSupplier.widen(lhsType, rhsType); - TestCaseSupplier.NumericTypeTestConfig expectedTypeStuff = typeStuff.get(expected); + TestCaseSupplier.NumericTypeTestConfig expectedTypeStuff = typeStuff.get(expected); BiFunction evaluatorToString = (lhs, rhs) -> expectedTypeStuff.evaluatorName() + "[" + "lhs" diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EqualsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EqualsTests.java index 0a1e9bdfaf34b..0739cd4670c08 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EqualsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/EqualsTests.java @@ -11,52 +11,198 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.Equals; +import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.DataTypes; -import org.hamcrest.Matcher; +import org.elasticsearch.xpack.ql.util.NumericUtils; +import java.math.BigInteger; +import java.util.ArrayList; import java.util.List; import java.util.function.Supplier; -import static org.hamcrest.Matchers.equalTo; - -public class EqualsTests extends AbstractBinaryComparisonTestCase { +public class EqualsTests extends AbstractFunctionTestCase { public EqualsTests(@Name("TestCase") Supplier testCaseSupplier) { this.testCase = testCaseSupplier.get(); } @ParametersFactory public static Iterable parameters() { - return parameterSuppliersFromTypedData(List.of(new TestCaseSupplier("Int == Int", () -> { - int rhs = randomInt(); - int lhs = randomInt(); - return new TestCaseSupplier.TestCase( - List.of( - new TestCaseSupplier.TypedData(lhs, DataTypes.INTEGER, "lhs"), - new TestCaseSupplier.TypedData(rhs, DataTypes.INTEGER, "rhs") + List suppliers = new ArrayList<>(); + suppliers.addAll( + TestCaseSupplier.forBinaryComparisonWithWidening( + new TestCaseSupplier.NumericTypeTestConfigs<>( + new TestCaseSupplier.NumericTypeTestConfig<>( + (Integer.MIN_VALUE >> 1) - 1, + (Integer.MAX_VALUE >> 1) - 1, + (l, r) -> l.intValue() == r.intValue(), + "EqualsIntsEvaluator" + ), + new TestCaseSupplier.NumericTypeTestConfig<>( + (Long.MIN_VALUE >> 1) - 1, + (Long.MAX_VALUE >> 1) - 1, + (l, r) -> l.longValue() == r.longValue(), + "EqualsLongsEvaluator" + ), + new TestCaseSupplier.NumericTypeTestConfig<>( + Double.NEGATIVE_INFINITY, + Double.POSITIVE_INFINITY, + // NB: this has different behavior than Double::equals + (l, r) -> l.doubleValue() == r.doubleValue(), + "EqualsDoublesEvaluator" + ) ), - "EqualsIntsEvaluator[lhs=Attribute[channel=0], rhs=Attribute[channel=1]]", + "lhs", + "rhs", + (lhs, rhs) -> List.of(), + false + ) + ); + + // Unsigned Long cases + // TODO: These should be integrated into the type cross product above, but are currently broken + // see https://github.com/elastic/elasticsearch/issues/102935 + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsLongsEvaluator", + "lhs", + "rhs", + Object::equals, DataTypes.BOOLEAN, - equalTo(lhs == rhs) - ); - }))); - } + TestCaseSupplier.ulongCases(BigInteger.ZERO, NumericUtils.UNSIGNED_LONG_MAX, true), + TestCaseSupplier.ulongCases(BigInteger.ZERO, NumericUtils.UNSIGNED_LONG_MAX, true), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsBoolsEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.booleanCases(), + TestCaseSupplier.booleanCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsKeywordsEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.ipCases(), + TestCaseSupplier.ipCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsKeywordsEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.versionCases(""), + TestCaseSupplier.versionCases(""), + List.of(), + false + ) + ); + // Datetime + // TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long? + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsLongsEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.dateCases(), + TestCaseSupplier.dateCases(), + List.of(), + false + ) + ); - @Override - protected > Matcher resultMatcher(T lhs, T rhs) { - return equalTo(lhs.equals(rhs)); - } + suppliers.addAll( + TestCaseSupplier.stringCases( + Object::equals, + (lhsType, rhsType) -> "EqualsKeywordsEvaluator[lhs=Attribute[channel=0], rhs=Attribute[channel=1]]", + List.of(), + DataTypes.BOOLEAN + ) + ); - @Override - protected BinaryComparison build(Source source, Expression lhs, Expression rhs) { - return new Equals(source, lhs, rhs); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsGeometriesEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.geoPointCases(), + TestCaseSupplier.geoPointCases(), + List.of(), + false + ) + ); + + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsGeometriesEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.geoShapeCases(), + TestCaseSupplier.geoShapeCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsGeometriesEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.cartesianPointCases(), + TestCaseSupplier.cartesianPointCases(), + List.of(), + false + ) + ); + + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "EqualsGeometriesEvaluator", + "lhs", + "rhs", + Object::equals, + DataTypes.BOOLEAN, + TestCaseSupplier.cartesianShapeCases(), + TestCaseSupplier.cartesianShapeCases(), + List.of(), + false + ) + ); + + return parameterSuppliersFromTypedData( + errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers), AbstractFunctionTestCase::errorMessageStringForBinaryOperators) + ); } @Override - protected boolean isEquality() { - return true; + protected Expression build(Source source, List args) { + return new Equals(source, args.get(0), args.get(1)); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanOrEqualTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanOrEqualTests.java index ad8dba7d63065..f45dedff837c4 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanOrEqualTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanOrEqualTests.java @@ -11,26 +11,25 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.GreaterThanOrEqual; +import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.DataTypes; -import org.hamcrest.Matcher; -import java.time.ZoneOffset; import java.util.List; import java.util.function.Supplier; import static org.hamcrest.Matchers.equalTo; -public class GreaterThanOrEqualTests extends AbstractBinaryComparisonTestCase { +public class GreaterThanOrEqualTests extends AbstractFunctionTestCase { public GreaterThanOrEqualTests(@Name("TestCase") Supplier testCaseSupplier) { this.testCase = testCaseSupplier.get(); } @ParametersFactory public static Iterable parameters() { + // ToDo: Add the full set of typed test cases here return parameterSuppliersFromTypedData(List.of(new TestCaseSupplier("Int >= Int", () -> { int rhs = randomInt(); int lhs = randomInt(); @@ -47,17 +46,7 @@ public static Iterable parameters() { } @Override - protected > Matcher resultMatcher(T lhs, T rhs) { - return equalTo(lhs.compareTo(rhs) >= 0); - } - - @Override - protected BinaryComparison build(Source source, Expression lhs, Expression rhs) { - return new GreaterThanOrEqual(source, lhs, rhs, ZoneOffset.UTC); - } - - @Override - protected boolean isEquality() { - return false; + protected Expression build(Source source, List args) { + return new GreaterThanOrEqual(source, args.get(0), args.get(1)); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanTests.java index b631a742f7885..e872af5b7c772 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/GreaterThanTests.java @@ -11,26 +11,25 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.GreaterThan; +import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.DataTypes; -import org.hamcrest.Matcher; -import java.time.ZoneOffset; import java.util.List; import java.util.function.Supplier; import static org.hamcrest.Matchers.equalTo; -public class GreaterThanTests extends AbstractBinaryComparisonTestCase { +public class GreaterThanTests extends AbstractFunctionTestCase { public GreaterThanTests(@Name("TestCase") Supplier testCaseSupplier) { this.testCase = testCaseSupplier.get(); } @ParametersFactory public static Iterable parameters() { + // ToDo: Add the full set of typed test cases here return parameterSuppliersFromTypedData(List.of(new TestCaseSupplier("Int > Int", () -> { int rhs = randomInt(); int lhs = randomInt(); @@ -47,17 +46,7 @@ public static Iterable parameters() { } @Override - protected > Matcher resultMatcher(T lhs, T rhs) { - return equalTo(lhs.compareTo(rhs) > 0); - } - - @Override - protected BinaryComparison build(Source source, Expression lhs, Expression rhs) { - return new GreaterThan(source, lhs, rhs, ZoneOffset.UTC); - } - - @Override - protected boolean isEquality() { - return false; + protected Expression build(Source source, List args) { + return new GreaterThan(source, args.get(0), args.get(1)); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanOrEqualTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanOrEqualTests.java index 7864a0dda9fe3..8bba0c4a5afb5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanOrEqualTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanOrEqualTests.java @@ -11,20 +11,18 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.LessThanOrEqual; +import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.DataTypes; -import org.hamcrest.Matcher; -import java.time.ZoneOffset; import java.util.List; import java.util.function.Supplier; import static org.hamcrest.Matchers.equalTo; -public class LessThanOrEqualTests extends AbstractBinaryComparisonTestCase { +public class LessThanOrEqualTests extends AbstractFunctionTestCase { public LessThanOrEqualTests(@Name("TestCase") Supplier testCaseSupplier) { this.testCase = testCaseSupplier.get(); } @@ -47,17 +45,7 @@ public static Iterable parameters() { } @Override - protected > Matcher resultMatcher(T lhs, T rhs) { - return equalTo(lhs.compareTo(rhs) <= 0); - } - - @Override - protected BinaryComparison build(Source source, Expression lhs, Expression rhs) { - return new LessThanOrEqual(source, lhs, rhs, ZoneOffset.UTC); - } - - @Override - protected boolean isEquality() { - return false; + protected Expression build(Source source, List args) { + return new LessThanOrEqual(source, args.get(0), args.get(1), null); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanTests.java index 826e88551077d..ab726dc51fbe4 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/LessThanTests.java @@ -11,20 +11,18 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.LessThan; +import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.DataTypes; -import org.hamcrest.Matcher; -import java.time.ZoneOffset; import java.util.List; import java.util.function.Supplier; import static org.hamcrest.Matchers.equalTo; -public class LessThanTests extends AbstractBinaryComparisonTestCase { +public class LessThanTests extends AbstractFunctionTestCase { public LessThanTests(@Name("TestCase") Supplier testCaseSupplier) { this.testCase = testCaseSupplier.get(); } @@ -47,17 +45,7 @@ public static Iterable parameters() { } @Override - protected > Matcher resultMatcher(T lhs, T rhs) { - return equalTo(lhs.compareTo(rhs) < 0); - } - - @Override - protected BinaryComparison build(Source source, Expression lhs, Expression rhs) { - return new LessThan(source, lhs, rhs, ZoneOffset.UTC); - } - - @Override - protected boolean isEquality() { - return false; + protected Expression build(Source source, List args) { + return new LessThan(source, args.get(0), args.get(1), null); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/NotEqualsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/NotEqualsTests.java index 0d6bb32fe2488..ec5d2338adae2 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/NotEqualsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/NotEqualsTests.java @@ -11,53 +11,192 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import org.elasticsearch.xpack.esql.evaluator.predicate.operator.comparison.NotEquals; +import org.elasticsearch.xpack.esql.expression.function.AbstractFunctionTestCase; import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier; import org.elasticsearch.xpack.ql.expression.Expression; -import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.DataTypes; -import org.hamcrest.Matcher; -import java.time.ZoneOffset; +import java.math.BigInteger; +import java.util.ArrayList; import java.util.List; import java.util.function.Supplier; -import static org.hamcrest.Matchers.equalTo; - -public class NotEqualsTests extends AbstractBinaryComparisonTestCase { +public class NotEqualsTests extends AbstractFunctionTestCase { public NotEqualsTests(@Name("TestCase") Supplier testCaseSupplier) { this.testCase = testCaseSupplier.get(); } @ParametersFactory public static Iterable parameters() { - return parameterSuppliersFromTypedData(List.of(new TestCaseSupplier("Int != Int", () -> { - int rhs = randomInt(); - int lhs = randomInt(); - return new TestCaseSupplier.TestCase( - List.of( - new TestCaseSupplier.TypedData(lhs, DataTypes.INTEGER, "lhs"), - new TestCaseSupplier.TypedData(rhs, DataTypes.INTEGER, "rhs") + List suppliers = new ArrayList<>(); + suppliers.addAll( + + TestCaseSupplier.forBinaryComparisonWithWidening( + new TestCaseSupplier.NumericTypeTestConfigs<>( + new TestCaseSupplier.NumericTypeTestConfig<>( + (Integer.MIN_VALUE >> 1) - 1, + (Integer.MAX_VALUE >> 1) - 1, + (l, r) -> l.intValue() != r.intValue(), + "NotEqualsIntsEvaluator" + ), + new TestCaseSupplier.NumericTypeTestConfig<>( + (Long.MIN_VALUE >> 1) - 1, + (Long.MAX_VALUE >> 1) - 1, + (l, r) -> l.longValue() != r.longValue(), + "NotEqualsLongsEvaluator" + ), + new TestCaseSupplier.NumericTypeTestConfig<>( + Double.NEGATIVE_INFINITY, + Double.POSITIVE_INFINITY, + // NB: this has different behavior than Double::equals + (l, r) -> l.doubleValue() != r.doubleValue(), + "NotEqualsDoublesEvaluator" + ) ), - "NotEqualsIntsEvaluator[lhs=Attribute[channel=0], rhs=Attribute[channel=1]]", + "lhs", + "rhs", + (lhs, rhs) -> List.of(), + false + ) + ); + // Unsigned Long cases + // TODO: These should be integrated into the type cross product above, but are currently broken + // see https://github.com/elastic/elasticsearch/issues/102935 + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsLongsEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), DataTypes.BOOLEAN, - equalTo(lhs != rhs) - ); - }))); - } - - @Override - protected > Matcher resultMatcher(T lhs, T rhs) { - return equalTo(false == lhs.equals(rhs)); - } - - @Override - protected BinaryComparison build(Source source, Expression lhs, Expression rhs) { - return new NotEquals(source, lhs, rhs, ZoneOffset.UTC); + TestCaseSupplier.ulongCases(BigInteger.ZERO, BigInteger.valueOf(Long.MAX_VALUE), true), + TestCaseSupplier.ulongCases(BigInteger.ZERO, BigInteger.valueOf(Long.MAX_VALUE), true), + List.of(), + true + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsBoolsEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.booleanCases(), + TestCaseSupplier.booleanCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsKeywordsEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.ipCases(), + TestCaseSupplier.ipCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsKeywordsEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.versionCases(""), + TestCaseSupplier.versionCases(""), + List.of(), + false + ) + ); + // Datetime + // TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long? + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsLongsEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.dateCases(), + TestCaseSupplier.dateCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.stringCases( + (l, r) -> false == l.equals(r), + (lhsType, rhsType) -> "NotEqualsKeywordsEvaluator[lhs=Attribute[channel=0], rhs=Attribute[channel=1]]", + List.of(), + DataTypes.BOOLEAN + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsGeometriesEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.geoPointCases(), + TestCaseSupplier.geoPointCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsGeometriesEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.geoShapeCases(), + TestCaseSupplier.geoShapeCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsGeometriesEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.cartesianPointCases(), + TestCaseSupplier.cartesianPointCases(), + List.of(), + false + ) + ); + suppliers.addAll( + TestCaseSupplier.forBinaryNotCasting( + "NotEqualsGeometriesEvaluator", + "lhs", + "rhs", + (l, r) -> false == l.equals(r), + DataTypes.BOOLEAN, + TestCaseSupplier.cartesianShapeCases(), + TestCaseSupplier.cartesianShapeCases(), + List.of(), + false + ) + ); + return parameterSuppliersFromTypedData( + errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers), AbstractFunctionTestCase::errorMessageStringForBinaryOperators) + ); } @Override - protected boolean isEquality() { - return true; + protected Expression build(Source source, List args) { + return new NotEquals(source, args.get(0), args.get(1)); } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRulesTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRulesTests.java index 01fcd222a5141..28944252191be 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRulesTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRulesTests.java @@ -495,7 +495,7 @@ public void testPropagateEquals_VarEq2OrVarNeq5() { // a = 2 OR 3 < a < 4 OR a > 2 OR a!= 2 -> TRUE public void testPropagateEquals_VarEq2OrVarRangeGt3Lt4OrVarGt2OrVarNe2() { FieldAttribute fa = getFieldAttribute(); - org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals eq = equalsOf(fa, TWO); + Equals eq = equalsOf(fa, TWO); Range range = rangeOf(fa, THREE, false, FOUR, false); GreaterThan gt = greaterThanOf(fa, TWO); NotEquals neq = notEqualsOf(fa, TWO);