Skip to content

Add tests for the supported query types. #35319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 8, 2018
Merged

Add tests for the supported query types. #35319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd53702
Add tests for the supported query types.
jtibshirani Oct 18, 2018
1f1272c
Disallow unbounded range queries on keyed JSON fields.
jtibshirani Nov 5, 2018
d5213c8
Add more range query test cases.
jtibshirani Nov 7, 2018
7a3c18e
Make sure MatchQueryBuilderTests only lowercases values for text fields.
jtibshirani Nov 7, 2018
4e7765a
Make sure MappedFieldType#hasDocValues always returns false.
jtibshirani Nov 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/JsonFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.Version;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -165,6 +166,11 @@ public Builder copyTo(CopyTo copyTo) {
throw new UnsupportedOperationException("[copy_to] is not supported for [" + CONTENT_TYPE + "] fields.");
}

@Override
protected boolean defaultDocValues(Version indexCreated) {
return false;
}

@Override
public JsonFieldMapper build(BuilderContext context) {
setupFieldType(context);
Expand Down Expand Up @@ -276,6 +282,25 @@ public Query existsQuery(QueryShardContext context) {
return new PrefixQuery(term);
}

@Override
public Query rangeQuery(Object lowerTerm,
Object upperTerm,
boolean includeLower,
boolean includeUpper,
QueryShardContext context) {

// We require range queries to specify both bounds because an unbounded query could incorrectly match
// values from other keys. For example, a query on the 'first' key with only a lower bound would become
// ("first\0value", null), which would also match the value "second\0value" belonging to the key 'second'.
if (lowerTerm == null || upperTerm == null) {
Copy link
Contributor Author

@jtibshirani jtibshirani Nov 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could fix this instead of disallowing unbounded range queries, but I wasn't sure the extra complexity was worth it. It's not clear how useful this range functionality will be right now, especially without support for numerics.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this limitation is ok for now. We can revisit this later if necessary

throw new IllegalArgumentException("[range] queries on keyed [" + CONTENT_TYPE +
"] fields must include both an upper and a lower bound.");
}

return super.rangeQuery(lowerTerm, upperTerm,
includeLower, includeUpper, context);
}

@Override
public Query fuzzyQuery(Object value, Fuzziness fuzziness, int prefixLength, int maxExpansions,
boolean transpositions) {
Expand Down
20 changes: 12 additions & 8 deletions server/src/test/java/org/elasticsearch/index/mapper/JsonFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.mapper.JsonFieldMapper.KeyedJsonFieldType;
import org.elasticsearch.index.mapper.JsonFieldMapper.RootJsonFieldType;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESSingleNodeTestCase;
Expand All @@ -42,7 +43,6 @@

import static org.apache.lucene.analysis.BaseTokenStreamTestCase.assertTokenStreamContents;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;

public class JsonFieldMapperTests extends ESSingleNodeTestCase {
private IndexService indexService;
Expand Down Expand Up @@ -417,7 +417,8 @@ public void testNullValues() throws Exception {
assertEquals(new BytesRef("key\0placeholder"), prefixedOtherFields[0].binaryValue());
}

public void testSplitQueriesOnWhitespace() throws IOException {
public void testSplitQueriesOnWhitespace() throws IOException {
MapperService mapperService = indexService.mapperService();
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject()
.startObject("type")
.startObject("properties")
Expand All @@ -427,13 +428,16 @@ public void testSplitQueriesOnWhitespace() throws IOException {
.endObject()
.endObject()
.endObject().endObject());
indexService.mapperService().merge("type", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE);
mapperService.merge("type", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE);

MappedFieldType fieldType = indexService.mapperService().fullName("field");
assertThat(fieldType, instanceOf(RootJsonFieldType.class));
RootJsonFieldType rootFieldType = (RootJsonFieldType) mapperService.fullName("field");
assertThat(rootFieldType.searchAnalyzer(), equalTo(JsonFieldMapper.WHITESPACE_ANALYZER));
assertTokenStreamContents(rootFieldType.searchAnalyzer().analyzer().tokenStream("", "Hello World"),
new String[] {"Hello", "World"});

RootJsonFieldType ft = (RootJsonFieldType) fieldType;
assertThat(ft.searchAnalyzer(), equalTo(JsonFieldMapper.WHITESPACE_ANALYZER));
assertTokenStreamContents(ft.searchAnalyzer().analyzer().tokenStream("", "Hello World"), new String[] {"Hello", "World"});
KeyedJsonFieldType keyedFieldType = (KeyedJsonFieldType) mapperService.fullName("field.key");
assertThat(keyedFieldType.searchAnalyzer(), equalTo(JsonFieldMapper.WHITESPACE_ANALYZER));
assertTokenStreamContents(keyedFieldType.searchAnalyzer().analyzer().tokenStream("", "Hello World"),
new String[] {"Hello", "World"});
}
}
55 changes: 55 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/KeyedJsonFieldTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,20 @@

import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.MultiTermQuery;
import org.apache.lucene.search.PrefixQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermInSetQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.TermRangeQuery;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.index.mapper.JsonFieldMapper.KeyedJsonFieldType;
import org.junit.Before;

import java.util.ArrayList;
import java.util.List;

public class KeyedJsonFieldTypeTests extends FieldTypeTestCase {

@Before
Expand Down Expand Up @@ -73,6 +79,22 @@ public void testTermQuery() {
assertEquals("Cannot search on field [field] since it is not indexed.", e.getMessage());
}

public void testTermsQuery() {
KeyedJsonFieldType ft = createDefaultFieldType();
ft.setName("field");

Query expected = new TermInSetQuery("field",
new BytesRef("key\0value1"),
new BytesRef("key\0value2"));

List<String> terms = new ArrayList<>();
terms.add("value1");
terms.add("value2");
Query actual = ft.termsQuery(terms, null);

assertEquals(expected, actual);
}

public void testExistsQuery() {
KeyedJsonFieldType ft = createDefaultFieldType();
ft.setName("field");
Expand All @@ -81,6 +103,14 @@ public void testExistsQuery() {
assertEquals(expected, ft.existsQuery(null));
}

public void testPrefixQuery() {
KeyedJsonFieldType ft = createDefaultFieldType();
ft.setName("field");

Query expected = new PrefixQuery(new Term("field", "key\0val"));
assertEquals(expected, ft.prefixQuery("val", MultiTermQuery.CONSTANT_SCORE_REWRITE, null));
}

public void testFuzzyQuery() {
KeyedJsonFieldType ft = createDefaultFieldType();
ft.setName("field");
Expand All @@ -90,6 +120,31 @@ public void testFuzzyQuery() {
assertEquals("[fuzzy] queries are not currently supported on [json] fields.", e.getMessage());
}

public void testRangeQuery() {
KeyedJsonFieldType ft = createDefaultFieldType();
ft.setName("field");

TermRangeQuery expected = new TermRangeQuery("field",
new BytesRef("key\0lower"),
new BytesRef("key\0upper"), false, false);
assertEquals(expected, ft.rangeQuery("lower", "upper", false, false, null));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also check that we do the right thing if the bounds are inclusive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


expected = new TermRangeQuery("field",
new BytesRef("key\0lower"),
new BytesRef("key\0upper"), true, true);
assertEquals(expected, ft.rangeQuery("lower", "upper", true, true, null));

IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
ft.rangeQuery("lower", null, false, false, null));
assertEquals("[range] queries on keyed [json] fields must include both an upper and a lower bound.",
e.getMessage());

e = expectThrows(IllegalArgumentException.class, () ->
ft.rangeQuery(null, "upper", false, false, null));
assertEquals("[range] queries on keyed [json] fields must include both an upper and a lower bound.",
e.getMessage());
}

public void testRegexpQuery() {
KeyedJsonFieldType ft = createDefaultFieldType();
ft.setName("field");
Expand Down
16 changes: 16 additions & 0 deletions server/src/test/java/org/elasticsearch/index/mapper/RootJsonFieldTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.lucene.index.Term;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.search.TermRangeQuery;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.index.mapper.JsonFieldMapper.RootJsonFieldType;
Expand Down Expand Up @@ -83,6 +84,21 @@ public void testFuzzyQuery() {
assertEquals("[fuzzy] queries are not currently supported on [json] fields.", e.getMessage());
}

public void testRangeQuery() {
RootJsonFieldType ft = createDefaultFieldType();
ft.setName("field");

TermRangeQuery expected = new TermRangeQuery("field",
new BytesRef("lower"),
new BytesRef("upper"), false, false);
assertEquals(expected, ft.rangeQuery("lower", "upper", false, false, null));

expected = new TermRangeQuery("field",
new BytesRef("lower"),
new BytesRef("upper"), true, true);
assertEquals(expected, ft.rangeQuery("lower", "upper", true, true, null));
}

public void testRegexpQuery() {
RootJsonFieldType ft = createDefaultFieldType();
ft.setName("field");
Expand Down
1 change: 0 additions & 1 deletion server/src/test/java/org/elasticsearch/index/query/MatchPhrasePrefixQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.util.HashMap;
import java.util.Map;

import static org.elasticsearch.test.AbstractBuilderTestCase.STRING_ALIAS_FIELD_NAME;
import static org.hamcrest.CoreMatchers.either;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.Matchers.containsString;
Expand Down
7 changes: 4 additions & 3 deletions server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class MatchQueryBuilderTests extends AbstractQueryTestCase<MatchQueryBuil
@Override
protected MatchQueryBuilder doCreateTestQueryBuilder() {
String fieldName = randomFrom(STRING_FIELD_NAME, STRING_ALIAS_FIELD_NAME, BOOLEAN_FIELD_NAME, INT_FIELD_NAME,
DOUBLE_FIELD_NAME, DATE_FIELD_NAME);
DOUBLE_FIELD_NAME, DATE_FIELD_NAME, JSON_FIELD_NAME);
Object value;
if (isTextField(fieldName)) {
int terms = randomIntBetween(0, 3);
Expand Down Expand Up @@ -153,7 +153,8 @@ protected void doAssertLuceneQuery(MatchQueryBuilder queryBuilder, Query query,
MappedFieldType fieldType = context.fieldMapper(queryBuilder.fieldName());
if (query instanceof TermQuery && fieldType != null) {
String queryValue = queryBuilder.value().toString();
if (queryBuilder.analyzer() == null || queryBuilder.analyzer().equals("simple")) {
if (isTextField(queryBuilder.fieldName())
&& (queryBuilder.analyzer() == null || queryBuilder.analyzer().equals("simple"))) {
queryValue = queryValue.toLowerCase(Locale.ROOT);
}
Query expectedTermQuery = fieldType.termQuery(queryValue, context);
Expand Down Expand Up @@ -404,7 +405,7 @@ public void testMaxBooleanClause() {
query.setAnalyzer(new MockGraphAnalyzer(createGiantGraphMultiTerms()));
expectThrows(BooleanQuery.TooManyClauses.class, () -> query.parse(Type.PHRASE, STRING_FIELD_NAME, ""));
}

private static class MockGraphAnalyzer extends Analyzer {

CannedBinaryTokenStream tokenStream;
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase<MultiMatc
@Override
protected MultiMatchQueryBuilder doCreateTestQueryBuilder() {
String fieldName = randomFrom(STRING_FIELD_NAME, INT_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_FIELD_NAME,
MISSING_FIELD_NAME, MISSING_WILDCARD_FIELD_NAME);
MISSING_FIELD_NAME, MISSING_WILDCARD_FIELD_NAME, JSON_FIELD_NAME);

final Object value;
if (fieldName.equals(STRING_FIELD_NAME)) {
Expand Down
5 changes: 2 additions & 3 deletions server/src/test/java/org/elasticsearch/index/search/MatchPhraseQueryIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.query.MatchPhraseQueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.search.MatchQuery.ZeroTermsQuery;
import org.elasticsearch.test.ESIntegTestCase;
import org.junit.Before;
Expand All @@ -33,6 +32,7 @@
import java.util.List;
import java.util.concurrent.ExecutionException;

import static org.elasticsearch.index.query.QueryBuilders.matchPhraseQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;

Expand All @@ -55,7 +55,7 @@ public void testZeroTermsQuery() throws ExecutionException, InterruptedException
List<IndexRequestBuilder> indexRequests = getIndexRequests();
indexRandom(true, false, indexRequests);

MatchPhraseQueryBuilder baseQuery = QueryBuilders.matchPhraseQuery("name", "the who")
MatchPhraseQueryBuilder baseQuery = matchPhraseQuery("name", "the who")
.analyzer("standard_stopwords");

MatchPhraseQueryBuilder matchNoneQuery = baseQuery.zeroTermsQuery(ZeroTermsQuery.NONE);
Expand All @@ -67,7 +67,6 @@ public void testZeroTermsQuery() throws ExecutionException, InterruptedException
assertHitCount(matchAllResponse, 2L);
}


private List<IndexRequestBuilder> getIndexRequests() {
List<IndexRequestBuilder> requests = new ArrayList<>();
requests.add(client().prepareIndex(INDEX, "band").setSource("name", "the beatles"));
Expand Down
38 changes: 38 additions & 0 deletions server/src/test/java/org/elasticsearch/search/query/ExistsIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
import static org.elasticsearch.index.query.QueryBuilders.existsQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
Expand Down Expand Up @@ -227,4 +228,41 @@ public void testFieldAliasWithNoDocValues() throws Exception {
assertSearchResponse(response);
assertHitCount(response, 2);
}

public void testJsonFields() throws Exception {
XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
.startObject("type")
.startObject("properties")
.startObject("headers")
.field("type", "json")
.endObject()
.endObject()
.endObject()
.endObject();
assertAcked(prepareCreate("test").addMapping("type", mapping));

IndexRequestBuilder indexRequest = client().prepareIndex("test", "type", "1")
.setSource(XContentFactory.jsonBuilder()
.startObject()
.startObject("headers")
.field("content-type", "application/json")
.endObject()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not especially related to the current PR, but how is case normalization being handled? The normal HTTP header here is Content-type with a capital C; is there a lowercase filter being applied anywhere, or is that being ignored for the moment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't apply any lowercasing to the object keys. This is consistent with the way normal field names are handled in the mapping, which are not lowercased (you could have two distinct fields for example, one named Content-Type and the other content-type). Do you think an option to automatically case-fold the keys would be useful?

Separately for field values (not keys), keyword fields provide a normalizer option. This is something we could consider, although it might need to be designed differently as json fields can contain non-string values.

.endObject());
indexRandom(true, false, indexRequest);

SearchResponse searchResponse = client().prepareSearch()
.setQuery(existsQuery("headers"))
.get();
assertHitCount(searchResponse, 1L);

searchResponse = client().prepareSearch()
.setQuery(existsQuery("headers.content-type"))
.get();
assertHitCount(searchResponse, 1L);

searchResponse = client().prepareSearch()
.setQuery(existsQuery("headers.nonexistent"))
.get();
assertHitCount(searchResponse, 0L);
}
}
Loading