From 5b47c67dec3236b96ebab5bdc453b592caa39874 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 20 Dec 2017 12:35:32 -0700 Subject: [PATCH] SQL: Add all MATCH() query options (elastic/x-pack-elasticsearch#3387) * SQL: Add all MATCH() query options This adds support for (almost) all of the options that the `match` query supports. Additionally, it reverts back to the default operator of `OR` rather than `AND` for the `MATCH()` query. Relates to elastic/x-pack-elasticsearch#3361 * Add getters required for Node usage Original commit: elastic/x-pack-elasticsearch@144e2bec02aa5bf34658f204b1d251f4bbec82fa --- qa/sql/src/main/resources/fulltext.csv-spec | 7 ++ .../fulltext/MatchQueryPredicate.java | 9 +-- .../xpack/sql/querydsl/query/MatchQuery.java | 69 +++++++++++++------ .../search/SourceGeneratorTests.java | 4 +- .../sql/querydsl/query/MatchQueryTests.java | 43 ++++++++++++ 5 files changed, 102 insertions(+), 30 deletions(-) create mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java diff --git a/qa/sql/src/main/resources/fulltext.csv-spec b/qa/sql/src/main/resources/fulltext.csv-spec index e952b564567..5ffcdb6afca 100644 --- a/qa/sql/src/main/resources/fulltext.csv-spec +++ b/qa/sql/src/main/resources/fulltext.csv-spec @@ -23,6 +23,13 @@ SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH(first_nam 10076 |Erez |F |Ritzmann ; +matchQueryWithOptions +SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH(first_name, 'Erez', 'lenient=true;cutoff_frequency=2;fuzzy_rewrite=scoring_boolean;minimum_should_match=1;operator=AND;max_expansions=30;prefix_length=1;analyzer=english;auto_generate_synonyms_phrase_query=true;fuzzy_transpositions=true'); + + emp_no:i | first_name:s | gender:s | last_name:s +10076 |Erez |F |Ritzmann +; + multiMatchQuery SELECT emp_no, first_name, gender, last_name FROM test_emp WHERE MATCH('first_name,last_name', 'Morton', 'type=best_fields;operator=OR'); diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/MatchQueryPredicate.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/MatchQueryPredicate.java index fd9df9e3ca7..8fe8e99e2e4 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/MatchQueryPredicate.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/MatchQueryPredicate.java @@ -15,23 +15,16 @@ import static java.util.Collections.singletonList; public class MatchQueryPredicate extends FullTextPredicate { private final Expression field; - private final Operator operator; - + public MatchQueryPredicate(Location location, Expression field, String query, String options) { super(location, query, options, singletonList(field)); this.field = field; - - this.operator = FullTextUtils.operator(optionMap(), "operator"); } public Expression field() { return field; } - public Operator operator() { - return operator; - } - @Override public int hashCode() { return Objects.hash(field, super.hashCode()); diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQuery.java index cf43069d605..59240ed1e32 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQuery.java @@ -5,54 +5,84 @@ */ package org.elasticsearch.xpack.sql.querydsl.query; -import java.util.Objects; - +import org.elasticsearch.common.Booleans; import org.elasticsearch.index.query.MatchQueryBuilder; +import org.elasticsearch.index.query.Operator; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate; -import org.elasticsearch.xpack.sql.expression.predicate.fulltext.FullTextPredicate.Operator; import org.elasticsearch.xpack.sql.tree.Location; -import static org.elasticsearch.index.query.QueryBuilders.matchQuery; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.function.BiConsumer; public class MatchQuery extends LeafQuery { + private static final Map> BUILDER_APPLIERS; + + static { + HashMap> appliers = new HashMap<>(14); + // TODO: it'd be great if these could be constants instead of Strings, needs a core change to make the fields public first + // TODO: add zero terms query support, I'm not sure the best way to parse it yet... + // appliers.put("zero_terms_query", (qb, s) -> qb.zeroTermsQuery(s)); + appliers.put("cutoff_frequency", (qb, s) -> qb.cutoffFrequency(Float.valueOf(s))); + appliers.put("lenient", (qb, s) -> qb.lenient(Booleans.parseBoolean(s))); + appliers.put("fuzzy_transpositions", (qb, s) -> qb.fuzzyTranspositions(Booleans.parseBoolean(s))); + appliers.put("fuzzy_rewrite", (qb, s) -> qb.fuzzyRewrite(s)); + appliers.put("minimum_should_match", (qb, s) -> qb.minimumShouldMatch(s)); + appliers.put("operator", (qb, s) -> qb.operator(Operator.fromString(s))); + appliers.put("max_expansions", (qb, s) -> qb.maxExpansions(Integer.valueOf(s))); + appliers.put("prefix_length", (qb, s) -> qb.prefixLength(Integer.valueOf(s))); + appliers.put("analyzer", (qb, s) -> qb.analyzer(s)); + appliers.put("auto_generate_synonyms_phrase_query", (qb, s) -> qb.autoGenerateSynonymsPhraseQuery(Booleans.parseBoolean(s))); + BUILDER_APPLIERS = Collections.unmodifiableMap(appliers); + } + private final String name; private final Object text; - private final Operator operator; private final MatchQueryPredicate predicate; + private final Map options; + public MatchQuery(Location location, String name, Object text) { - this(location, name, text, Operator.AND, null); + this(location, name, text, null); } public MatchQuery(Location location, String name, Object text, MatchQueryPredicate predicate) { - this(location, name, text, null, predicate); - } - - private MatchQuery(Location location, String name, Object text, Operator operator, MatchQueryPredicate predicate) { super(location); this.name = name; this.text = text; this.predicate = predicate; - this.operator = operator != null ? operator : predicate.operator(); + this.options = predicate == null ? Collections.emptyMap() : predicate.optionMap(); } @Override public QueryBuilder asBuilder() { - MatchQueryBuilder queryBuilder = matchQuery(name, text); - if (operator != null) { - queryBuilder.operator(operator.toEs()); - } - if (predicate != null) { - queryBuilder.analyzer(predicate.analyzer()); - } + final MatchQueryBuilder queryBuilder = QueryBuilders.matchQuery(name, text); + options.forEach((k, v) -> { + if (BUILDER_APPLIERS.containsKey(k)) { + BUILDER_APPLIERS.get(k).accept(queryBuilder, v); + } else { + throw new IllegalArgumentException("illegal match option [" + k + "]"); + } + }); return queryBuilder; } + public String name() { + return name; + } + + public Object text() { + return text; + } + @Override public int hashCode() { - return Objects.hash(text, name, operator, predicate); + return Objects.hash(text, name, predicate); } @Override @@ -68,7 +98,6 @@ public class MatchQuery extends LeafQuery { MatchQuery other = (MatchQuery) obj; return Objects.equals(text, other.text) && Objects.equals(name, other.name) - && Objects.equals(operator, other.operator) && Objects.equals(predicate, other.predicate); } } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java index 5857b8553e0..57a34569a1a 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java @@ -45,7 +45,7 @@ public class SourceGeneratorTests extends ESTestCase { public void testQueryNoFilter() { QueryContainer container = new QueryContainer().with(new MatchQuery(Location.EMPTY, "foo", "bar")); SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, null, randomIntBetween(1, 10)); - assertEquals(new MatchQueryBuilder("foo", "bar").operator(Operator.AND), sourceBuilder.query()); + assertEquals(new MatchQueryBuilder("foo", "bar").operator(Operator.OR), sourceBuilder.query()); } public void testNoQueryFilter() { @@ -59,7 +59,7 @@ public class SourceGeneratorTests extends ESTestCase { QueryContainer container = new QueryContainer().with(new MatchQuery(Location.EMPTY, "foo", "bar")); QueryBuilder filter = new MatchQueryBuilder("bar", "baz"); SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, filter, randomIntBetween(1, 10)); - assertEquals(new BoolQueryBuilder().must(new MatchQueryBuilder("foo", "bar").operator(Operator.AND)) + assertEquals(new BoolQueryBuilder().must(new MatchQueryBuilder("foo", "bar").operator(Operator.OR)) .filter(new MatchQueryBuilder("bar", "baz")), sourceBuilder.query()); } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java new file mode 100644 index 00000000000..d04932c4741 --- /dev/null +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MatchQueryTests.java @@ -0,0 +1,43 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.sql.querydsl.query; + +import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.index.query.MatchQueryBuilder; +import org.elasticsearch.index.query.Operator; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate; +import org.elasticsearch.xpack.sql.tree.Location; + +import java.util.HashMap; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; + +public class MatchQueryTests extends ESTestCase { + + public void testQueryBuilding() { + MatchQueryBuilder qb = getBuilder("lenient=true"); + assertThat(qb.lenient(), equalTo(true)); + + qb = getBuilder("lenient=true;operator=AND"); + assertThat(qb.lenient(), equalTo(true)); + assertThat(qb.operator(), equalTo(Operator.AND)); + + Exception e = expectThrows(IllegalArgumentException.class, () -> getBuilder("pizza=yummy")); + assertThat(e.getMessage(), equalTo("illegal match option [pizza]")); + + e = expectThrows(IllegalArgumentException.class, () -> getBuilder("operator=aoeu")); + assertThat(e.getMessage(), equalTo("No enum constant org.elasticsearch.index.query.Operator.AOEU")); + } + + private static MatchQueryBuilder getBuilder(String options) { + final Location location = new Location(1, 1); + final MatchQueryPredicate mmqp = new MatchQueryPredicate(location, null, "eggplant", options); + final MatchQuery mmq = new MatchQuery(location, "eggplant", "foo", mmqp); + return (MatchQueryBuilder) mmq.asBuilder(); + } +}