Skip to content

Commit 5396d8e

Browse files
s1monws1monw
committed
Copy slow in wrapped comparator even if root doc is the first doc in the
segment. We missed to copy the slot if the root doc is the first doc in the segment. Closes elastic#3706
1 parent a9e0f4d commit 5396d8e

File tree

3 files changed

+27
-46
lines changed

3 files changed

+27
-46
lines changed

‎src/main/java/org/elasticsearch/index/search/nested/NestedFieldComparatorSource.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ public int compareBottom(int rootDoc) throws IOException {
376376
@Override
377377
public void copy(int slot, int rootDoc) throws IOException {
378378
if (rootDoc == 0 || rootDocuments == null || innerDocuments == null) {
379+
copyMissing(wrappedComparator, slot);
379380
return;
380381
}
381382

‎src/test/java/org/elasticsearch/search/geo/GeoDistanceTests.java‎

Lines changed: 25 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.search.geo;
2121

22+
import org.elasticsearch.AbstractSharedClusterTest;
2223
import org.elasticsearch.action.search.SearchPhaseExecutionException;
2324
import org.elasticsearch.action.search.SearchResponse;
2425
import org.elasticsearch.common.Priority;
@@ -34,8 +35,6 @@
3435
import org.elasticsearch.search.SearchHit;
3536
import org.elasticsearch.search.sort.SortBuilders;
3637
import org.elasticsearch.search.sort.SortOrder;
37-
import org.elasticsearch.test.hamcrest.ElasticsearchAssertions;
38-
import org.elasticsearch.AbstractSharedClusterTest;
3938
import org.junit.Test;
4039

4140
import java.io.IOException;
@@ -45,7 +44,7 @@
4544
import static org.elasticsearch.index.query.FilterBuilders.*;
4645
import static org.elasticsearch.index.query.QueryBuilders.filteredQuery;
4746
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
48-
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
47+
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
4948
import static org.hamcrest.Matchers.*;
5049

5150
/**
@@ -54,11 +53,6 @@ public class GeoDistanceTests extends AbstractSharedClusterTest {
5453

5554
@Test
5655
public void simpleDistanceTests() throws Exception {
57-
try {
58-
client().admin().indices().prepareDelete("test").execute().actionGet();
59-
} catch (Exception e) {
60-
// ignore
61-
}
6256
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1")
6357
.startObject("properties").startObject("location").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
6458
.endObject().endObject().string();
@@ -207,19 +201,18 @@ public void simpleDistanceTests() throws Exception {
207201
.execute().actionGet();
208202

209203
assertHitCount(searchResponse, 7);
210-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "3", "4", "5", "6", "2", "7");
204+
assertOrderedSearchHits(searchResponse, "1", "3", "4", "5", "6", "2", "7");
211205

212206
searchResponse = client().prepareSearch().setQuery(matchAllQuery())
213207
.addSort(SortBuilders.geoDistanceSort("location").point(40.7143528, -74.0059731).order(SortOrder.DESC))
214208
.execute().actionGet();
215209

216210
assertHitCount(searchResponse, 7);
217-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "7", "2", "6", "5", "4", "3", "1");
211+
assertOrderedSearchHits(searchResponse, "7", "2", "6", "5", "4", "3", "1");
218212
}
219213

220214
@Test
221215
public void testDistanceSortingMVFields() throws Exception {
222-
client().admin().indices().prepareDelete().execute().actionGet();
223216
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1")
224217
.startObject("properties").startObject("locations").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
225218
.endObject().endObject().string();
@@ -274,7 +267,7 @@ public void testDistanceSortingMVFields() throws Exception {
274267
.execute().actionGet();
275268

276269
assertHitCount(searchResponse, 4);
277-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "2", "3", "4");
270+
assertOrderedSearchHits(searchResponse, "1", "2", "3", "4");
278271
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), equalTo(0d));
279272
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(0.4621d, 0.01d));
280273
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(1.055d, 0.01d));
@@ -286,7 +279,7 @@ public void testDistanceSortingMVFields() throws Exception {
286279
.execute().actionGet();
287280

288281
assertHitCount(searchResponse, 4);
289-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
282+
assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
290283
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), equalTo(0d));
291284
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(1.258d, 0.01d));
292285
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(5.286d, 0.01d));
@@ -298,7 +291,7 @@ public void testDistanceSortingMVFields() throws Exception {
298291
.execute().actionGet();
299292

300293
assertHitCount(searchResponse, 4);
301-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
294+
assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
302295
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(8.572d, 0.01d));
303296
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(5.286d, 0.01d));
304297
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(1.258d, 0.01d));
@@ -310,7 +303,7 @@ public void testDistanceSortingMVFields() throws Exception {
310303
.execute().actionGet();
311304

312305
assertHitCount(searchResponse, 4);
313-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "4", "3", "2", "1");
306+
assertOrderedSearchHits(searchResponse, "4", "3", "2", "1");
314307
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(2.029d, 0.01d));
315308
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(1.055d, 0.01d));
316309
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(0.4621d, 0.01d));
@@ -321,7 +314,7 @@ public void testDistanceSortingMVFields() throws Exception {
321314
.execute().actionGet();
322315

323316
assertHitCount(searchResponse, 4);
324-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
317+
assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
325318
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), equalTo(0d));
326319
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(1.157d, 0.01d));
327320
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(2.874d, 0.01d));
@@ -332,7 +325,7 @@ public void testDistanceSortingMVFields() throws Exception {
332325
.execute().actionGet();
333326

334327
assertHitCount(searchResponse, 4);
335-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
328+
assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
336329
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(5.301d, 0.01d));
337330
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(2.874d, 0.01d));
338331
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(1.157d, 0.01d));
@@ -351,7 +344,6 @@ public void testDistanceSortingMVFields() throws Exception {
351344
@Test
352345
// Regression bug: https://github.com/elasticsearch/elasticsearch/issues/2851
353346
public void testDistanceSortingWithMissingGeoPoint() throws Exception {
354-
client().admin().indices().prepareDelete().execute().actionGet();
355347
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type1")
356348
.startObject("properties").startObject("locations").field("type", "geo_point").field("lat_lon", true).endObject().endObject()
357349
.endObject().endObject().string();
@@ -384,7 +376,7 @@ public void testDistanceSortingWithMissingGeoPoint() throws Exception {
384376
.execute().actionGet();
385377

386378
assertHitCount(searchResponse, 2);
387-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "2");
379+
assertOrderedSearchHits(searchResponse, "1", "2");
388380
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(0.4621d, 0.01d));
389381
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), equalTo(Double.MAX_VALUE));
390382

@@ -395,19 +387,13 @@ public void testDistanceSortingWithMissingGeoPoint() throws Exception {
395387

396388
// Doc with missing geo point is first, is consistent with 0.20.x
397389
assertHitCount(searchResponse, 2);
398-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "2", "1");
390+
assertOrderedSearchHits(searchResponse, "2", "1");
399391
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), equalTo(Double.MAX_VALUE));
400392
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(5.286d, 0.01d));
401393
}
402394

403395
@Test
404396
public void distanceScriptTests() throws Exception {
405-
try {
406-
client().admin().indices().prepareDelete("test").execute().actionGet();
407-
} catch (Exception e) {
408-
// ignore
409-
}
410-
411397
double source_lat = 32.798;
412398
double source_long = -117.151;
413399
double target_lat = 32.81;
@@ -472,7 +458,7 @@ public void testDistanceSortingNestedFields() throws Exception {
472458
.addMapping("company", mapping)
473459
.execute().actionGet();
474460
client().admin().cluster().prepareHealth("companies").setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().execute().actionGet();
475-
461+
indexRandom("companies", true,
476462
client().prepareIndex("companies", "company", "1").setSource(jsonBuilder().startObject()
477463
.field("name", "company 1")
478464
.startArray("branches")
@@ -481,8 +467,7 @@ public void testDistanceSortingNestedFields() throws Exception {
481467
.startObject("location").field("lat", 40.7143528).field("lon", -74.0059731).endObject()
482468
.endObject()
483469
.endArray()
484-
.endObject()).execute().actionGet();
485-
470+
.endObject()),
486471
client().prepareIndex("companies", "company", "2").setSource(jsonBuilder().startObject()
487472
.field("name", "company 2")
488473
.startArray("branches")
@@ -495,8 +480,7 @@ public void testDistanceSortingNestedFields() throws Exception {
495480
.startObject("location").field("lat", 40.718266).field("lon", -74.007819).endObject() // to NY: 0.4621 km
496481
.endObject()
497482
.endArray()
498-
.endObject()).execute().actionGet();
499-
483+
.endObject()),
500484
client().prepareIndex("companies", "company", "3").setSource(jsonBuilder().startObject()
501485
.field("name", "company 3")
502486
.startArray("branches")
@@ -509,9 +493,7 @@ public void testDistanceSortingNestedFields() throws Exception {
509493
.startObject("location").field("lat", 40.7247222).field("lon", -74).endObject() // to NY: 1.258 km
510494
.endObject()
511495
.endArray()
512-
.endObject()).execute().actionGet();
513-
514-
496+
.endObject()),
515497
client().prepareIndex("companies", "company", "4").setSource(jsonBuilder().startObject()
516498
.field("name", "company 4")
517499
.startArray("branches")
@@ -524,17 +506,15 @@ public void testDistanceSortingNestedFields() throws Exception {
524506
.startObject("location").field("lat", 40.65).field("lon", -73.95).endObject() // to NY: 8.572 km
525507
.endObject()
526508
.endArray()
527-
.endObject()).execute().actionGet();
528-
529-
client().admin().indices().prepareRefresh().execute().actionGet();
509+
.endObject()));
530510

531511
// Order: Asc
532512
SearchResponse searchResponse = client().prepareSearch("companies").setQuery(matchAllQuery())
533513
.addSort(SortBuilders.geoDistanceSort("branches.location").point(40.7143528, -74.0059731).order(SortOrder.ASC))
534514
.execute().actionGet();
535515

536516
assertHitCount(searchResponse, 4);
537-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "2", "3", "4");
517+
assertOrderedSearchHits(searchResponse, "1", "2", "3", "4");
538518
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), equalTo(0d));
539519
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(0.4621d, 0.01d));
540520
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(1.055d, 0.01d));
@@ -546,7 +526,7 @@ public void testDistanceSortingNestedFields() throws Exception {
546526
.execute().actionGet();
547527

548528
assertHitCount(searchResponse, 4);
549-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
529+
assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
550530
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), equalTo(0d));
551531
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(1.258d, 0.01d));
552532
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(5.286d, 0.01d));
@@ -558,7 +538,7 @@ public void testDistanceSortingNestedFields() throws Exception {
558538
.execute().actionGet();
559539

560540
assertHitCount(searchResponse, 4);
561-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
541+
assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
562542
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(8.572d, 0.01d));
563543
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(5.286d, 0.01d));
564544
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(1.258d, 0.01d));
@@ -570,7 +550,7 @@ public void testDistanceSortingNestedFields() throws Exception {
570550
.execute().actionGet();
571551

572552
assertHitCount(searchResponse, 4);
573-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "4", "3", "2", "1");
553+
assertOrderedSearchHits(searchResponse, "4", "3", "2", "1");
574554
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(2.029d, 0.01d));
575555
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(1.055d, 0.01d));
576556
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(0.4621d, 0.01d));
@@ -581,7 +561,7 @@ public void testDistanceSortingNestedFields() throws Exception {
581561
.execute().actionGet();
582562

583563
assertHitCount(searchResponse, 4);
584-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
564+
assertOrderedSearchHits(searchResponse, "1", "3", "2", "4");
585565
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), equalTo(0d));
586566
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(1.157d, 0.01d));
587567
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(2.874d, 0.01d));
@@ -595,7 +575,7 @@ public void testDistanceSortingNestedFields() throws Exception {
595575
.execute().actionGet();
596576

597577
assertHitCount(searchResponse, 4);
598-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
578+
assertOrderedSearchHits(searchResponse, "4", "2", "3", "1");
599579
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(5.301d, 0.01d));
600580
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), closeTo(2.874d, 0.01d));
601581
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), closeTo(1.157d, 0.01d));
@@ -607,9 +587,9 @@ public void testDistanceSortingNestedFields() throws Exception {
607587
.point(40.7143528, -74.0059731).sortMode("avg").order(SortOrder.ASC)
608588
)
609589
.execute().actionGet();
610-
611590
assertHitCount(searchResponse, 4);
612-
ElasticsearchAssertions.assertOrderedSearchHits(searchResponse, "4", "1", "2", "3");
591+
assertFirstHit(searchResponse, hasId("4"));
592+
assertSearchHits(searchResponse, "1", "2", "3", "4");
613593
assertThat(((Number) searchResponse.getHits().getAt(0).sortValues()[0]).doubleValue(), closeTo(8.572d, 0.01d));
614594
assertThat(((Number) searchResponse.getHits().getAt(1).sortValues()[0]).doubleValue(), equalTo(Double.MAX_VALUE));
615595
assertThat(((Number) searchResponse.getHits().getAt(2).sortValues()[0]).doubleValue(), equalTo(Double.MAX_VALUE));

‎src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public static void assertOrderedSearchHits(SearchResponse searchResponse, String
120120
assertThat("Expected different hit count. " + shardStatus, searchResponse.getHits().hits().length, equalTo(ids.length));
121121
for (int i=0; i<ids.length; i++) {
122122
SearchHit hit = searchResponse.getHits().hits()[i];
123-
assertThat("Expected id: " + hit.getId() + " at position " + i + " but wasn't." + shardStatus, hit.getId(), equalTo(ids[i]));
123+
assertThat("Expected id: " + ids[i] + " at position " + i + " but wasn't." + shardStatus, hit.getId(), equalTo(ids[i]));
124124
}
125125
}
126126

0 commit comments

Comments
 (0)