diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/ExpressionBuilder.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/ExpressionBuilder.java index 6510fdc3561..e5e344f5acd 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/ExpressionBuilder.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/ExpressionBuilder.java @@ -35,6 +35,7 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Sub; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThan; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThanOrEqual; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.In; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThan; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThanOrEqual; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.NotEquals; @@ -137,14 +138,7 @@ public class ExpressionBuilder extends IdentifierBuilder { } List container = expressions(predicate.expression()); - - // TODO: Add IN to QL and use that directly - Expression checkInSet = null; - - for (Expression inner : container) { - Expression termCheck = new Equals(source, expr, inner); - checkInSet = checkInSet == null ? termCheck : new Or(source, checkInSet, termCheck); - } + Expression checkInSet = new In(source, expr, container); return predicate.NOT() != null ? new Not(source, checkInSet) : checkInSet; } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java index 23763bec6ec..03125afc05b 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java @@ -298,6 +298,12 @@ public class VerifierTests extends ESTestCase { error(idxr, "foo where ip_range_field == ''")); } + public void testMixedSet() { + final IndexResolution idxr = loadIndexResolution("mapping-numeric.json"); + assertEquals("1:11: 2nd argument of [long_field in (1, 'string')] must be [long], found value ['string'] type [keyword]", + error(idxr, "foo where long_field in (1, 'string')")); + } + public void testObject() { final IndexResolution idxr = loadIndexResolution("mapping-object.json"); accept(idxr, "foo where endgame.pid == 0"); diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/parser/ExpressionTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/parser/ExpressionTests.java index d8b12d85934..5a6c4b6573e 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/parser/ExpressionTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/parser/ExpressionTests.java @@ -13,16 +13,19 @@ import org.elasticsearch.xpack.ql.expression.Literal; import org.elasticsearch.xpack.ql.expression.UnresolvedAttribute; import org.elasticsearch.xpack.ql.expression.function.UnresolvedFunction; import org.elasticsearch.xpack.ql.expression.predicate.logical.And; +import org.elasticsearch.xpack.ql.expression.predicate.logical.Not; import org.elasticsearch.xpack.ql.expression.predicate.logical.Or; import org.elasticsearch.xpack.ql.expression.predicate.operator.arithmetic.Neg; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThan; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThanOrEqual; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.In; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThan; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThanOrEqual; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.NotEquals; import org.elasticsearch.xpack.ql.type.DataTypes; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -40,6 +43,14 @@ public class ExpressionTests extends ESTestCase { return parser.createExpression(source); } + private List exprs(String... sources) { + List results = new ArrayList(sources.length); + for (String s : sources) { + results.add(expr(s)); + } + return results; + } + public void testStrings() throws Exception { assertEquals("hello\"world", unquoteString("'hello\"world'")); @@ -156,19 +167,49 @@ public class ExpressionTests extends ESTestCase { } public void testInSet() { + assertEquals( + expr("name in (1)"), + new In(null, expr("name"), exprs("1")) + ); + + assertEquals( + expr("name in (2, 1)"), + new In(null, expr("name"), exprs("2", "1")) + ); assertEquals( expr("name in ('net.exe')"), - expr("name == 'net.exe'") + new In(null, expr("name"), exprs("'net.exe'")) ); assertEquals( expr("name in ('net.exe', 'whoami.exe', 'hostname.exe')"), - expr("name == 'net.exe' or name == 'whoami.exe' or name == 'hostname.exe'") + new In(null, expr("name"), exprs("'net.exe'", "'whoami.exe'", "'hostname.exe'")) + ); + } + + public void testInSetDuplicates() { + assertEquals( + expr("name in (1, 1)"), + new In(null, expr("name"), exprs("1", "1")) ); assertEquals( - expr("name not in ('net.exe', 'whoami.exe', 'hostname.exe')"), - expr("not (name == 'net.exe' or name == 'whoami.exe' or name == 'hostname.exe')") + expr("name in ('net.exe', 'net.exe')"), + new In(null, expr("name"), exprs("'net.exe'", "'net.exe'")) ); } + + public void testNotInSet() { + assertEquals( + expr("name not in ('net.exe', 'whoami.exe', 'hostname.exe')"), + new Not(null, new In(null, + expr("name"), + exprs("'net.exe'", "'whoami.exe'", "'hostname.exe'"))) + ); + } + + public void testInEmptySet() { + expectThrows(ParsingException.class, "Expected syntax error", + () -> expr("name in ()")); + } } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderFailTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderFailTests.java index 471ee4c5694..c8542e780ff 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderFailTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderFailTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.eql.planner; +import org.elasticsearch.xpack.eql.analysis.VerificationException; import org.elasticsearch.xpack.ql.QlIllegalArgumentException; public class QueryFolderFailTests extends AbstractQueryFolderTestCase { @@ -17,9 +18,10 @@ public class QueryFolderFailTests extends AbstractQueryFolderTestCase { } public void testPropertyEquationInClauseFilterUnsupported() { - QlIllegalArgumentException e = expectThrows(QlIllegalArgumentException.class, + VerificationException e = expectThrows(VerificationException.class, () -> plan("process where opcode in (1,3) and process_name in (parent_process_name, \"SYSTEM\")")); String msg = e.getMessage(); - assertEquals("Line 1:52: Comparisons against variables are not (currently) supported; offender [parent_process_name] in [==]", msg); + assertEquals("Found 1 problem\nline 1:35: Comparisons against variables are not (currently) supported; " + + "offender [parent_process_name] in [process_name in (parent_process_name, \"SYSTEM\")]", msg); } } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderOkTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderOkTests.java index 2beb44a269b..25a4f673b1f 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderOkTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/planner/QueryFolderOkTests.java @@ -20,6 +20,7 @@ import static org.elasticsearch.xpack.ql.type.DataTypes.KEYWORD; import static org.hamcrest.Matchers.containsString; public class QueryFolderOkTests extends AbstractQueryFolderTestCase { + private final String name; private final String query; private final Object expect; diff --git a/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt b/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt index a4f73f1b196..66bbb5f29b1 100644 --- a/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt +++ b/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt @@ -52,18 +52,13 @@ process where not (exit_code > -1) inFilter process where process_name in ("python.exe", "SMSS.exe", "explorer.exe") -"term":{"process_name":{"value":"python.exe" -"term":{"process_name":{"value":"SMSS.exe" -"term":{"process_name":{"value":"explorer.exe" +"terms":{"process_name":["python.exe","SMSS.exe","explorer.exe"], equalsAndInFilter process where process_path == "*\\red_ttp\\wininit.*" and opcode in (0,1,2,3) "wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*" -"term":{"opcode":{"value":0 -"term":{"opcode":{"value":1 -"term":{"opcode":{"value":2 -"term":{"opcode":{"value":3 +{"terms":{"opcode":[0,1,2,3] substringFunction diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslator.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslator.java index 706e53a176a..85f644a8ce2 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslator.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslator.java @@ -32,4 +32,4 @@ public abstract class ExpressionTranslator { } return query; } -} \ No newline at end of file +} diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java index 2c2d6f657d1..4e3d4133a7b 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java @@ -23,6 +23,7 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Binar import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThan; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.GreaterThanOrEqual; +import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.In; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThan; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.LessThanOrEqual; import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.NotEquals; @@ -41,29 +42,37 @@ import org.elasticsearch.xpack.ql.querydsl.query.RangeQuery; import org.elasticsearch.xpack.ql.querydsl.query.RegexQuery; import org.elasticsearch.xpack.ql.querydsl.query.ScriptQuery; import org.elasticsearch.xpack.ql.querydsl.query.TermQuery; +import org.elasticsearch.xpack.ql.querydsl.query.TermsQuery; import org.elasticsearch.xpack.ql.querydsl.query.WildcardQuery; import org.elasticsearch.xpack.ql.tree.Source; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypes; import org.elasticsearch.xpack.ql.util.Check; +import org.elasticsearch.xpack.ql.util.CollectionUtils; import org.elasticsearch.xpack.ql.util.Holder; import java.time.OffsetTime; import java.time.ZonedDateTime; import java.time.temporal.TemporalAccessor; import java.util.Arrays; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; + public final class ExpressionTranslators { public static final String DATE_FORMAT = "strict_date_time"; public static final String TIME_FORMAT = "strict_hour_minute_second_millis"; - + public static final List> QUERY_TRANSLATORS = Arrays.asList( new BinaryComparisons(), new Ranges(), new BinaryLogic(), new Nots(), new Likes(), + new InComparisons(), new StringQueries(), new Matches(), new MultiMatches(), @@ -86,6 +95,13 @@ public final class ExpressionTranslators { throw new QlIllegalArgumentException("Don't know how to translate {} {}", e.nodeName(), e); } + public static Object valueOf(Expression e) { + if (e.foldable()) { + return e.fold(); + } + throw new QlIllegalArgumentException("Cannot determine value for {}", e); + } + // TODO: see whether escaping is needed @SuppressWarnings("rawtypes") public static class Likes extends ExpressionTranslator { @@ -196,7 +212,7 @@ public final class ExpressionTranslators { protected Query asQuery(BinaryComparison bc, TranslatorHandler handler) { return doTranslate(bc, handler); } - + public static void checkBinaryComparison(BinaryComparison bc) { Check.isTrue(bc.right().foldable(), "Line {}:{}: Comparisons against variables are not (currently) supported; offender [{}] in [{}]", @@ -315,6 +331,36 @@ public final class ExpressionTranslators { } } + public static class InComparisons extends ExpressionTranslator { + + protected Query asQuery(In in, TranslatorHandler handler) { + return doTranslate(in, handler); + } + + public static Query doTranslate(In in, TranslatorHandler handler) { + Query q; + if (in.value() instanceof FieldAttribute) { + // equality should always be against an exact match (which is important for strings) + FieldAttribute fa = (FieldAttribute) in.value(); + List list = in.list(); + + // TODO: this needs to be handled inside the optimizer + list.removeIf(e -> DataTypes.isNull(e.dataType())); + DataType dt = list.get(0).dataType(); + Set set = new LinkedHashSet<>(CollectionUtils.mapSize(list.size())); + + for (Expression e : list) { + set.add(handler.convert(valueOf(e), dt)); + } + + q = new TermsQuery(in.source(), fa.exactAttribute().name(), set); + } else { + q = new ScriptQuery(in.source(), in.asScript()); + } + return handler.wrapFunctionQuery(in, in.value(), q); + } + } + public static class Scalars extends ExpressionTranslator { @Override @@ -327,13 +373,6 @@ public final class ExpressionTranslators { } } - public static Object valueOf(Expression e) { - if (e.foldable()) { - return e.fold(); - } - throw new QlIllegalArgumentException("Cannot determine value for {}", e); - } - public static Query or(Source source, Query left, Query right) { return boolQuery(source, left, right, false); } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/QlTranslatorHandler.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/QlTranslatorHandler.java index 157062cc3fa..6ec98e524bf 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/QlTranslatorHandler.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/QlTranslatorHandler.java @@ -12,6 +12,8 @@ import org.elasticsearch.xpack.ql.expression.NamedExpression; import org.elasticsearch.xpack.ql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.ql.querydsl.query.Query; import org.elasticsearch.xpack.ql.querydsl.query.ScriptQuery; +import org.elasticsearch.xpack.ql.type.DataType; +import org.elasticsearch.xpack.ql.type.DataTypeConverter; public class QlTranslatorHandler implements TranslatorHandler { @@ -41,4 +43,9 @@ public class QlTranslatorHandler implements TranslatorHandler { public String dateFormat(Expression e) { return null; } + + @Override + public Object convert(Object value, DataType dataType) { + return DataTypeConverter.convert(value, dataType); + } } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/TranslatorHandler.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/TranslatorHandler.java index 902ee0dbb8a..ecd39fda933 100644 --- a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/TranslatorHandler.java +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/TranslatorHandler.java @@ -9,10 +9,11 @@ package org.elasticsearch.xpack.ql.planner; import org.elasticsearch.xpack.ql.expression.Expression; import org.elasticsearch.xpack.ql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.ql.querydsl.query.Query; +import org.elasticsearch.xpack.ql.type.DataType; /** * Parameterized handler used during query translation. - * + * * Provides contextual utilities for an individual query to be performed. */ public interface TranslatorHandler { @@ -24,4 +25,6 @@ public interface TranslatorHandler { String nameOf(Expression e); String dateFormat(Expression e); + + Object convert(Object value, DataType dataType); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java index 3c8357262cf..32f0c7a145f 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java @@ -37,11 +37,8 @@ import org.elasticsearch.xpack.ql.querydsl.query.GeoDistanceQuery; import org.elasticsearch.xpack.ql.querydsl.query.NotQuery; import org.elasticsearch.xpack.ql.querydsl.query.Query; import org.elasticsearch.xpack.ql.querydsl.query.ScriptQuery; -import org.elasticsearch.xpack.ql.querydsl.query.TermsQuery; import org.elasticsearch.xpack.ql.tree.Source; -import org.elasticsearch.xpack.ql.type.DataType; import org.elasticsearch.xpack.ql.type.DataTypes; -import org.elasticsearch.xpack.ql.util.CollectionUtils; import org.elasticsearch.xpack.ql.util.ReflectionUtils; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.function.aggregate.Avg; @@ -84,9 +81,7 @@ import org.elasticsearch.xpack.sql.util.Check; import java.util.ArrayList; import java.util.Arrays; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Set; import static java.util.Collections.singletonList; import static org.elasticsearch.xpack.ql.expression.Expressions.id; @@ -461,23 +456,7 @@ final class QueryTranslator { aggFilter = new AggFilter(id(in.value()), in.asScript()); } else { - Query q = null; - if (in.value() instanceof FieldAttribute) { - // equality should always be against an exact match (which is important for strings) - FieldAttribute fa = (FieldAttribute) in.value(); - List list = in.list(); - // TODO: this needs to be handled inside the optimizer - list.removeIf(e -> DataTypes.isNull(e.dataType())); - DataType dt = list.get(0).dataType(); - Set set = new LinkedHashSet<>(CollectionUtils.mapSize(list.size())); - for (Expression e : list) { - set.add(SqlDataTypeConverter.convert(e.fold(), dt)); - } - q = new TermsQuery(in.source(), fa.exactAttribute().name(), set); - } else { - q = new ScriptQuery(in.source(), in.asScript()); - } - query = handler.wrapFunctionQuery(in, in.value(), q); + query = org.elasticsearch.xpack.ql.planner.ExpressionTranslators.InComparisons.doTranslate(in, handler); } return new QueryTranslation(query, aggFilter); } @@ -698,7 +677,7 @@ final class QueryTranslator { protected abstract LeafAgg toAgg(String id, C f); } - + private static List foldAndConvertToDoubles(List list) { List values = new ArrayList<>(list.size()); for (Expression e : list) { @@ -706,4 +685,4 @@ final class QueryTranslator { } return values; } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/SqlTranslatorHandler.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/SqlTranslatorHandler.java index a6179dd8e01..61467046c9e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/SqlTranslatorHandler.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/SqlTranslatorHandler.java @@ -14,8 +14,10 @@ import org.elasticsearch.xpack.ql.planner.TranslatorHandler; import org.elasticsearch.xpack.ql.querydsl.query.GeoDistanceQuery; import org.elasticsearch.xpack.ql.querydsl.query.Query; import org.elasticsearch.xpack.ql.querydsl.query.ScriptQuery; +import org.elasticsearch.xpack.ql.type.DataType; import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFunction; import org.elasticsearch.xpack.sql.expression.function.scalar.geo.StDistance; +import org.elasticsearch.xpack.sql.type.SqlDataTypeConverter; public class SqlTranslatorHandler implements TranslatorHandler { @@ -53,4 +55,9 @@ public class SqlTranslatorHandler implements TranslatorHandler { } return null; } -} \ No newline at end of file + + @Override + public Object convert(Object value, DataType dataType) { + return SqlDataTypeConverter.convert(value, dataType); + } +}