From dfe4c6c568975c9145f70c61ab312d0b2981f5df Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Mon, 1 Apr 2019 18:10:51 +0300 Subject: [PATCH] SQL: have LIKE/RLIKE use wildcard and regexp queries (#40628) * Have LIKE and RLIKE only use term-level queries (wildcard and regexp respectively). They are already working only with exact fields, thus be in-line with how SQL works in general (what you index is what you search on). (cherry picked from commit 1bba887d481b49db231a1442922f1813952dcc67) --- .../xpack/sql/expression/FieldAttribute.java | 2 +- .../xpack/sql/planner/QueryTranslator.java | 24 ++----- .../analyzer/VerifierErrorMessagesTests.java | 8 ++- .../sql/planner/QueryTranslatorTests.java | 68 ++++++++++++------- 4 files changed, 57 insertions(+), 45 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java index 811cc299ccb..cb86e2742b2 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java @@ -88,7 +88,7 @@ public class FieldAttribute extends TypedAttribute { public FieldAttribute exactAttribute() { EsField exactField = field.getExactField(); if (exactField.equals(field) == false) { - return innerField(field.getExactField()); + return innerField(exactField); } return this; } 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 1fdd27d9b0b..1ad5f812777 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 @@ -24,9 +24,9 @@ import org.elasticsearch.xpack.sql.expression.function.aggregate.Count; import org.elasticsearch.xpack.sql.expression.function.aggregate.ExtendedStats; import org.elasticsearch.xpack.sql.expression.function.aggregate.First; import org.elasticsearch.xpack.sql.expression.function.aggregate.Last; -import org.elasticsearch.xpack.sql.expression.function.aggregate.MedianAbsoluteDeviation; import org.elasticsearch.xpack.sql.expression.function.aggregate.MatrixStats; import org.elasticsearch.xpack.sql.expression.function.aggregate.Max; +import org.elasticsearch.xpack.sql.expression.function.aggregate.MedianAbsoluteDeviation; import org.elasticsearch.xpack.sql.expression.function.aggregate.Min; import org.elasticsearch.xpack.sql.expression.function.aggregate.PercentileRanks; import org.elasticsearch.xpack.sql.expression.function.aggregate.Percentiles; @@ -470,7 +470,6 @@ final class QueryTranslator { af.nodeString()); } - // TODO: need to optimize on ngram // TODO: see whether escaping is needed @SuppressWarnings("rawtypes") static class Likes extends ExpressionTranslator { @@ -478,34 +477,23 @@ final class QueryTranslator { @Override protected QueryTranslation asQuery(RegexMatch e, boolean onAggs) { Query q = null; - boolean inexact = true; - String target = null; + String targetFieldName = null; if (e.field() instanceof FieldAttribute) { - target = nameOf(((FieldAttribute) e.field()).exactAttribute()); + targetFieldName = nameOf(((FieldAttribute) e.field()).exactAttribute()); } else { - throw new SqlIllegalArgumentException("Scalar function ({}) not allowed (yet) as arguments for LIKE", + throw new SqlIllegalArgumentException("Scalar function [{}] not allowed (yet) as argument for " + e.functionName(), Expressions.name(e.field())); } if (e instanceof Like) { LikePattern p = ((Like) e).pattern(); - if (inexact) { - q = new QueryStringQuery(e.source(), p.asLuceneWildcard(), target); - } - else { - q = new WildcardQuery(e.source(), nameOf(e.field()), p.asLuceneWildcard()); - } + q = new WildcardQuery(e.source(), targetFieldName, p.asLuceneWildcard()); } if (e instanceof RLike) { String pattern = ((RLike) e).pattern(); - if (inexact) { - q = new QueryStringQuery(e.source(), "/" + pattern + "/", target); - } - else { - q = new RegexQuery(e.source(), nameOf(e.field()), pattern); - } + q = new RegexQuery(e.source(), targetFieldName, pattern); } return q != null ? new QueryTranslation(wrapIfNested(q, e.field())) : null; diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java index 9d55d4aeec7..c76ddffe437 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java @@ -535,12 +535,18 @@ public class VerifierErrorMessagesTests extends ESTestCase { error("SELECT INSERT('text', 1, 2, 3)")); } - public void testInvalidTypeForRegexMatch() { + public void testInvalidTypeForLikeMatch() { assertEquals("1:26: [text LIKE 'foo'] cannot operate on field of data type [text]: " + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", error("SELECT * FROM test WHERE text LIKE 'foo'")); } + public void testInvalidTypeForRLikeMatch() { + assertEquals("1:26: [text RLIKE 'foo'] cannot operate on field of data type [text]: " + + "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead", + error("SELECT * FROM test WHERE text RLIKE 'foo'")); + } + public void testAllowCorrectFieldsInIncompatibleMappings() { assertNotNull(incompatibleAccept("SELECT languages FROM \"*\"")); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index a39b5466bc1..d1408688427 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -41,11 +41,12 @@ import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery; import org.elasticsearch.xpack.sql.querydsl.query.ExistsQuery; import org.elasticsearch.xpack.sql.querydsl.query.NotQuery; import org.elasticsearch.xpack.sql.querydsl.query.Query; -import org.elasticsearch.xpack.sql.querydsl.query.QueryStringQuery; import org.elasticsearch.xpack.sql.querydsl.query.RangeQuery; +import org.elasticsearch.xpack.sql.querydsl.query.RegexQuery; import org.elasticsearch.xpack.sql.querydsl.query.ScriptQuery; import org.elasticsearch.xpack.sql.querydsl.query.TermQuery; import org.elasticsearch.xpack.sql.querydsl.query.TermsQuery; +import org.elasticsearch.xpack.sql.querydsl.query.WildcardQuery; import org.elasticsearch.xpack.sql.stats.Metrics; import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.EsField; @@ -186,20 +187,41 @@ public class QueryTranslatorTests extends ESTestCase { assertTrue(p instanceof Filter); Expression condition = ((Filter) p).condition(); QueryTranslation qt = QueryTranslator.toQuery(condition, false); - assertEquals(QueryStringQuery.class, qt.query.getClass()); - QueryStringQuery qsq = ((QueryStringQuery) qt.query); - assertEquals(1, qsq.fields().size()); - assertEquals("some.string.typical", qsq.fields().keySet().iterator().next()); + assertEquals(WildcardQuery.class, qt.query.getClass()); + WildcardQuery qsq = ((WildcardQuery) qt.query); + assertEquals("some.string.typical", qsq.field()); + } + + public void testRLikeOnInexact() { + LogicalPlan p = plan("SELECT * FROM test WHERE some.string RLIKE '.*a.*'"); + assertTrue(p instanceof Project); + p = ((Project) p).child(); + assertTrue(p instanceof Filter); + Expression condition = ((Filter) p).condition(); + QueryTranslation qt = QueryTranslator.toQuery(condition, false); + assertEquals(RegexQuery.class, qt.query.getClass()); + RegexQuery qsq = ((RegexQuery) qt.query); + assertEquals("some.string.typical", qsq.field()); } public void testLikeConstructsNotSupported() { - LogicalPlan p = plan("SELECT LTRIM(keyword) lt FROM test WHERE LTRIM(keyword) LIKE '%a%'"); + LogicalPlan p = plan("SELECT LTRIM(keyword) lt FROM test WHERE LTRIM(keyword) like '%a%'"); assertTrue(p instanceof Project); p = ((Project) p).child(); assertTrue(p instanceof Filter); Expression condition = ((Filter) p).condition(); SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false)); - assertEquals("Scalar function (LTRIM(keyword)) not allowed (yet) as arguments for LIKE", ex.getMessage()); + assertEquals("Scalar function [LTRIM(keyword)] not allowed (yet) as argument for LIKE", ex.getMessage()); + } + + public void testRLikeConstructsNotSupported() { + LogicalPlan p = plan("SELECT LTRIM(keyword) lt FROM test WHERE LTRIM(keyword) RLIKE '.*a.*'"); + assertTrue(p instanceof Project); + p = ((Project) p).child(); + assertTrue(p instanceof Filter); + Expression condition = ((Filter) p).condition(); + SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false)); + assertEquals("Scalar function [LTRIM(keyword)] not allowed (yet) as argument for RLIKE", ex.getMessage()); } public void testDifferentLikeAndNotLikePatterns() { @@ -213,20 +235,18 @@ public class QueryTranslatorTests extends ESTestCase { assertEquals(BoolQuery.class, qt.query.getClass()); BoolQuery bq = ((BoolQuery) qt.query); assertTrue(bq.isAnd()); - assertTrue(bq.left() instanceof QueryStringQuery); + assertTrue(bq.left() instanceof WildcardQuery); assertTrue(bq.right() instanceof NotQuery); NotQuery nq = (NotQuery) bq.right(); - assertTrue(nq.child() instanceof QueryStringQuery); - QueryStringQuery lqsq = (QueryStringQuery) bq.left(); - QueryStringQuery rqsq = (QueryStringQuery) nq.child(); + assertTrue(nq.child() instanceof WildcardQuery); + WildcardQuery lqsq = (WildcardQuery) bq.left(); + WildcardQuery rqsq = (WildcardQuery) nq.child(); assertEquals("X*", lqsq.query()); - assertEquals(1, lqsq.fields().size()); - assertEquals("keyword", lqsq.fields().keySet().iterator().next()); + assertEquals("keyword", lqsq.field()); assertEquals("Y*", rqsq.query()); - assertEquals(1, rqsq.fields().size()); - assertEquals("keyword", rqsq.fields().keySet().iterator().next()); + assertEquals("keyword", rqsq.field()); } public void testRLikePatterns() { @@ -248,20 +268,18 @@ public class QueryTranslatorTests extends ESTestCase { assertEquals(BoolQuery.class, qt.query.getClass()); BoolQuery bq = ((BoolQuery) qt.query); assertTrue(bq.isAnd()); - assertTrue(bq.left() instanceof QueryStringQuery); + assertTrue(bq.left() instanceof RegexQuery); assertTrue(bq.right() instanceof NotQuery); NotQuery nq = (NotQuery) bq.right(); - assertTrue(nq.child() instanceof QueryStringQuery); - QueryStringQuery lqsq = (QueryStringQuery) bq.left(); - QueryStringQuery rqsq = (QueryStringQuery) nq.child(); + assertTrue(nq.child() instanceof RegexQuery); + RegexQuery lqsq = (RegexQuery) bq.left(); + RegexQuery rqsq = (RegexQuery) nq.child(); - assertEquals("/" + firstPattern + "/", lqsq.query()); - assertEquals(1, lqsq.fields().size()); - assertEquals("keyword", lqsq.fields().keySet().iterator().next()); - assertEquals("/" + secondPattern + "/", rqsq.query()); - assertEquals(1, rqsq.fields().size()); - assertEquals("keyword", rqsq.fields().keySet().iterator().next()); + assertEquals(firstPattern, lqsq.regex()); + assertEquals("keyword", lqsq.field()); + assertEquals(secondPattern, rqsq.regex()); + assertEquals("keyword", rqsq.field()); } public void testTranslateNotExpression_WhereClause_Painless() {