Skip to content

Commit d7b0cd8

Browse files
committed
Some additional refactors and bug fixes.
* Add integration test coverage for typeless lookup queries. * Fix a bug around typeless terms lookup queries. * Make sure to provide non-deprecated methods in QueryBuilders. * Make the deprecation messages more accurate. * Update the query DSL documentation. * Avoid creating duplicate query builder tests. Don't use 'ids' when generating random queries to avoid types warnings.
1 parent 8e892f5 commit d7b0cd8

File tree

27 files changed

+417
-1059
lines changed

27 files changed

+417
-1059
lines changed

client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/QueryDSLDocumentationTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ public void testHasParent() {
235235

236236
public void testIds() {
237237
// tag::ids
238-
idsQuery()
238+
idsQuery() // <1>
239239
.addIds("1", "4", "100");
240240
// end::ids
241241
}

docs/reference/query-dsl/geo-shape-query.asciidoc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ GET /example/_search
8181
==== Pre-Indexed Shape
8282

8383
The Query also supports using a shape which has already been indexed in
84-
another index and/or index type. This is particularly useful for when
84+
another index. This is particularly useful for when
8585
you have a pre-defined list of shapes which are useful to your
8686
application and you want to reference this using a logical name (for
8787
example 'New Zealand') rather than having to provide their coordinates
@@ -90,7 +90,6 @@ each time. In this situation it is only necessary to provide:
9090
* `id` - The ID of the document that containing the pre-indexed shape.
9191
* `index` - Name of the index where the pre-indexed shape is. Defaults
9292
to 'shapes'.
93-
* `type` - Index type where the pre-indexed shape is.
9493
* `path` - The field specified as path containing the pre-indexed shape.
9594
Defaults to 'shape'.
9695
* `routing` - The routing of the shape document if required.
@@ -130,7 +129,6 @@ GET /example/_search
130129
"location": {
131130
"indexed_shape": {
132131
"index": "shapes",
133-
"type": "_doc",
134132
"id": "deu",
135133
"path": "location"
136134
}

docs/reference/query-dsl/ids-query.asciidoc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,9 @@ GET /_search
1010
{
1111
"query": {
1212
"ids" : {
13-
"type" : "_doc",
1413
"values" : ["1", "4", "100"]
1514
}
1615
}
1716
}
1817
--------------------------------------------------
1918
// CONSOLE
20-
21-
The `type` is optional and can be omitted, and can also accept an array
22-
of values. If no type is specified, all types defined in the index mapping are tried.

docs/reference/query-dsl/mlt-query.asciidoc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,10 @@ GET /_search
4242
"like" : [
4343
{
4444
"_index" : "imdb",
45-
"_type" : "movies",
4645
"_id" : "1"
4746
},
4847
{
4948
"_index" : "imdb",
50-
"_type" : "movies",
5149
"_id" : "2"
5250
},
5351
"and potentially some more text here as well"
@@ -74,7 +72,6 @@ GET /_search
7472
"like" : [
7573
{
7674
"_index" : "marvel",
77-
"_type" : "quotes",
7875
"doc" : {
7976
"name": {
8077
"first": "Ben",
@@ -85,7 +82,6 @@ GET /_search
8582
},
8683
{
8784
"_index" : "marvel",
88-
"_type" : "quotes",
8985
"_id" : "2"
9086
}
9187
],

docs/reference/query-dsl/terms-query.asciidoc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ The terms lookup mechanism supports the following options:
3636
`index`::
3737
The index to fetch the term values from.
3838

39-
`type`::
40-
The type to fetch the term values from.
41-
4239
`id`::
4340
The id of the document to fetch the term values from.
4441

@@ -93,7 +90,6 @@ GET /tweets/_search
9390
"terms" : {
9491
"user" : {
9592
"index" : "users",
96-
"type" : "_doc",
9793
"id" : "2",
9894
"path" : "followers"
9995
}

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ public void testStoringQueries() throws Exception {
537537
public void testQueryWithRewrite() throws Exception {
538538
addQueryFieldMappings();
539539
client().prepareIndex("remote", "doc", "1").setSource("field", "value").get();
540-
QueryBuilder queryBuilder = termsLookupQuery("field", new TermsLookup("remote", "doc", "1", "field"));
540+
QueryBuilder queryBuilder = termsLookupQuery("field", new TermsLookup("remote", "1", "field"));
541541
ParsedDocument doc = mapperService.documentMapper("doc").parse(new SourceToParse("test", "doc", "1",
542542
BytesReference.bytes(XContentFactory
543543
.jsonBuilder()
@@ -553,7 +553,6 @@ public void testQueryWithRewrite() throws Exception {
553553
PlainActionFuture<QueryBuilder> future = new PlainActionFuture<>();
554554
Rewriteable.rewriteAndFetch(queryBuilder, shardContext, future);
555555
assertQueryBuilder(qbSource, future.get());
556-
assertWarnings(QueryShardContext.TYPES_DEPRECATION_MESSAGE);
557556
}
558557

559558

rest-api-spec/src/main/resources/rest-api-spec/test/search/170_terms_query.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@
4848
search:
4949
rest_total_hits_as_int: true
5050
index: test_index
51-
body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u1", "path" : "followers"}}}}
51+
body: {"query" : {"terms" : {"user" : {"index" : "test_index", "id" : "u1", "path" : "followers"}}}}
5252
- match: { hits.total: 2 }
5353

5454
- do:
5555
catch: bad_request
5656
search:
5757
rest_total_hits_as_int: true
5858
index: test_index
59-
body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u2", "path" : "followers"}}}}
59+
body: {"query" : {"terms" : {"user" : {"index" : "test_index", "id" : "u2", "path" : "followers"}}}}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"Terms Query with No.of terms exceeding index.max_terms_count should FAIL":
3+
- skip:
4+
version: " - 6.99.99"
5+
reason: index.max_terms_count setting has been added in 7.0.0
6+
- do:
7+
indices.create:
8+
index: test_index
9+
body:
10+
settings:
11+
number_of_shards: 1
12+
index.max_terms_count: 2
13+
mappings:
14+
test_type:
15+
properties:
16+
user:
17+
type: keyword
18+
followers:
19+
type: keyword
20+
- do:
21+
bulk:
22+
refresh: true
23+
body:
24+
- '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u1"}}'
25+
- '{"user": "u1", "followers": ["u2", "u3"]}'
26+
- '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u2"}}'
27+
- '{"user": "u2", "followers": ["u1", "u3", "u4"]}'
28+
- '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u3"}}'
29+
- '{"user": "u3", "followers": ["u1"]}'
30+
- '{"index": {"_index": "test_index", "_type": "test_type", "_id": "u4"}}'
31+
- '{"user": "u4", "followers": ["u3"]}'
32+
33+
- do:
34+
search:
35+
rest_total_hits_as_int: true
36+
index: test_index
37+
body: {"query" : {"terms" : {"user" : ["u1", "u2"]}}}
38+
- match: { hits.total: 2 }
39+
40+
- do:
41+
catch: bad_request
42+
search:
43+
rest_total_hits_as_int: true
44+
index: test_index
45+
body: {"query" : {"terms" : {"user" : ["u1", "u2", "u3"]}}}
46+
47+
- do:
48+
search:
49+
rest_total_hits_as_int: true
50+
index: test_index
51+
body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u1", "path" : "followers"}}}}
52+
- match: { hits.total: 2 }
53+
54+
- do:
55+
catch: bad_request
56+
search:
57+
rest_total_hits_as_int: true
58+
index: test_index
59+
body: {"query" : {"terms" : {"user" : {"index" : "test_index", "type" : "test_type", "id" : "u2", "path" : "followers"}}}}

server/src/main/java/org/elasticsearch/index/query/GeoShapeQueryBuilder.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919

2020
package org.elasticsearch.index.query;
2121

22+
import org.apache.logging.log4j.LogManager;
2223
import org.apache.lucene.document.LatLonShape;
2324
import org.apache.lucene.geo.Line;
2425
import org.apache.lucene.geo.Polygon;
2526
import org.apache.lucene.geo.Rectangle;
26-
import org.apache.logging.log4j.LogManager;
2727
import org.apache.lucene.search.BooleanClause;
2828
import org.apache.lucene.search.BooleanQuery;
2929
import org.apache.lucene.search.ConstantScoreQuery;
@@ -69,7 +69,10 @@
6969
*/
7070
public class GeoShapeQueryBuilder extends AbstractQueryBuilder<GeoShapeQueryBuilder> {
7171
public static final String NAME = "geo_shape";
72-
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(GeoShapeQueryBuilder.class));
72+
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
73+
LogManager.getLogger(GeoShapeQueryBuilder.class));
74+
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [geo_shape] queries. " +
75+
"The type should no longer be specified in the [indexed_shape] section.";
7376

7477
public static final String DEFAULT_SHAPE_INDEX_NAME = "shapes";
7578
public static final String DEFAULT_SHAPE_FIELD_NAME = "shape";
@@ -664,8 +667,6 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO
664667
if (SHAPE_ID_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
665668
id = parser.text();
666669
} else if (SHAPE_TYPE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
667-
deprecationLogger.deprecatedAndMaybeLog(
668-
"geo_share_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE);
669670
type = parser.text();
670671
} else if (SHAPE_INDEX_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
671672
index = parser.text();
@@ -699,6 +700,11 @@ public static GeoShapeQueryBuilder fromXContent(XContentParser parser) throws IO
699700
}
700701
}
701702
GeoShapeQueryBuilder builder;
703+
if (type != null) {
704+
deprecationLogger.deprecatedAndMaybeLog(
705+
"geo_share_query_with_types", TYPES_DEPRECATION_MESSAGE);
706+
}
707+
702708
if (shape != null) {
703709
builder = new GeoShapeQueryBuilder(fieldName, shape);
704710
} else {

server/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@
5353
*/
5454
public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
5555
public static final String NAME = "ids";
56-
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(IdsQueryBuilder.class));
56+
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
57+
LogManager.getLogger(IdsQueryBuilder.class));
58+
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [ids] queries.";
5759

5860
private static final ParseField TYPE_FIELD = new ParseField("type");
5961
private static final ParseField VALUES_FIELD = new ParseField("values");
@@ -87,17 +89,11 @@ protected void doWriteTo(StreamOutput out) throws IOException {
8789
/**
8890
* Add types to query
8991
*/
90-
// TODO: Remove in 8.0
9192
@Deprecated
9293
public IdsQueryBuilder types(String... types) {
9394
if (types == null) {
9495
throw new IllegalArgumentException("[" + NAME + "] types cannot be null");
9596
}
96-
// Even if types are null, IdsQueryBuilder uses an empty array for decoding types.
97-
// For this reason, issue deprecation warning if types contain something.
98-
if (types.length > 0) {
99-
deprecationLogger.deprecatedAndMaybeLog("ids_query_with_types", QueryShardContext.TYPES_DEPRECATION_MESSAGE);
100-
}
10197
this.types = types;
10298
return this;
10399
}
@@ -131,7 +127,9 @@ public Set<String> ids() {
131127
@Override
132128
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
133129
builder.startObject(NAME);
134-
builder.array(TYPE_FIELD.getPreferredName(), types);
130+
if (types.length > 0) {
131+
builder.array(TYPE_FIELD.getPreferredName(), types);
132+
}
135133
builder.startArray(VALUES_FIELD.getPreferredName());
136134
for (String value : ids) {
137135
builder.value(value);
@@ -152,7 +150,11 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
152150

153151
public static IdsQueryBuilder fromXContent(XContentParser parser) {
154152
try {
155-
return PARSER.apply(parser, null);
153+
IdsQueryBuilder builder = PARSER.apply(parser, null);
154+
if (builder.types().length > 0) {
155+
deprecationLogger.deprecatedAndMaybeLog("ids_query_with_types", TYPES_DEPRECATION_MESSAGE);
156+
}
157+
return builder;
156158
} catch (IllegalArgumentException e) {
157159
throw new ParsingException(parser.getTokenLocation(), e.getMessage(), e);
158160
}

0 commit comments

Comments
 (0)