honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Propagate a doc-value `format` with inner hits. (#36068) 

While sql passes a doc-value `format` when asking for doc-value fields, it
doesn't do it when asking for fields via inner hits.
This commit is contained in:
Adrien Grand 2018-12-05 14:06:32 +01:00 committed by GitHub
parent 37127c19ba
commit 46209591fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 70 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java
View File

@ -11,6 +11,7 @@ import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.execution.search.FieldExtraction;
import org.elasticsearch.xpack.sql.execution.search.SourceGenerator;
@ -29,8 +30,10 @@ 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 org.elasticsearch.xpack.sql.type.DataType;
import java.io.IOException;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashMap;
@ -180,8 +183,9 @@ public class QueryContainer {
List<FieldExtraction> nestedRefs = new ArrayList<>();
String name = aliasName(attr);
String format = attr.field().getDataType() == DataType.DATE ? "epoch_millis" : DocValueFieldsContext.USE_DEFAULT_FORMAT;
Query q = rewriteToContainNestedField(query, attr.location(),
attr.nestedParent().name(), name, attr.field().isAggregatable());
attr.nestedParent().name(), name, format, attr.field().isAggregatable());
SearchHitFieldRef nestedFieldRef = new SearchHitFieldRef(name, attr.field().getDataType(),
attr.field().isAggregatable(), attr.parent().name());
@ -190,11 +194,13 @@ public class QueryContainer {
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) {
static Query rewriteToContainNestedField(@Nullable Query query, Location location, String path, String name, String format,
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));
return new NestedQuery(location, path, singletonMap(name, new AbstractMap.SimpleImmutableEntry<>(hasDocValues, format)),
new MatchAll(location));
}
if (query.containsNestedField(path, name)) {
// The query already has the nested field. Nothing to do.
@ -202,7 +208,7 @@ public class QueryContainer {
}
/* The query doesn't have the nested field so we have to ask
* it to add it. */
Query rewritten = query.addNestedField(path, name, hasDocValues);
Query rewritten = query.addNestedField(path, name, format, hasDocValues);
if (rewritten != query) {
/* It successfully added it so we can use the rewritten
* query. */
@ -210,7 +216,8 @@ public class QueryContainer {
}
/* 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));
NestedQuery nested = new NestedQuery(location, path,
singletonMap(name, new AbstractMap.SimpleImmutableEntry<>(hasDocValues, format)), new MatchAll(location));
return new BoolQuery(location, true, query, nested);
}

6
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java
View File

@ -44,9 +44,9 @@ public class BoolQuery extends Query {
}
@Override
public Query addNestedField(String path, String field, boolean hasDocValues) {
Query rewrittenLeft = left.addNestedField(path, field, hasDocValues);
Query rewrittenRight = right.addNestedField(path, field, hasDocValues);
public Query addNestedField(String path, String field, String format, boolean hasDocValues) {
Query rewrittenLeft = left.addNestedField(path, field, format, hasDocValues);
Query rewrittenRight = right.addNestedField(path, field, format, hasDocValues);
if (rewrittenLeft == left && rewrittenRight == right) {
return this;
}

2
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQuery.java
View File

@ -20,7 +20,7 @@ abstract class LeafQuery extends Query {
}
@Override
public Query addNestedField(String path, String field, boolean hasDocValues) {
public Query addNestedField(String path, String field, String format, boolean hasDocValues) {
// No leaf queries are nested
return this;
}

22
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQuery.java
View File

@ -5,11 +5,11 @@
*/
package org.elasticsearch.xpack.sql.querydsl.query;
import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import org.apache.lucene.search.join.ScoreMode;
@ -37,14 +37,14 @@ public class NestedQuery extends Query {
private static final List<String> NO_STORED_FIELD = singletonList(StoredFieldsContext._NONE_);
private final String path;
private final Map<String, Boolean> fields;
private final Map<String, Map.Entry<Boolean, String>> fields; // field -> (useDocValues, format)
private final Query child;
public NestedQuery(Location location, String path, Query child) {
this(location, path, emptyMap(), child);
}
public NestedQuery(Location location, String path, Map<String, Boolean> fields, Query child) {
public NestedQuery(Location location, String path, Map<String, Map.Entry<Boolean, String>> fields, Query child) {
super(location);
if (path == null) {
throw new IllegalArgumentException("path is required");
@ -68,10 +68,10 @@ public class NestedQuery extends Query {
}
@Override
public Query addNestedField(String path, String field, boolean hasDocValues) {
public Query addNestedField(String path, String field, String format, 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);
Query rewrittenChild = child.addNestedField(path, field, format, hasDocValues);
if (rewrittenChild == child) {
return this;
}
@ -81,9 +81,9 @@ public class NestedQuery extends Query {
// I already have the field, no rewriting needed
return this;
}
Map<String, Boolean> newFields = new HashMap<>(fields.size() + 1);
Map<String, Map.Entry<Boolean, String>> newFields = new HashMap<>(fields.size() + 1);
newFields.putAll(fields);
newFields.put(field, hasDocValues);
newFields.put(field, new AbstractMap.SimpleImmutableEntry<>(hasDocValues, format));
return new NestedQuery(location(), path, unmodifiableMap(newFields), child);
}
@ -113,9 +113,9 @@ public class NestedQuery extends Query {
boolean noSourceNeeded = true;
List<String> sourceFields = new ArrayList<>();
for (Entry<String, Boolean> entry : fields.entrySet()) {
if (entry.getValue()) {
ihb.addDocValueField(entry.getKey());
for (Map.Entry<String, Map.Entry<Boolean, String>> entry : fields.entrySet()) {
if (entry.getValue().getKey()) {
ihb.addDocValueField(entry.getKey(), entry.getValue().getValue());
}
else {
sourceFields.add(entry.getKey());
@ -141,7 +141,7 @@ public class NestedQuery extends Query {
return path;
}
Map<String, Boolean> fields() {
Map<String, Map.Entry<Boolean, String>> fields() {
return fields;
}

4
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java
View File

@ -34,8 +34,8 @@ public class NotQuery extends Query {
}
@Override
public Query addNestedField(String path, String field, boolean hasDocValues) {
Query rewrittenChild = child.addNestedField(path, field, hasDocValues);
public Query addNestedField(String path, String field, String format, boolean hasDocValues) {
Query rewrittenChild = child.addNestedField(path, field, format, hasDocValues);
if (child == rewrittenChild) {
return this;
}

2
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/Query.java
View File

@ -44,7 +44,7 @@ public abstract class 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);
public abstract Query addNestedField(String path, String field, String format, boolean hasDocValues);
/**
* Attach the one and only one matching nested query's filter to this

24
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java
View File

@ -5,6 +5,9 @@
*/
package org.elasticsearch.xpack.sql.querydsl.container;
import java.util.AbstractMap.SimpleImmutableEntry;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery;
import org.elasticsearch.xpack.sql.querydsl.query.MatchAll;
@ -21,18 +24,21 @@ public class QueryContainerTests extends ESTestCase {
private Location location = LocationTests.randomLocation();
private String path = randomAlphaOfLength(5);
private String name = randomAlphaOfLength(5);
private String format = DocValueFieldsContext.USE_DEFAULT_FORMAT;
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));
Query expected = new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)),
new MatchAll(location));
assertEquals(expected, QueryContainer.rewriteToContainNestedField(null, location, path, name, format, hasDocValues));
}
public void testRewriteToContainsNestedFieldWhenContainsNestedField() {
Query original = new BoolQuery(location, true,
new NestedQuery(location, path, singletonMap(name, hasDocValues), new MatchAll(location)),
new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)),
new MatchAll(location)),
new RangeQuery(location, randomAlphaOfLength(5), 0, randomBoolean(), 100, randomBoolean()));
assertSame(original, QueryContainer.rewriteToContainNestedField(original, location, path, name, randomBoolean()));
assertSame(original, QueryContainer.rewriteToContainNestedField(original, location, path, name, format, randomBoolean()));
}
public void testRewriteToContainsNestedFieldWhenCanAddNestedField() {
@ -41,16 +47,18 @@ public class QueryContainerTests extends ESTestCase {
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)),
new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)),
new MatchAll(location)),
buddy);
assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, hasDocValues));
assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, format, 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));
new NestedQuery(location, path, singletonMap(name, new SimpleImmutableEntry<>(hasDocValues, format)),
new MatchAll(location)));
assertEquals(expected, QueryContainer.rewriteToContainNestedField(original, location, path, name, format, hasDocValues));
}
}

