From 4df10bbd783d3fc5e991ff9d9bdfbab732196e16 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 28 Dec 2017 17:16:55 -0500 Subject: [PATCH] SQL: Drop Node superclass from Query (elastic/x-pack-elasticsearch#3414) SQL's `Node` class is a utility for performing tree traversal and rewrites using reflection. We're not big fans of the reflection part of that we feel that, where possible, the tree traversal should be done explicitly. In this case `Query`'s tree is traversed for three things: 1. To generate the Elasticsearch `QueryBuilder`s. This uses explict tree traversal and I left it that way. 2. To ehance sorts on nested fields with the filter in the nested field. I added `enrichNestedSort` to `Query` so each query can implement this on its own. 3. To add inner hits that are not explicitly mentioned in the query already when they need to be fetched. I added `containsNestedField` and `addNestedField` to `Query` to support this. Note: The nested field code is somewhat dead because we don't support nested fields at the moment but SQL once did. I've tried to keep it mostly intact but it is difficult to know for sure if it is still functioning the same way that it has always functioned. Original commit: elastic/x-pack-elasticsearch@5d8a8d687a5198da1ed03596ea66cf6fc4771fce --- .../sql/execution/search/SourceGenerator.java | 21 +-- .../xpack/sql/planner/QueryTranslator.java | 9 +- .../querydsl/container/QueryContainer.java | 63 ++++---- .../xpack/sql/querydsl/query/AndQuery.java | 45 ------ .../xpack/sql/querydsl/query/BoolQuery.java | 108 ++++++++++++++ .../xpack/sql/querydsl/query/ExistsQuery.java | 5 + .../xpack/sql/querydsl/query/LeafQuery.java | 25 +++- .../xpack/sql/querydsl/query/MatchAll.java | 12 +- .../xpack/sql/querydsl/query/MatchQuery.java | 17 ++- .../sql/querydsl/query/MultiMatchQuery.java | 9 +- .../xpack/sql/querydsl/query/NestedQuery.java | 105 +++++++++++--- .../xpack/sql/querydsl/query/NotQuery.java | 52 ++++++- .../xpack/sql/querydsl/query/OrQuery.java | 45 ------ .../xpack/sql/querydsl/query/Query.java | 75 +++++++++- .../sql/querydsl/query/QueryStringQuery.java | 5 + .../xpack/sql/querydsl/query/RangeQuery.java | 17 ++- .../xpack/sql/querydsl/query/RegexQuery.java | 13 +- .../xpack/sql/querydsl/query/ScriptQuery.java | 9 +- .../xpack/sql/querydsl/query/TermQuery.java | 11 +- .../xpack/sql/querydsl/query/UnaryQuery.java | 24 ---- .../sql/querydsl/query/WildcardQuery.java | 13 +- .../xpack/sql/tree/Location.java | 17 +++ .../plan/logical/UnresolvedRelationTests.java | 3 +- .../container/QueryContainerTests.java | 56 ++++++++ .../sql/querydsl/query/BoolQueryTests.java | 111 ++++++++++++++ .../sql/querydsl/query/LeafQueryTests.java | 69 +++++++++ .../sql/querydsl/query/MatchQueryTests.java | 39 ++++- .../querydsl/query/MultiMatchQueryTests.java | 12 ++ .../sql/querydsl/query/NestedQueryTests.java | 135 ++++++++++++++++++ .../querydsl/query/QueryStringQueryTests.java | 10 +- .../xpack/sql/tree/LocationTests.java | 38 +++++ 31 files changed, 929 insertions(+), 244 deletions(-) delete mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/AndQuery.java create mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java delete mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/OrQuery.java delete mode 100644 sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/UnaryQuery.java create mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java create mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java create mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java create mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java create mode 100644 sql/server/src/test/java/org/elasticsearch/xpack/sql/tree/LocationTests.java diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java index 02da2cc0da2..94bbb33384d 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java @@ -161,26 +161,9 @@ public abstract class SourceGenerator { nestedSort = newSort; - List nestedQuery = new ArrayList<>(1); - - // copy also the nested queries fr(if any) if (container.query() != null) { - container.query().forEachDown(nq -> { - // found a match - if (nestedPath.equals(nq.path())) { - // get the child query - the nested wrapping and inner hits are not needed - nestedQuery.add(nq.child().asBuilder()); - } - }, NestedQuery.class); + container.query().enrichNestedSort(nestedSort); } - - if (nestedQuery.size() > 0) { - if (nestedQuery.size() > 1) { - throw new SqlIllegalArgumentException("nested query should have been grouped in one place"); - } - nestedSort.setFilter(nestedQuery.get(0)); - } - sortBuilder = fieldSort; } } @@ -207,4 +190,4 @@ public abstract class SourceGenerator { source.storedFields(NO_STORED_FIELD); } } -} \ No newline at end of file +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java index 7a2e3058880..9a6d601b6aa 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java @@ -70,13 +70,12 @@ import org.elasticsearch.xpack.sql.querydsl.agg.PercentileRanksAgg; import org.elasticsearch.xpack.sql.querydsl.agg.PercentilesAgg; import org.elasticsearch.xpack.sql.querydsl.agg.StatsAgg; import org.elasticsearch.xpack.sql.querydsl.agg.SumAgg; -import org.elasticsearch.xpack.sql.querydsl.query.AndQuery; +import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery; import org.elasticsearch.xpack.sql.querydsl.query.ExistsQuery; import org.elasticsearch.xpack.sql.querydsl.query.MatchQuery; import org.elasticsearch.xpack.sql.querydsl.query.MultiMatchQuery; import org.elasticsearch.xpack.sql.querydsl.query.NestedQuery; import org.elasticsearch.xpack.sql.querydsl.query.NotQuery; -import org.elasticsearch.xpack.sql.querydsl.query.OrQuery; import org.elasticsearch.xpack.sql.querydsl.query.Query; import org.elasticsearch.xpack.sql.querydsl.query.QueryStringQuery; import org.elasticsearch.xpack.sql.querydsl.query.RangeQuery; @@ -335,7 +334,7 @@ abstract class QueryTranslator { if (right == null) { return left; } - return new AndQuery(loc, left, right); + return new BoolQuery(loc, true, left, right); } static QueryTranslation or(Location loc, QueryTranslation left, QueryTranslation right) { @@ -376,7 +375,7 @@ abstract class QueryTranslator { if (right == null) { return left; } - return new OrQuery(loc, left, right); + return new BoolQuery(loc, false, left, right); } static Query not(Query query) { @@ -877,4 +876,4 @@ abstract class QueryTranslator { return query; } } -} \ No newline at end of file +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java index 16a7ae84437..5dc0d6358bb 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.sql.querydsl.container; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -24,11 +25,11 @@ import org.elasticsearch.xpack.sql.querydsl.agg.AggPath; import org.elasticsearch.xpack.sql.querydsl.agg.Aggs; import org.elasticsearch.xpack.sql.querydsl.agg.GroupingAgg; import org.elasticsearch.xpack.sql.querydsl.agg.LeafAgg; -import org.elasticsearch.xpack.sql.querydsl.query.AndQuery; +import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery; import org.elasticsearch.xpack.sql.querydsl.query.MatchAll; import org.elasticsearch.xpack.sql.querydsl.query.NestedQuery; import org.elasticsearch.xpack.sql.querydsl.query.Query; - +import org.elasticsearch.xpack.sql.tree.Location; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; @@ -183,44 +184,42 @@ public class QueryContainer { } private Tuple nestedFieldRef(FieldAttribute attr) { - // attach the field to the relevant nested query + // Find the nested query for this field. If there isn't one then create it List nestedRefs = new ArrayList<>(); - String parent = attr.nestedParent().name(); - String name = aliasName(attr); + Query q = rewriteToContainNestedField(query, attr.location(), + attr.nestedParent().path(), aliasName(attr), attr.dataType().hasDocValues()); - Query q = query; - Map field = singletonMap(name, attr.dataType().hasDocValues()); - if (q == null) { - q = new NestedQuery(attr.location(), parent, field, new MatchAll(attr.location())); - } - else { - AtomicBoolean foundMatch = new AtomicBoolean(false); - q = q.transformDown(n -> { - if (parent.equals(n.path())) { - if (!n.fields().keySet().contains(name)) { - foundMatch.set(true); - Map fields = new LinkedHashMap<>(n.fields()); - fields.putAll(field); - return new NestedQuery(n.location(), n.path(), fields, n.child()); - } - } - return n; - }, NestedQuery.class); - - // no nested query exists for the given field, add one to retrieve its content - if (!foundMatch.get()) { - NestedQuery nested = new NestedQuery(attr.location(), parent, field, new MatchAll(attr.location())); - q = new AndQuery(attr.location(), q, nested); - } - } - - SearchHitFieldRef nestedFieldRef = new SearchHitFieldRef(attr.name(), attr.dataType().hasDocValues(), parent); + SearchHitFieldRef nestedFieldRef = new SearchHitFieldRef(attr.name(), attr.dataType().hasDocValues(), attr.parent().name()); nestedRefs.add(nestedFieldRef); return new Tuple<>(new QueryContainer(q, aggs, columns, aliases, pseudoFunctions, scalarFunctions, sort, limit), nestedFieldRef); } + static Query rewriteToContainNestedField(@Nullable Query query, Location location, String path, String name, boolean hasDocValues) { + if (query == null) { + /* There is no query so we must add the nested query + * ourselves to fetch the field. */ + return new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)); + } + if (query.containsNestedField(path, name)) { + // The query already has the nested field. Nothing to do. + return query; + } + /* The query doesn't have the nested field so we have to ask + * it to add it. */ + Query rewritten = query.addNestedField(path, name, hasDocValues); + if (rewritten != query) { + /* It successfully added it so we can use the rewritten + * query. */ + return rewritten; + } + /* There is no nested query with a matching path so we must + * add the nested query ourselves just to fetch the field. */ + NestedQuery nested = new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)); + return new BoolQuery(location, true, query, nested); + } + // replace function's input with references private Tuple computingRef(ScalarFunctionAttribute sfa) { Attribute name = aliases.getOrDefault(sfa, sfa); diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/AndQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/AndQuery.java deleted file mode 100644 index d288e9af32f..00000000000 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/AndQuery.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * 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 java.util.Arrays; - -import org.elasticsearch.index.query.BoolQueryBuilder; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.xpack.sql.tree.Location; - -import static org.elasticsearch.index.query.QueryBuilders.boolQuery; - -public class AndQuery extends Query { - - private final Query left, right; - - public AndQuery(Location location, Query left, Query right) { - super(location, Arrays.asList(left, right)); - this.left = left; - this.right = right; - } - - public Query left() { - return left; - } - - public Query right() { - return right; - } - - @Override - public QueryBuilder asBuilder() { - BoolQueryBuilder boolQuery = boolQuery(); - if (left != null) { - boolQuery.filter(left.asBuilder()); - } - if (right != null) { - boolQuery.filter(right.asBuilder()); - } - return boolQuery; - } -} \ No newline at end of file diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java new file mode 100644 index 00000000000..64949fe318c --- /dev/null +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java @@ -0,0 +1,108 @@ +/* + * 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 java.util.Objects; + +import org.elasticsearch.index.query.BoolQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.search.sort.NestedSortBuilder; +import org.elasticsearch.xpack.sql.tree.Location; + +import static org.elasticsearch.index.query.QueryBuilders.boolQuery; + +/** + * Query representing boolean AND or boolean OR. + */ +public class BoolQuery extends Query { + /** + * {@code true} for boolean {@code AND}, {@code false} for boolean {@code OR}. + */ + private final boolean isAnd; + private final Query left; + private final Query right; + + public BoolQuery(Location location, boolean isAnd, Query left, Query right) { + super(location); + if (left == null) { + throw new IllegalArgumentException("left is required"); + } + if (right == null) { + throw new IllegalArgumentException("right is required"); + } + this.isAnd = isAnd; + this.left = left; + this.right = right; + } + + @Override + public boolean containsNestedField(String path, String field) { + return left.containsNestedField(path, field) || right.containsNestedField(path, field); + } + + @Override + public Query addNestedField(String path, String field, boolean hasDocValues) { + Query rewrittenLeft = left.addNestedField(path, field, hasDocValues); + Query rewrittenRight = right.addNestedField(path, field, hasDocValues); + if (rewrittenLeft == left && rewrittenRight == right) { + return this; + } + return new BoolQuery(location(), isAnd, rewrittenLeft, rewrittenRight); + } + + @Override + public void enrichNestedSort(NestedSortBuilder sort) { + left.enrichNestedSort(sort); + right.enrichNestedSort(sort); + } + + @Override + public QueryBuilder asBuilder() { + BoolQueryBuilder boolQuery = boolQuery(); + if (isAnd) { + // TODO are we throwing out score by using filter? + boolQuery.filter(left.asBuilder()); + boolQuery.filter(right.asBuilder()); + } else { + boolQuery.should(left.asBuilder()); + boolQuery.should(right.asBuilder()); + } + return boolQuery; + } + + boolean isAnd() { + return isAnd; + } + + Query left() { + return left; + } + + Query right() { + return right; + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), isAnd, left, right); + } + + @Override + public boolean equals(Object obj) { + if (false == super.equals(obj)) { + return false; + } + BoolQuery other = (BoolQuery) obj; + return isAnd == other.isAnd + && left.equals(other.left) + && right.equals(other.right); + } + + @Override + protected String innerToString() { + return left + (isAnd ? " AND " : " OR ") + right; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ExistsQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ExistsQuery.java index b9087186da4..aa0f39f1bec 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ExistsQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ExistsQuery.java @@ -23,4 +23,9 @@ public class ExistsQuery extends LeafQuery { public QueryBuilder asBuilder() { return existsQuery(name); } + + @Override + protected String innerToString() { + return name; + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java index 990ee2d1c98..c66de04f550 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java @@ -5,13 +5,28 @@ */ package org.elasticsearch.xpack.sql.querydsl.query; +import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.xpack.sql.tree.Location; -import static java.util.Collections.emptyList; - abstract class LeafQuery extends Query { - LeafQuery(Location location) { - super(location, emptyList()); + super(location); } -} \ No newline at end of file + + @Override + public final boolean containsNestedField(String path, String field) { + // No leaf queries are nested + return false; + } + + @Override + public Query addNestedField(String path, String field, boolean hasDocValues) { + // No leaf queries are nested + return this; + } + + @Override + public void enrichNestedSort(NestedSortBuilder sort) { + // No leaf queries are nested + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchAll.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchAll.java index 71085add799..af2801694d0 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchAll.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MatchAll.java @@ -8,18 +8,20 @@ package org.elasticsearch.xpack.sql.querydsl.query; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.xpack.sql.tree.Location; -import static java.util.Collections.emptyList; - import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; -public class MatchAll extends Query { - +public class MatchAll extends LeafQuery { public MatchAll(Location location) { - super(location, emptyList()); + super(location); } @Override public QueryBuilder asBuilder() { return matchAllQuery(); } + + @Override + protected String innerToString() { + return ""; + } } 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 0f884f7d0cf..292a8b9a8e3 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 @@ -80,6 +80,10 @@ public class MatchQuery extends LeafQuery { return text; } + MatchQueryPredicate predicate() { + return predicate; + } + @Override public int hashCode() { return Objects.hash(text, name, predicate); @@ -87,17 +91,18 @@ public class MatchQuery extends LeafQuery { @Override public boolean equals(Object obj) { - if (this == obj) { - return true; - } - - if (obj == null || getClass() != obj.getClass()) { + if (false == super.equals(obj)) { return false; } - + MatchQuery other = (MatchQuery) obj; return Objects.equals(text, other.text) && Objects.equals(name, other.name) && Objects.equals(predicate, other.predicate); } + + @Override + protected String innerToString() { + return name + ":" + text; + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java index b6f14c3be1a..81c990f85bd 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQuery.java @@ -83,14 +83,19 @@ public class MultiMatchQuery extends LeafQuery { if (this == obj) { return true; } - + if (obj == null || getClass() != obj.getClass()) { return false; } - + MultiMatchQuery other = (MultiMatchQuery) obj; return Objects.equals(query, other.query) && Objects.equals(fields, other.fields) && Objects.equals(predicate, other.predicate); } + + @Override + protected String innerToString() { + return fields + ":" + query; + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java index e2e1d1f0f74..03af1433f30 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.querydsl.query; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -17,45 +18,92 @@ import org.elasticsearch.index.query.NestedQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; +import org.elasticsearch.search.sort.NestedSortBuilder; +import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.tree.Location; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; +import static java.util.Collections.unmodifiableMap; import static org.elasticsearch.index.query.QueryBuilders.nestedQuery; -public class NestedQuery extends UnaryQuery { - +/** + * A query to a nested document. + */ +public class NestedQuery extends Query { // TODO: make this configurable private static final int MAX_INNER_HITS = 99; private static final List NO_STORED_FIELD = singletonList(StoredFieldsContext._NONE_); private final String path; private final Map fields; + private final Query child; public NestedQuery(Location location, String path, Query child) { this(location, path, emptyMap(), child); } public NestedQuery(Location location, String path, Map fields, Query child) { - super(location, child); + super(location); + if (path == null) { + throw new IllegalArgumentException("path is required"); + } + if (fields == null) { + throw new IllegalArgumentException("fields is required"); + } + if (child == null) { + throw new IllegalArgumentException("child is required"); + } this.path = path; this.fields = fields; + this.child = child; } - public String path() { - return path; + @Override + public boolean containsNestedField(String path, String field) { + boolean iContainThisField = this.path.equals(path) && fields.containsKey(field); + boolean myChildContainsThisField = child.containsNestedField(path, field); + return iContainThisField || myChildContainsThisField; } - public Map fields() { - return fields; + @Override + public Query addNestedField(String path, String field, boolean hasDocValues) { + if (false == this.path.equals(path)) { + // I'm not at the right path so let my child query have a crack at it + Query rewrittenChild = child.addNestedField(path, field, hasDocValues); + if (rewrittenChild == child) { + return this; + } + return new NestedQuery(location(), path, fields, rewrittenChild); + } + if (fields.containsKey(field)) { + // I already have the field, no rewriting needed + return this; + } + Map newFields = new HashMap<>(fields.size() + 1); + newFields.putAll(fields); + newFields.put(field, hasDocValues); + return new NestedQuery(location(), path, unmodifiableMap(newFields), child); + } + + @Override + public void enrichNestedSort(NestedSortBuilder sort) { + child.enrichNestedSort(sort); + if (false == sort.getPath().equals(path)) { + return; + } + QueryBuilder childAsBuilder = child.asBuilder(); + if (sort.getFilter() != null && false == sort.getFilter().equals(childAsBuilder)) { + throw new SqlIllegalArgumentException("nested query should have been grouped in one place"); + } + sort.setFilter(childAsBuilder); } @Override public QueryBuilder asBuilder() { - // disable source - - NestedQueryBuilder query = nestedQuery(path, child().asBuilder(), ScoreMode.None); + // disable score + NestedQueryBuilder query = nestedQuery(path, child.asBuilder(), ScoreMode.None); if (!fields.isEmpty()) { InnerHitBuilder ihb = new InnerHitBuilder(); @@ -83,31 +131,42 @@ public class NestedQuery extends UnaryQuery { ihb.setFetchSourceContext(new FetchSourceContext(true, sourceFields.toArray(new String[sourceFields.size()]), null)); } - query.innerHit(ihb); } return query; } - + + String path() { + return path; + } + + Map fields() { + return fields; + } + + Query child() { + return child; + } + @Override public int hashCode() { - return Objects.hash(path, fields, child()); + return Objects.hash(super.hashCode(), path, fields, child); } @Override public boolean equals(Object obj) { - if (this == obj) { - return true; - } - - if (obj == null || getClass() != obj.getClass()) { + if (false == super.equals(obj)) { return false; } - NestedQuery other = (NestedQuery) obj; - return Objects.equals(path, other.path) - && Objects.equals(fields, other.fields) - && Objects.equals(child(), other.child()); + return path.equals(other.path) + && fields.equals(other.fields) + && child.equals(other.child); } -} \ No newline at end of file + + @Override + protected String innerToString() { + return path + "." + fields + "[" + child + "]"; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java index fce28a177c6..b3d50b8149a 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java @@ -6,18 +6,64 @@ package org.elasticsearch.xpack.sql.querydsl.query; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.xpack.sql.tree.Location; import static org.elasticsearch.index.query.QueryBuilders.boolQuery; -public class NotQuery extends UnaryQuery { +import java.util.Objects; + +public class NotQuery extends Query { + private final Query child; public NotQuery(Location location, Query child) { - super(location, child); + super(location); + if (child == null) { + throw new IllegalArgumentException("child is required"); + } + this.child = child; + } + + @Override + public boolean containsNestedField(String path, String field) { + return child.containsNestedField(path, field); + } + + @Override + public Query addNestedField(String path, String field, boolean hasDocValues) { + Query rewrittenChild = child.addNestedField(path, field, hasDocValues); + if (child == rewrittenChild) { + return this; + } + return new NotQuery(location(), child); + } + + @Override + public void enrichNestedSort(NestedSortBuilder sort) { + child.enrichNestedSort(sort); } @Override public QueryBuilder asBuilder() { - return boolQuery().mustNot(child().asBuilder()); + return boolQuery().mustNot(child.asBuilder()); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), child.hashCode()); + } + + @Override + public boolean equals(Object obj) { + if (false == super.equals(obj)) { + return false; + } + NotQuery other = (NotQuery) obj; + return child.equals(other.child); + } + + @Override + protected String innerToString() { + return child.toString(); } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/OrQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/OrQuery.java deleted file mode 100644 index 21e079d76a7..00000000000 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/OrQuery.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * 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 java.util.Arrays; - -import org.elasticsearch.index.query.BoolQueryBuilder; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.xpack.sql.tree.Location; - -import static org.elasticsearch.index.query.QueryBuilders.boolQuery; - -public class OrQuery extends Query { - - private final Query left, right; - - public OrQuery(Location location, Query left, Query right) { - super(location, Arrays.asList(left, right)); - this.left = left; - this.right = right; - } - - public Query left() { - return left; - } - - public Query right() { - return right; - } - - @Override - public QueryBuilder asBuilder() { - BoolQueryBuilder boolQuery = boolQuery(); - if (left != null) { - boolQuery.should(left.asBuilder()); - } - if (right != null) { - boolQuery.should(right.asBuilder()); - } - return boolQuery; - } -} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java index edaef5f1450..057454b6fd8 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java @@ -5,17 +5,80 @@ */ package org.elasticsearch.xpack.sql.querydsl.query; -import java.util.List; - import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.search.sort.NestedSortBuilder; import org.elasticsearch.xpack.sql.tree.Location; -import org.elasticsearch.xpack.sql.tree.Node; -public abstract class Query extends Node { +/** + * Intermediate representation of queries that is rewritten to fetch + * otherwise unreferenced nested fields and then used to build + * Elasticsearch {@link QueryBuilder}s. + */ +public abstract class Query { + private final Location location; - Query(Location location, List children) { - super(location, children); + Query(Location location) { + if (location == null) { + throw new IllegalArgumentException("location must be specified"); + } + this.location = location; } + /** + * Location in the source statement. + */ + public Location location() { + return location; + } + + /** + * Does this query contain a particular nested field? + */ + public abstract boolean containsNestedField(String path, String field); + + /** + * Rewrite this query to one that contains the specified nested field. + *

+ * Used to make sure that we fetch nested fields even if they aren't + * explicitly part of the query. + * @return a new query if we could add the nested field, the same query + * instance otherwise + */ + public abstract Query addNestedField(String path, String field, boolean hasDocValues); + + /** + * Attach the one and only one matching nested query's filter to this + * sort. + */ + public abstract void enrichNestedSort(NestedSortBuilder sort); + + /** + * Convert to an Elasticsearch {@link QueryBuilder} all set up to execute + * the query. + */ public abstract QueryBuilder asBuilder(); + + /** + * Used by {@link Query#toString()} to produce a pretty string. + */ + protected abstract String innerToString(); + + @Override + public boolean equals(Object obj) { + if (obj == null || obj.getClass() != getClass()) { + return false; + } + Query other = (Query) obj; + return location.equals(other.location); + } + + @Override + public int hashCode() { + return location.hashCode(); + } + + @Override + public String toString() { + return getClass().getSimpleName() + location + "[" + innerToString() + "]"; + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQuery.java index fadb548fe91..9c0e814db78 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQuery.java @@ -118,4 +118,9 @@ public class QueryStringQuery extends LeafQuery { && Objects.equals(fields, other.fields) && Objects.equals(predicate, other.predicate); } + + @Override + protected String innerToString() { + return fields + ":" + query; + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RangeQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RangeQuery.java index b824c31808f..5f97ca529a1 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RangeQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RangeQuery.java @@ -38,7 +38,7 @@ public class RangeQuery extends LeafQuery { public String field() { return field; } - + public Object lower() { return lower; } @@ -54,7 +54,7 @@ public class RangeQuery extends LeafQuery { public boolean includeUpper() { return includeUpper; } - + public String format() { return format; } @@ -79,11 +79,11 @@ public class RangeQuery extends LeafQuery { if (this == obj) { return true; } - + if (obj == null || getClass() != obj.getClass()) { return false; } - + RangeQuery other = (RangeQuery) obj; return Objects.equals(field, other.field) && Objects.equals(includeLower, other.includeLower) && @@ -92,4 +92,11 @@ public class RangeQuery extends LeafQuery { Objects.equals(upper, other.upper) && Objects.equals(format, other.format); } -} \ No newline at end of file + + @Override + protected String innerToString() { + return field + ":" + + (includeLower ? "[" : "(") + lower + ", " + + upper + (includeUpper ? "]" : ")"); + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RegexQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RegexQuery.java index 16a7b53d5c1..bf3364388a9 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RegexQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/RegexQuery.java @@ -39,19 +39,24 @@ public class RegexQuery extends LeafQuery { public int hashCode() { return Objects.hash(field, regex); } - + @Override public boolean equals(Object obj) { if (this == obj) { return true; } - + if (obj == null || getClass() != obj.getClass()) { return false; } - + RegexQuery other = (RegexQuery) obj; return Objects.equals(field, other.field) && Objects.equals(regex, other.regex); } -} \ No newline at end of file + + @Override + protected String innerToString() { + return field + "~ /" + regex + "/"; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ScriptQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ScriptQuery.java index d8f1c37f9b5..b918fd71a58 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ScriptQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/ScriptQuery.java @@ -41,12 +41,17 @@ public class ScriptQuery extends LeafQuery { if (this == obj) { return true; } - + if (obj == null || getClass() != obj.getClass()) { return false; } - + ScriptQuery other = (ScriptQuery) obj; return Objects.equals(script, other.script); } + + @Override + protected String innerToString() { + return script.toString(); + } } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermQuery.java index 2774624dfe3..af14272dff5 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/TermQuery.java @@ -46,13 +46,18 @@ public class TermQuery extends LeafQuery { if (this == obj) { return true; } - + if (obj == null || getClass() != obj.getClass()) { return false; } - + TermQuery other = (TermQuery) obj; return Objects.equals(term, other.term) && Objects.equals(value, other.value); } -} \ No newline at end of file + + @Override + protected String innerToString() { + return term + ":" + value; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/UnaryQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/UnaryQuery.java deleted file mode 100644 index 0c48f3a6236..00000000000 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/UnaryQuery.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * 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.xpack.sql.tree.Location; - -import static java.util.Collections.singletonList; - -abstract class UnaryQuery extends Query { - - private final Query child; - - UnaryQuery(Location location, Query child) { - super(location, singletonList(child)); - this.child = child; - } - - public Query child() { - return child; - } -} \ No newline at end of file diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/WildcardQuery.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/WildcardQuery.java index 95bf6e0b8bc..6c252450c6f 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/WildcardQuery.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/WildcardQuery.java @@ -39,19 +39,24 @@ public class WildcardQuery extends LeafQuery { public int hashCode() { return Objects.hash(field, query); } - + @Override public boolean equals(Object obj) { if (this == obj) { return true; } - + if (obj == null || getClass() != obj.getClass()) { return false; } - + WildcardQuery other = (WildcardQuery) obj; return Objects.equals(field, other.field) && Objects.equals(query, other.query); } -} \ No newline at end of file + + @Override + protected String innerToString() { + return field + ":" + query; + } +} diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/tree/Location.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/tree/Location.java index 17989a5a612..6a5543a69db 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/tree/Location.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/tree/Location.java @@ -5,6 +5,8 @@ */ package org.elasticsearch.xpack.sql.tree; +import java.util.Objects; + public final class Location { private final int line; private final int charPositionInLine; @@ -28,4 +30,19 @@ public final class Location { public String toString() { return "@" + getLineNumber() + ":" + getColumnNumber(); } + + @Override + public int hashCode() { + return Objects.hash(line, charPositionInLine); + } + + @Override + public boolean equals(Object obj) { + if (obj == null || obj.getClass() != getClass()) { + return false; + } + Location other = (Location) obj; + return line == other.line + && charPositionInLine == other.charPositionInLine; + } } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plan/logical/UnresolvedRelationTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plan/logical/UnresolvedRelationTests.java index 5dfe5ffd607..cb2210b818d 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plan/logical/UnresolvedRelationTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plan/logical/UnresolvedRelationTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.plan.logical; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.sql.plan.TableIdentifier; import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.tree.LocationTests; import java.util.ArrayList; import java.util.List; @@ -25,7 +26,7 @@ public class UnresolvedRelationTests extends ESTestCase { UnresolvedRelation relation = new UnresolvedRelation(location, table, alias, unresolvedMessage); List> mutators = new ArrayList<>(); mutators.add(r -> new UnresolvedRelation( - new Location(r.location().getLineNumber() + 1, r.location().getColumnNumber()), + LocationTests.mutate(r.location()), r.table(), r.alias(), r.unresolvedMessage())); diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java new file mode 100644 index 00000000000..f943e325cf7 --- /dev/null +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java @@ -0,0 +1,56 @@ +/* + * 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.container; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery; +import org.elasticsearch.xpack.sql.querydsl.query.MatchAll; +import org.elasticsearch.xpack.sql.querydsl.query.NestedQuery; +import org.elasticsearch.xpack.sql.querydsl.query.Query; +import org.elasticsearch.xpack.sql.querydsl.query.RangeQuery; +import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.tree.LocationTests; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonMap; + +public class QueryContainerTests extends ESTestCase { + private Location location = LocationTests.randomLocation(); + private String path = randomAlphaOfLength(5); + private String name = randomAlphaOfLength(5); + private boolean hasDocValues = randomBoolean(); + + public void testRewriteToContainNestedFieldNoQuery() { + Query expected = new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)); + assertEquals(expected, QueryContainer.rewriteToContainNestedField(null, location, path, name, hasDocValues)); + } + + public void testRewriteToContainsNestedFieldWhenContainsNestedField() { + Query original = new BoolQuery(location, true, + new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)), + new RangeQuery(location, randomAlphaOfLength(5), 0, randomBoolean(), 100, randomBoolean())); + assertSame(original, QueryContainer.rewriteToContainNestedField(original, location, path, name, randomBoolean())); + } + + public void testRewriteToContainsNestedFieldWhenCanAddNestedField() { + Query buddy = new RangeQuery(location, randomAlphaOfLength(5), 0, randomBoolean(), 100, randomBoolean()); + Query original = new BoolQuery(location, true, + new NestedQuery(location, path, emptyMap(), new MatchAll(location)), + buddy); + Query expected = new BoolQuery(location, true, + new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)), + buddy); + assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, hasDocValues)); + } + + public void testRewriteToContainsNestedFieldWhenDoesNotContainNestedFieldAndCantAdd() { + Query original = new RangeQuery(location, randomAlphaOfLength(5), 0, randomBoolean(), 100, randomBoolean()); + Query expected = new BoolQuery(location, true, + original, + new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location))); + assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, hasDocValues)); + } +} diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java new file mode 100644 index 00000000000..901709479d3 --- /dev/null +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java @@ -0,0 +1,111 @@ +/* + * 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.search.sort.NestedSortBuilder; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.tree.LocationTests; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.function.Supplier; + +import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; +import static org.hamcrest.Matchers.hasEntry; +import static java.util.Collections.singletonMap; + +public class BoolQueryTests extends ESTestCase { + static BoolQuery randomBoolQuery(int depth) { + return new BoolQuery(LocationTests.randomLocation(), randomBoolean(), + NestedQueryTests.randomQuery(depth), NestedQueryTests.randomQuery(depth)); + } + + public void testEqualsAndHashCode() { + checkEqualsAndHashCode(randomBoolQuery(5), BoolQueryTests::copy, BoolQueryTests::mutate); + } + + private static BoolQuery copy(BoolQuery query) { + return new BoolQuery(query.location(), query.isAnd(), query.left(), query.right()); + } + + private static BoolQuery mutate(BoolQuery query) { + List> options = Arrays.asList( + q -> new BoolQuery(LocationTests.mutate(q.location()), q.isAnd(), q.left(), q.right()), + q -> new BoolQuery(q.location(), false == q.isAnd(), q.left(), q.right()), + q -> new BoolQuery(q.location(), q.isAnd(), randomValueOtherThan(q.left(), () -> NestedQueryTests.randomQuery(5)), q.right()), + q -> new BoolQuery(q.location(), q.isAnd(), q.left(), randomValueOtherThan(q.right(), () -> NestedQueryTests.randomQuery(5)))); + return randomFrom(options).apply(query); + } + + public void testContainsNestedField() { + assertFalse(boolQueryWithoutNestedChildren().containsNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5))); + + String path = randomAlphaOfLength(5); + String field = randomAlphaOfLength(5); + assertTrue(boolQueryWithNestedChildren(path, field).containsNestedField(path, field)); + } + + public void testAddNestedField() { + Query q = boolQueryWithoutNestedChildren(); + assertSame(q, q.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), randomBoolean())); + + String path = randomAlphaOfLength(5); + String field = randomAlphaOfLength(5); + q = boolQueryWithNestedChildren(path, field); + String newField = randomAlphaOfLength(5); + boolean hasDocValues = randomBoolean(); + Query rewritten = q.addNestedField(path, newField, hasDocValues); + assertNotSame(q, rewritten); + assertTrue(rewritten.containsNestedField(path, newField)); + } + + public void testEnrichNestedSort() { + Query q = boolQueryWithoutNestedChildren(); + NestedSortBuilder sort = new NestedSortBuilder(randomAlphaOfLength(5)); + q.enrichNestedSort(sort); + assertNull(sort.getFilter()); + + String path = randomAlphaOfLength(5); + String field = randomAlphaOfLength(5); + q = boolQueryWithNestedChildren(path, field); + sort = new NestedSortBuilder(path); + q.enrichNestedSort(sort); + assertNotNull(sort.getFilter()); + } + + private Query boolQueryWithoutNestedChildren() { + return new BoolQuery(LocationTests.randomLocation(), randomBoolean(), + new MatchAll(LocationTests.randomLocation()), new MatchAll(LocationTests.randomLocation())); + } + + private Query boolQueryWithNestedChildren(String path, String field) { + NestedQuery match = new NestedQuery(LocationTests.randomLocation(), path, + singletonMap(field, randomBoolean()), new MatchAll(LocationTests.randomLocation())); + Query matchAll = new MatchAll(LocationTests.randomLocation()); + Query left; + Query right; + if (randomBoolean()) { + left = match; + right = matchAll; + } else { + left = matchAll; + right = match; + } + return new BoolQuery(LocationTests.randomLocation(), randomBoolean(), left, right); + } + + public void testToString() { + assertEquals("BoolQuery@1:2[ExistsQuery@1:2[f1] AND ExistsQuery@1:8[f2]]", + new BoolQuery(new Location(1, 1), true, + new ExistsQuery(new Location(1, 1), "f1"), + new ExistsQuery(new Location(1, 7), "f2")).toString()); + } +} diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java new file mode 100644 index 00000000000..232040b79bc --- /dev/null +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java @@ -0,0 +1,69 @@ +/* + * 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.index.query.QueryBuilder; +import org.elasticsearch.search.sort.NestedSortBuilder; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.tree.LocationTests; + +import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; + +public class LeafQueryTests extends ESTestCase { + private static class DummyLeafQuery extends LeafQuery { + private DummyLeafQuery(Location location) { + super(location); + } + + @Override + public QueryBuilder asBuilder() { + return null; + } + + @Override + protected String innerToString() { + return ""; + } + } + + public void testEqualsAndHashCode() { + DummyLeafQuery query = new DummyLeafQuery(LocationTests.randomLocation()); + checkEqualsAndHashCode(query, LeafQueryTests::copy, LeafQueryTests::mutate); + } + + private static DummyLeafQuery copy(DummyLeafQuery query) { + return new DummyLeafQuery(query.location()); + } + + private static DummyLeafQuery mutate(DummyLeafQuery query) { + return new DummyLeafQuery(LocationTests.mutate(query.location())); + } + + public void testContainsNestedField() { + Query query = new DummyLeafQuery(LocationTests.randomLocation()); + // Leaf queries don't contain nested fields. + assertFalse(query.containsNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5))); + } + + public void testAddNestedField() { + Query query = new DummyLeafQuery(LocationTests.randomLocation()); + // Leaf queries don't contain nested fields. + assertSame(query, query.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), randomBoolean())); + } + + public void testEnrichNestedSort() { + Query query = new DummyLeafQuery(LocationTests.randomLocation()); + // Leaf queries don't contain nested fields. + NestedSortBuilder sort = new NestedSortBuilder(randomAlphaOfLength(5)); + query.enrichNestedSort(sort); + assertNull(sort.getFilter()); + } + + public void testToString() { + assertEquals("DummyLeafQuery@1:2[]", new DummyLeafQuery(new Location(1, 1)).toString()); + } +} 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 index d04932c4741..431a6a146ae 100644 --- 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 @@ -5,19 +5,45 @@ */ 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 org.elasticsearch.xpack.sql.tree.LocationTests; -import java.util.HashMap; -import java.util.Map; +import java.util.Arrays; +import java.util.List; +import java.util.function.Function; import static org.hamcrest.Matchers.equalTo; +import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; public class MatchQueryTests extends ESTestCase { + static MatchQuery randomMatchQuery() { + return new MatchQuery( + LocationTests.randomLocation(), + randomAlphaOfLength(5), + randomAlphaOfLength(5)); + // TODO add the predicate + } + + public void testEqualsAndHashCode() { + checkEqualsAndHashCode(randomMatchQuery(), MatchQueryTests::copy, MatchQueryTests::mutate); + } + + private static MatchQuery copy(MatchQuery query) { + return new MatchQuery(query.location(), query.name(), query.text(), query.predicate()); + } + + private static MatchQuery mutate(MatchQuery query) { + List> options = Arrays.asList( + q -> new MatchQuery(LocationTests.mutate(q.location()), q.name(), q.text(), q.predicate()), + q -> new MatchQuery(q.location(), randomValueOtherThan(q.name(), () -> randomAlphaOfLength(5)), q.text(), q.predicate()), + q -> new MatchQuery(q.location(), q.name(), randomValueOtherThan(q.text(), () -> randomAlphaOfLength(5)), q.predicate())); + // TODO mutate the predicate + return randomFrom(options).apply(query); + } public void testQueryBuilding() { MatchQueryBuilder qb = getBuilder("lenient=true"); @@ -40,4 +66,11 @@ public class MatchQueryTests extends ESTestCase { final MatchQuery mmq = new MatchQuery(location, "eggplant", "foo", mmqp); return (MatchQueryBuilder) mmq.asBuilder(); } + + public void testToString() { + final Location location = new Location(1, 1); + final MatchQueryPredicate mmqp = new MatchQueryPredicate(location, null, "eggplant", ""); + final MatchQuery mmq = new MatchQuery(location, "eggplant", "foo", mmqp); + assertEquals("MatchQuery@1:2[eggplant:foo]", mmq.toString()); + } } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java index c575b091830..ba2d548cde9 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/MultiMatchQueryTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.xpack.sql.tree.Location; import java.util.HashMap; import java.util.Map; +import java.util.TreeMap; import static org.hamcrest.Matchers.equalTo; @@ -42,4 +43,15 @@ public class MultiMatchQueryTests extends ESTestCase { final MultiMatchQuery mmq = new MultiMatchQuery(location, "eggplant", fields, mmqp); return (MultiMatchQueryBuilder) mmq.asBuilder(); } + + public void testToString() { + final Location location = new Location(1, 1); + final MultiMatchQueryPredicate mmqp = new MultiMatchQueryPredicate(location, "foo,bar", "eggplant", ""); + // Use a TreeMap so we get the fields in a predictable order. + final Map fields = new TreeMap<>(); + fields.put("foo", 1.0f); + fields.put("bar", 1.0f); + final MultiMatchQuery mmq = new MultiMatchQuery(location, "eggplant", fields, mmqp); + assertEquals("MultiMatchQuery@1:2[{bar=1.0, foo=1.0}:eggplant]", mmq.toString()); + } } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java new file mode 100644 index 00000000000..7cd53bc73de --- /dev/null +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java @@ -0,0 +1,135 @@ +/* + * 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.search.sort.NestedSortBuilder; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; +import org.elasticsearch.xpack.sql.tree.Location; +import org.elasticsearch.xpack.sql.tree.LocationTests; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.function.Supplier; + +import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; +import static org.hamcrest.Matchers.hasEntry; +import static java.util.Collections.singletonMap; + +public class NestedQueryTests extends ESTestCase { + static Query randomQuery(int depth) { + List> options = new ArrayList<>(); + options.add(MatchQueryTests::randomMatchQuery); + if (depth > 0) { + options.add(() -> randomNestedQuery(depth - 1)); + options.add(() -> BoolQueryTests.randomBoolQuery(depth - 1)); + } + return randomFrom(options).get(); + } + + static NestedQuery randomNestedQuery(int depth) { + return new NestedQuery(LocationTests.randomLocation(), randomAlphaOfLength(5), randomFields(), randomQuery(depth)); + } + + private static Map randomFields() { + int size = between(0, 5); + Map fields = new HashMap<>(size); + while (fields.size() < size) { + fields.put(randomAlphaOfLength(5), randomBoolean()); + } + return fields; + } + + public void testEqualsAndHashCode() { + checkEqualsAndHashCode(randomNestedQuery(5), NestedQueryTests::copy, NestedQueryTests::mutate); + } + + private static NestedQuery copy(NestedQuery query) { + return new NestedQuery(query.location(), query.path(), query.fields(), query.child()); + } + + private static NestedQuery mutate(NestedQuery query) { + List> options = Arrays.asList( + q -> new NestedQuery(LocationTests.mutate(q.location()), q.path(), q.fields(), q.child()), + q -> new NestedQuery(q.location(), randomValueOtherThan(q.path(), () -> randomAlphaOfLength(5)), q.fields(), q.child()), + q -> new NestedQuery(q.location(), q.path(), randomValueOtherThan(q.fields(), NestedQueryTests::randomFields), q.child()), + q -> new NestedQuery(q.location(), q.path(), q.fields(), randomValueOtherThan(q.child(), () -> randomQuery(5)))); + return randomFrom(options).apply(query); + } + + public void testContainsNestedField() { + NestedQuery q = randomNestedQuery(0); + for (String field : q.fields().keySet()) { + assertTrue(q.containsNestedField(q.path(), field)); + assertFalse(q.containsNestedField(randomValueOtherThan(q.path(), () -> randomAlphaOfLength(5)), field)); + } + assertFalse(q.containsNestedField(q.path(), randomValueOtherThanMany(q.fields()::containsKey, () -> randomAlphaOfLength(5)))); + } + + public void testAddNestedField() { + NestedQuery q = randomNestedQuery(0); + for (String field : q.fields().keySet()) { + // add does nothing if the field is already there + assertSame(q, q.addNestedField(q.path(), field, randomBoolean())); + String otherPath = randomValueOtherThan(q.path(), () -> randomAlphaOfLength(5)); + // add does nothing if the path doesn't match + assertSame(q, q.addNestedField(otherPath, randomAlphaOfLength(5), randomBoolean())); + } + + // if the field isn't in the list then add rewrites to a query with all the old fields and the new one + String newField = randomValueOtherThanMany(q.fields()::containsKey, () -> randomAlphaOfLength(5)); + boolean hasDocValues = randomBoolean(); + NestedQuery added = (NestedQuery) q.addNestedField(q.path(), newField, hasDocValues); + assertNotSame(q, added); + assertThat(added.fields(), hasEntry(newField, hasDocValues)); + assertTrue(added.containsNestedField(q.path(), newField)); + for (Map.Entry field : q.fields().entrySet()) { + assertThat(added.fields(), hasEntry(field.getKey(), field.getValue())); + assertTrue(added.containsNestedField(q.path(), field.getKey())); + } + } + + public void testEnrichNestedSort() { + NestedQuery q = randomNestedQuery(0); + + // enrich adds the filter if the path matches + { + NestedSortBuilder sort = new NestedSortBuilder(q.path()); + q.enrichNestedSort(sort); + assertEquals(q.child().asBuilder(), sort.getFilter()); + } + + // but doesn't if it doesn't match + { + NestedSortBuilder sort = new NestedSortBuilder(randomValueOtherThan(q.path(), () -> randomAlphaOfLength(5))); + q.enrichNestedSort(sort); + assertNull(sort.getFilter()); + } + + // enriching with the same query twice is fine + { + NestedSortBuilder sort = new NestedSortBuilder(q.path()); + q.enrichNestedSort(sort); + assertEquals(q.child().asBuilder(), sort.getFilter()); + q.enrichNestedSort(sort); + + // But enriching using another query is not + NestedQuery other = new NestedQuery(LocationTests.randomLocation(), q.path(), q.fields(), + randomValueOtherThan(q.child(), () -> randomQuery(0))); + Exception e = expectThrows(SqlIllegalArgumentException.class, () -> other.enrichNestedSort(sort)); + assertEquals("nested query should have been grouped in one place", e.getMessage()); + } + } + + public void testToString() { + NestedQuery q = new NestedQuery(new Location(1, 1), "a.b", singletonMap("f", true), new MatchAll(new Location(1, 1))); + assertEquals("NestedQuery@1:2[a.b.{f=true}[MatchAll@1:2[]]]", q.toString()); + } +} diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQueryTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQueryTests.java index 136ea3a676c..229a4392ed2 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQueryTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/QueryStringQueryTests.java @@ -13,8 +13,6 @@ import org.elasticsearch.xpack.sql.expression.predicate.fulltext.StringQueryPred import org.elasticsearch.xpack.sql.tree.Location; import java.util.Collections; -import java.util.HashMap; -import java.util.Map; import static org.hamcrest.Matchers.equalTo; @@ -41,4 +39,12 @@ public class QueryStringQueryTests extends ESTestCase { final QueryStringQuery mmq = new QueryStringQuery(location, "eggplant", Collections.singletonMap("foo", 1.0f), mmqp); return (QueryStringQueryBuilder) mmq.asBuilder(); } + + + public void testToString() { + final Location location = new Location(1, 1); + final StringQueryPredicate mmqp = new StringQueryPredicate(location, "eggplant", ""); + final QueryStringQuery mmq = new QueryStringQuery(location, "eggplant", Collections.singletonMap("foo", 1.0f), mmqp); + assertEquals("QueryStringQuery@1:2[{foo=1.0}:eggplant]", mmq.toString()); + } } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/tree/LocationTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/tree/LocationTests.java new file mode 100644 index 00000000000..45c429d0402 --- /dev/null +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/tree/LocationTests.java @@ -0,0 +1,38 @@ +/* + * 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.tree; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.EqualsHashCodeTestUtils.MutateFunction; + +import java.util.Arrays; +import java.util.List; +import java.util.function.Function; + +import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; + +public class LocationTests extends ESTestCase { + public static Location randomLocation() { + return new Location(between(1, Integer.MAX_VALUE), between(1, Integer.MAX_VALUE)); + } + + public static Location mutate(Location location) { + List> options = Arrays.asList( + l -> new Location( + randomValueOtherThan(l.getLineNumber(), () -> between(1, Integer.MAX_VALUE)), + l.getColumnNumber() - 1), + l -> new Location( + l.getLineNumber(), + randomValueOtherThan(l.getColumnNumber() - 1, () -> between(1, Integer.MAX_VALUE)))); + return randomFrom(options).apply(location); + } + + public void testEqualsAndHashCode() { + checkEqualsAndHashCode(randomLocation(), + l -> new Location(l.getLineNumber(), l.getColumnNumber() - 1), + LocationTests::mutate); + } +}