From 1343d6cbd1721f4c0411d08525ccfcf82c9932a0 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 27 Jan 2016 17:50:24 +0100 Subject: [PATCH] Remove search_after from the query string param of the rest api spec. Handle null values in search_after. Ensure that the cluster is green after each index creation in the integ tests. --- .../action/search/SearchRequestBuilder.java | 1 - .../searchafter/SearchAfterBuilder.java | 77 +++++++++++-------- .../searchafter/SearchAfterBuilderTests.java | 23 +++--- .../search/searchafter/SearchAfterIT.java | 39 +++++++--- .../resources/rest-api-spec/api/search.json | 4 - 5 files changed, 81 insertions(+), 63 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java b/core/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java index 9d6f61ed580..07d7b2fa3d0 100644 --- a/core/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java +++ b/core/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java @@ -28,7 +28,6 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.script.Script; import org.elasticsearch.script.Template; import org.elasticsearch.search.Scroll; -import org.elasticsearch.search.searchafter.SearchAfterBuilder; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.fetch.innerhits.InnerHitsBuilder; diff --git a/core/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java b/core/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java index 7cfcee4de59..13fae70174d 100644 --- a/core/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java @@ -82,7 +82,11 @@ public class SearchAfterBuilder implements ToXContent, FromXContentBuilder type = fieldValue.getClass(); + if (type == String.class) { + out.writeByte((byte) 1); + out.writeString((String) fieldValue); + } else if (type == Integer.class) { + out.writeByte((byte) 2); + out.writeInt((Integer) fieldValue); + } else if (type == Long.class) { + out.writeByte((byte) 3); + out.writeLong((Long) fieldValue); + } else if (type == Float.class) { + out.writeByte((byte) 4); + out.writeFloat((Float) fieldValue); + } else if (type == Double.class) { + out.writeByte((byte) 5); + out.writeDouble((Double) fieldValue); + } else if (type == Byte.class) { + out.writeByte((byte) 6); + out.writeByte((Byte) fieldValue); + } else if (type == Short.class) { + out.writeByte((byte) 7); + out.writeShort((Short) fieldValue); + } else if (type == Boolean.class) { + out.writeByte((byte) 8); + out.writeBoolean((Boolean) fieldValue); + } else if (fieldValue instanceof Text) { + out.writeByte((byte) 9); + out.writeText((Text) fieldValue); + } else { + throw new IOException("Can't handle " + SEARCH_AFTER.getPreferredName() + " field value of type [" + type + "]"); + } } } } @@ -250,7 +257,9 @@ public class SearchAfterBuilder implements ToXContent, FromXContentBuilder 0."); } catch (SearchPhaseExecutionException e) { @@ -87,7 +87,7 @@ public class SearchAfterIT extends ESIntegTestCase { client().prepareSearch("test") .setQuery(matchAllQuery()) .searchAfter(new Object[]{0.75f}) - .execute().actionGet(); + .get(); fail("Should fail on search_after on score only is disabled"); } catch (SearchPhaseExecutionException e) { @@ -115,7 +115,7 @@ public class SearchAfterIT extends ESIntegTestCase { .setQuery(matchAllQuery()) .addSort("field1", SortOrder.ASC) .searchAfter(new Object[]{1, 2}) - .execute().actionGet(); + .get(); fail("Should fail on search_after size differs from sort field size"); } catch (SearchPhaseExecutionException e) { assertThat(e.getCause().getClass(), Matchers.equalTo(RemoteTransportException.class)); @@ -128,7 +128,7 @@ public class SearchAfterIT extends ESIntegTestCase { .setQuery(matchAllQuery()) .addSort("field1", SortOrder.ASC) .searchAfter(new Object[]{"toto"}) - .execute().actionGet(); + .get(); fail("Should fail on search_after on score only is disabled"); } catch (SearchPhaseExecutionException e) { @@ -138,13 +138,31 @@ public class SearchAfterIT extends ESIntegTestCase { } } + public void testWithNullStrings() throws ExecutionException, InterruptedException { + createIndex("test"); + ensureGreen(); + indexRandom(true, + client().prepareIndex("test", "type1", "0").setSource("field1", 0), + client().prepareIndex("test", "type1", "1").setSource("field1", 100, "field2", "toto")); + SearchResponse searchResponse = client().prepareSearch("test") + .addSort("field1", SortOrder.ASC) + .addSort("field2", SortOrder.ASC) + .setQuery(matchAllQuery()) + .searchAfter(new Object[]{0, null}) + .get(); + assertThat(searchResponse.getHits().getTotalHits(), Matchers.equalTo(2L)); + assertThat(searchResponse.getHits().getHits().length, Matchers.equalTo(1)); + assertThat(searchResponse.getHits().getHits()[0].sourceAsMap().get("field1"), Matchers.equalTo(100)); + assertThat(searchResponse.getHits().getHits()[0].sourceAsMap().get("field2"), Matchers.equalTo("toto")); + } + public void testWithSimpleTypes() throws Exception { int numFields = randomInt(20) + 1; int[] types = new int[numFields-1]; for (int i = 0; i < numFields-1; i++) { types[i] = randomInt(6); } - List documents = new ArrayList<> (); + List documents = new ArrayList<>(); for (int i = 0; i < NUM_DOCS; i++) { List values = new ArrayList<>(); for (int type : types) { @@ -239,7 +257,7 @@ public class SearchAfterIT extends ESIntegTestCase { if (sortValues != null) { req.searchAfter(sortValues); } - SearchResponse searchResponse = req.execute().actionGet(); + SearchResponse searchResponse = req.get(); for (SearchHit hit : searchResponse.getHits()) { List toCompare = convertSortValues(documents.get(offset++)); assertThat(LST_COMPARATOR.compare(toCompare, Arrays.asList(hit.sortValues())), equalTo(0)); @@ -282,7 +300,8 @@ public class SearchAfterIT extends ESIntegTestCase { fail("Can't match type [" + type + "]"); } } - indexRequestBuilder.addMapping(typeName, mappings.toArray()).execute().actionGet(); + indexRequestBuilder.addMapping(typeName, mappings.toArray()).get(); + ensureGreen(); } // Convert Integer, Short, Byte and Boolean to Long in order to match the conversion done diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/search.json b/rest-api-spec/src/main/resources/rest-api-spec/api/search.json index d1c19f3ef21..d2b9b8cf9b4 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/search.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/search.json @@ -154,10 +154,6 @@ "request_cache": { "type" : "boolean", "description" : "Specify if request cache should be used for this request or not, defaults to index level setting" - }, - "search_after": { - "type" : "list", - "description" : "An array of sort values that indicates where the sort of the top hits should start" } } },