10
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQueryTests.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.sql.querydsl.query;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.search.sort.NestedSortBuilder;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.tree.Location;
@ -12,6 +13,7 @@ import org.elasticsearch.xpack.sql.tree.LocationTests;
import java.util.Arrays;
import java.util.List;
import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.function.Function;
import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
@ -50,14 +52,15 @@ public class BoolQueryTests extends ESTestCase {
public void testAddNestedField() {
Query q = boolQueryWithoutNestedChildren();
assertSame(q, q.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), randomBoolean()));
assertSame(q, q.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), DocValueFieldsContext.USE_DEFAULT_FORMAT,
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);
Query rewritten = q.addNestedField(path, newField, DocValueFieldsContext.USE_DEFAULT_FORMAT, hasDocValues);
assertNotSame(q, rewritten);
assertTrue(rewritten.containsNestedField(path, newField));
}
@ -83,7 +86,8 @@ public class BoolQueryTests extends ESTestCase {
private Query boolQueryWithNestedChildren(String path, String field) {
NestedQuery match = new NestedQuery(LocationTests.randomLocation(), path,
singletonMap(field, randomBoolean()), new MatchAll(LocationTests.randomLocation()));
singletonMap(field, new SimpleImmutableEntry<>(randomBoolean(), DocValueFieldsContext.USE_DEFAULT_FORMAT)),
new MatchAll(LocationTests.randomLocation()));
Query matchAll = new MatchAll(LocationTests.randomLocation());
Query left;
Query right;

4
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/LeafQueryTests.java
View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.sql.querydsl.query;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.search.sort.NestedSortBuilder;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.tree.Location;
@ -52,7 +53,8 @@ public class LeafQueryTests extends ESTestCase {
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()));
assertSame(query, query.addNestedField(randomAlphaOfLength(5), randomAlphaOfLength(5), DocValueFieldsContext.USE_DEFAULT_FORMAT,
randomBoolean()));
}
public void testEnrichNestedSort() {

24
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/query/NestedQueryTests.java
View File

@ -5,12 +5,14 @@
*/
package org.elasticsearch.xpack.sql.querydsl.query;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
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.AbstractMap.SimpleImmutableEntry;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@ -38,11 +40,11 @@ public class NestedQueryTests extends ESTestCase {
return new NestedQuery(LocationTests.randomLocation(), randomAlphaOfLength(5), randomFields(), randomQuery(depth));
}
private static Map<String, Boolean> randomFields() {
private static Map<String, Map.Entry<Boolean, String>> randomFields() {
int size = between(0, 5);
Map<String, Boolean> fields = new HashMap<>(size);
Map<String, Map.Entry<Boolean, String>> fields = new HashMap<>(size);
while (fields.size() < size) {
fields.put(randomAlphaOfLength(5), randomBoolean());
fields.put(randomAlphaOfLength(5), new SimpleImmutableEntry<>(randomBoolean(), DocValueFieldsContext.USE_DEFAULT_FORMAT));
}
return fields;
}
@ -77,20 +79,20 @@ public class NestedQueryTests extends ESTestCase {
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()));
assertSame(q, q.addNestedField(q.path(), field, DocValueFieldsContext.USE_DEFAULT_FORMAT, 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()));
assertSame(q, q.addNestedField(otherPath, randomAlphaOfLength(5), DocValueFieldsContext.USE_DEFAULT_FORMAT, 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);
NestedQuery added = (NestedQuery) q.addNestedField(q.path(), newField, DocValueFieldsContext.USE_DEFAULT_FORMAT, hasDocValues);
assertNotSame(q, added);
assertThat(added.fields(), hasEntry(newField, hasDocValues));
assertThat(added.fields(), hasEntry(newField, new SimpleImmutableEntry<>(hasDocValues, DocValueFieldsContext.USE_DEFAULT_FORMAT)));
assertTrue(added.containsNestedField(q.path(), newField));
for (Map.Entry<String, Boolean> field : q.fields().entrySet()) {
for (Map.Entry<String, Map.Entry<Boolean, String>> field : q.fields().entrySet()) {
assertThat(added.fields(), hasEntry(field.getKey(), field.getValue()));
assertTrue(added.containsNestedField(q.path(), field.getKey()));
}
@ -129,7 +131,9 @@ public class NestedQueryTests extends ESTestCase {
}
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());
NestedQuery q = new NestedQuery(new Location(1, 1), "a.b",
singletonMap("f", new SimpleImmutableEntry<>(true, DocValueFieldsContext.USE_DEFAULT_FORMAT)),
new MatchAll(new Location(1, 1)));
assertEquals("NestedQuery@1:2[a.b.{f=true=use_field_mapping}[MatchAll@1:2[]]]", q.toString());
}
}