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

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.
This commit is contained in:
Jim Ferenczi 2016-01-27 17:50:24 +01:00
parent ba29818629
commit 1343d6cbd1
5 changed files with 81 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
core/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java
View File

@ -28,7 +28,6 @@ import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.script.Script; import org.elasticsearch.script.Script;
import org.elasticsearch.script.Template; import org.elasticsearch.script.Template;
import org.elasticsearch.search.Scroll; import org.elasticsearch.search.Scroll;
import org.elasticsearch.search.searchafter.SearchAfterBuilder;
import org.elasticsearch.search.aggregations.AbstractAggregationBuilder; import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.fetch.innerhits.InnerHitsBuilder; import org.elasticsearch.search.fetch.innerhits.InnerHitsBuilder;

77
core/src/main/java/org/elasticsearch/search/searchafter/SearchAfterBuilder.java
View File

@ -82,7 +82,11 @@ public class SearchAfterBuilder implements ToXContent, FromXContentBuilder<Searc
Object[] fieldValues = new Object[sortFields.length]; Object[] fieldValues = new Object[sortFields.length];
for (int i = 0; i < sortFields.length; i++) { for (int i = 0; i < sortFields.length; i++) {
SortField sortField = sortFields[i]; SortField sortField = sortFields[i];
fieldValues[i] = convertValueFromSortField(values[i], sortField); if (values[i] != null) {
fieldValues[i] = convertValueFromSortField(values[i], sortField);
} else {
fieldValues[i] = null;
}
} }
// We set the doc id to Integer.MAX_VALUE in order to make sure that the search starts "after" the first document that is equal to the field values. // We set the doc id to Integer.MAX_VALUE in order to make sure that the search starts "after" the first document that is equal to the field values.
return new FieldDoc(Integer.MAX_VALUE, 0, fieldValues); return new FieldDoc(Integer.MAX_VALUE, 0, fieldValues);
@ -191,8 +195,10 @@ public class SearchAfterBuilder implements ToXContent, FromXContentBuilder<Searc
values.add(parser.text()); values.add(parser.text());
} else if (token == XContentParser.Token.VALUE_BOOLEAN) { } else if (token == XContentParser.Token.VALUE_BOOLEAN) {
values.add(parser.booleanValue()); values.add(parser.booleanValue());
} else if (token == XContentParser.Token.VALUE_NULL) {
values.add(null);
} else { } else {
throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING + "] or [" + XContentParser.Token.VALUE_NUMBER + "] or [" + XContentParser.Token.VALUE_BOOLEAN + "] but found [" + token + "] inside search_after.", parser.getTokenLocation()); throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING + "] or [" + XContentParser.Token.VALUE_NUMBER + "] or [" + XContentParser.Token.VALUE_BOOLEAN + "] or [" + XContentParser.Token.VALUE_NULL + "] but found [" + token + "] inside search_after.", parser.getTokenLocation());
} }
} }
} else { } else {
@ -207,38 +213,39 @@ public class SearchAfterBuilder implements ToXContent, FromXContentBuilder<Searc
out.writeVInt(sortValues.length); out.writeVInt(sortValues.length);
for (Object fieldValue : sortValues) { for (Object fieldValue : sortValues) {
if (fieldValue == null) { if (fieldValue == null) {
throw new IOException("Can't handle " + SEARCH_AFTER.getPreferredName() + " field value of type [null]"); out.writeByte((byte) 0);
}
Class 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 { } else {
throw new IOException("Can't handle " + SEARCH_AFTER.getPreferredName() + " field value of type [" + type + "]"); Class<?> 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<Searc
Object[] values = new Object[size]; Object[] values = new Object[size];
for (int i = 0; i < size; i++) { for (int i = 0; i < size; i++) {
byte type = in.readByte(); byte type = in.readByte();
if (type == 1) { if (type == 0) {
values[i] = null;
} else if (type == 1) {
values[i] = in.readString(); values[i] = in.readString();
} else if (type == 2) { } else if (type == 2) {
values[i] = in.readInt(); values[i] = in.readInt();

23
core/src/test/java/org/elasticsearch/search/searchafter/SearchAfterBuilderTests.java
View File

@ -70,7 +70,7 @@ public class SearchAfterBuilderTests extends ESTestCase {
SearchAfterBuilder searchAfterBuilder = new SearchAfterBuilder(); SearchAfterBuilder searchAfterBuilder = new SearchAfterBuilder();
Object[] values = new Object[numSearchFrom]; Object[] values = new Object[numSearchFrom];
for (int i = 0; i < numSearchFrom; i++) { for (int i = 0; i < numSearchFrom; i++) {
int branch = randomInt(8); int branch = randomInt(9);
switch (branch) { switch (branch) {
case 0: case 0:
values[i] = randomInt(); values[i] = randomInt();
@ -99,6 +99,9 @@ public class SearchAfterBuilderTests extends ESTestCase {
case 8: case 8:
values[i] = new Text(randomAsciiOfLengthBetween(5, 20)); values[i] = new Text(randomAsciiOfLengthBetween(5, 20));
break; break;
case 9:
values[i] = null;
break;
} }
} }
searchAfterBuilder.setSortValues(values); searchAfterBuilder.setSortValues(values);
@ -115,7 +118,7 @@ public class SearchAfterBuilderTests extends ESTestCase {
jsonBuilder.startObject(); jsonBuilder.startObject();
jsonBuilder.startArray("search_after"); jsonBuilder.startArray("search_after");
for (int i = 0; i < numSearchAfter; i++) { for (int i = 0; i < numSearchAfter; i++) {
int branch = randomInt(8); int branch = randomInt(9);
switch (branch) { switch (branch) {
case 0: case 0:
jsonBuilder.value(randomInt()); jsonBuilder.value(randomInt());
@ -144,6 +147,9 @@ public class SearchAfterBuilderTests extends ESTestCase {
case 8: case 8:
jsonBuilder.value(new Text(randomAsciiOfLengthBetween(5, 20))); jsonBuilder.value(new Text(randomAsciiOfLengthBetween(5, 20)));
break; break;
case 9:
jsonBuilder.nullValue();
break;
} }
} }
jsonBuilder.endArray(); jsonBuilder.endArray();
@ -223,18 +229,7 @@ public class SearchAfterBuilderTests extends ESTestCase {
assertEquals(searchAfterBuilder.hashCode(), secondSearchAfterBuilder.hashCode()); assertEquals(searchAfterBuilder.hashCode(), secondSearchAfterBuilder.hashCode());
} }
} }
public void testWithNullValue() throws Exception {
SearchAfterBuilder builder = new SearchAfterBuilder();
builder.setSortValues(new Object[] {1, "1", null});
try {
serializedCopy(builder);
fail("Should fail on null values");
} catch (IOException e) {
assertThat(e.getMessage(), Matchers.equalTo("Can't handle search_after field value of type [null]"));
}
}
public void testWithNullArray() throws Exception { public void testWithNullArray() throws Exception {
SearchAfterBuilder builder = new SearchAfterBuilder(); SearchAfterBuilder builder = new SearchAfterBuilder();
try { try {

39
core/src/test/java/org/elasticsearch/search/searchafter/SearchAfterIT.java
View File

@ -34,12 +34,12 @@ import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.transport.RemoteTransportException; import org.elasticsearch.transport.RemoteTransportException;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import java.io.IOException;
import java.util.List; import java.util.List;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Comparator; import java.util.Comparator;
import java.util.Collections; import java.util.Collections;
import java.util.Arrays; import java.util.Arrays;
import java.util.concurrent.ExecutionException;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
@ -52,15 +52,15 @@ public class SearchAfterIT extends ESIntegTestCase {
public void testsShouldFail() throws Exception { public void testsShouldFail() throws Exception {
createIndex("test"); createIndex("test");
ensureGreen();
indexRandom(true, client().prepareIndex("test", "type1", "0").setSource("field1", 0, "field2", "toto")); indexRandom(true, client().prepareIndex("test", "type1", "0").setSource("field1", 0, "field2", "toto"));
try { try {
client().prepareSearch("test") client().prepareSearch("test")
.addSort("field1", SortOrder.ASC) .addSort("field1", SortOrder.ASC)
.setQuery(matchAllQuery()) .setQuery(matchAllQuery())
.searchAfter(new Object[]{0}) .searchAfter(new Object[]{0})
.setScroll("1m") .setScroll("1m")
.execute().actionGet(); .get();
fail("Should fail on search_after cannot be used with scroll."); fail("Should fail on search_after cannot be used with scroll.");
} catch (SearchPhaseExecutionException e) { } catch (SearchPhaseExecutionException e) {
@ -74,7 +74,7 @@ public class SearchAfterIT extends ESIntegTestCase {
.setQuery(matchAllQuery()) .setQuery(matchAllQuery())
.searchAfter(new Object[]{0}) .searchAfter(new Object[]{0})
.setFrom(10) .setFrom(10)
.execute().actionGet(); .get();
fail("Should fail on search_after cannot be used with from > 0."); fail("Should fail on search_after cannot be used with from > 0.");
} catch (SearchPhaseExecutionException e) { } catch (SearchPhaseExecutionException e) {
@ -87,7 +87,7 @@ public class SearchAfterIT extends ESIntegTestCase {
client().prepareSearch("test") client().prepareSearch("test")
.setQuery(matchAllQuery()) .setQuery(matchAllQuery())
.searchAfter(new Object[]{0.75f}) .searchAfter(new Object[]{0.75f})
.execute().actionGet(); .get();
fail("Should fail on search_after on score only is disabled"); fail("Should fail on search_after on score only is disabled");
} catch (SearchPhaseExecutionException e) { } catch (SearchPhaseExecutionException e) {
@ -115,7 +115,7 @@ public class SearchAfterIT extends ESIntegTestCase {
.setQuery(matchAllQuery()) .setQuery(matchAllQuery())
.addSort("field1", SortOrder.ASC) .addSort("field1", SortOrder.ASC)
.searchAfter(new Object[]{1, 2}) .searchAfter(new Object[]{1, 2})
.execute().actionGet(); .get();
fail("Should fail on search_after size differs from sort field size"); fail("Should fail on search_after size differs from sort field size");
} catch (SearchPhaseExecutionException e) { } catch (SearchPhaseExecutionException e) {
assertThat(e.getCause().getClass(), Matchers.equalTo(RemoteTransportException.class)); assertThat(e.getCause().getClass(), Matchers.equalTo(RemoteTransportException.class));
@ -128,7 +128,7 @@ public class SearchAfterIT extends ESIntegTestCase {
.setQuery(matchAllQuery()) .setQuery(matchAllQuery())
.addSort("field1", SortOrder.ASC) .addSort("field1", SortOrder.ASC)
.searchAfter(new Object[]{"toto"}) .searchAfter(new Object[]{"toto"})
.execute().actionGet(); .get();
fail("Should fail on search_after on score only is disabled"); fail("Should fail on search_after on score only is disabled");
} catch (SearchPhaseExecutionException e) { } 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 { public void testWithSimpleTypes() throws Exception {
int numFields = randomInt(20) + 1; int numFields = randomInt(20) + 1;
int[] types = new int[numFields-1]; int[] types = new int[numFields-1];
for (int i = 0; i < numFields-1; i++) { for (int i = 0; i < numFields-1; i++) {
types[i] = randomInt(6); types[i] = randomInt(6);
} }
List<List> documents = new ArrayList<> (); List<List> documents = new ArrayList<>();
for (int i = 0; i < NUM_DOCS; i++) { for (int i = 0; i < NUM_DOCS; i++) {
List values = new ArrayList<>(); List values = new ArrayList<>();
for (int type : types) { for (int type : types) {
@ -239,7 +257,7 @@ public class SearchAfterIT extends ESIntegTestCase {
if (sortValues != null) { if (sortValues != null) {
req.searchAfter(sortValues); req.searchAfter(sortValues);
} }
SearchResponse searchResponse = req.execute().actionGet(); SearchResponse searchResponse = req.get();
for (SearchHit hit : searchResponse.getHits()) { for (SearchHit hit : searchResponse.getHits()) {
List toCompare = convertSortValues(documents.get(offset++)); List toCompare = convertSortValues(documents.get(offset++));
assertThat(LST_COMPARATOR.compare(toCompare, Arrays.asList(hit.sortValues())), equalTo(0)); 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 + "]"); 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 // Convert Integer, Short, Byte and Boolean to Long in order to match the conversion done

4
rest-api-spec/src/main/resources/rest-api-spec/api/search.json
View File

@ -154,10 +154,6 @@
"request_cache": { "request_cache": {
"type" : "boolean", "type" : "boolean",
"description" : "Specify if request cache should be used for this request or not, defaults to index level setting" "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"
} }
} }
}, },