From 8346e92ebb8ed10ac5e8c0653eb514372802f2c2 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 21 Nov 2014 16:42:37 +0100 Subject: [PATCH] Core: Fix script fields to be returned as a multivalued field when they produce a list. This change is essentially the same as #3015 but on script fields. Close #8592 --- docs/reference/migration/migrate_2_0.asciidoc | 32 +++++++++++++++++++ .../script/ScriptFieldsFetchSubPhase.java | 15 +++++++-- .../script/IndexLookupTests.java | 2 +- .../search/fields/SearchFieldsTests.java | 4 +-- 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/docs/reference/migration/migrate_2_0.asciidoc b/docs/reference/migration/migrate_2_0.asciidoc index 56b31bf3ba0..2bd71e7ebeb 100644 --- a/docs/reference/migration/migrate_2_0.asciidoc +++ b/docs/reference/migration/migrate_2_0.asciidoc @@ -56,3 +56,35 @@ The `memory` / `ram` store (`index.store.type`) option was removed in Elasticsea === Term Vectors API Usage of `/_termvector` is deprecated, and replaced in favor of `/_termvectors`. + +=== Script fields + +Script fields in 1.x were only returned as a single value. So even if the return +value of a script used to be list, it would be returned as an array containing +a single value that is a list too, such as: + +[source,json] +--------------- +"fields": { + "my_field": [ + [ + "v1", + "v2" + ] + ] +} +--------------- + +In elasticsearch 2.x, scripts that return a list of values are considered as +multivalued fields. So the same example would return the following response, +with values in a single array. + +[source,json] +--------------- +"fields": { + "my_field": [ + "v1", + "v2" + ] +} +--------------- diff --git a/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java b/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java index 2c6cd0fa2ca..beb9e22b659 100644 --- a/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java +++ b/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.fetch.script; import com.google.common.collect.ImmutableMap; + import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.search.SearchHitField; @@ -29,7 +30,10 @@ import org.elasticsearch.search.internal.InternalSearchHitField; import org.elasticsearch.search.internal.SearchContext; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; /** @@ -86,10 +90,17 @@ public class ScriptFieldsFetchSubPhase implements FetchSubPhase { SearchHitField hitField = hitContext.hit().fields().get(scriptField.name()); if (hitField == null) { - hitField = new InternalSearchHitField(scriptField.name(), new ArrayList<>(2)); + final List values; + if (value == null) { + values = Collections.emptyList(); + } else if (value instanceof Collection) { + values = new ArrayList<>((Collection) value); + } else { + values = Collections.singletonList(value); + } + hitField = new InternalSearchHitField(scriptField.name(), values); hitContext.hit().fields().put(scriptField.name(), hitField); } - hitField.values().add(value); } } } diff --git a/src/test/java/org/elasticsearch/script/IndexLookupTests.java b/src/test/java/org/elasticsearch/script/IndexLookupTests.java index b4082508fa9..95ec815e2df 100644 --- a/src/test/java/org/elasticsearch/script/IndexLookupTests.java +++ b/src/test/java/org/elasticsearch/script/IndexLookupTests.java @@ -385,7 +385,7 @@ public class IndexLookupTests extends ElasticsearchIntegrationTest { assertHitCount(sr, expectedHitSize); int nullCounter = 0; for (SearchHit hit : sr.getHits().getHits()) { - Object result = hit.getFields().get("tvtest").getValues().get(0); + Object result = hit.getFields().get("tvtest").getValues(); Object expectedResult = expectedArray.get(hit.getId()); assertThat("for doc " + hit.getId(), result, equalTo(expectedResult)); if (expectedResult != null) { diff --git a/src/test/java/org/elasticsearch/search/fields/SearchFieldsTests.java b/src/test/java/org/elasticsearch/search/fields/SearchFieldsTests.java index f25a4d02f69..6fd3356d40a 100644 --- a/src/test/java/org/elasticsearch/search/fields/SearchFieldsTests.java +++ b/src/test/java/org/elasticsearch/search/fields/SearchFieldsTests.java @@ -298,12 +298,12 @@ public class SearchFieldsTests extends ElasticsearchIntegrationTest { assertThat(sObj2Arr2.get(0).toString(), equalTo("arr_value1")); assertThat(sObj2Arr2.get(1).toString(), equalTo("arr_value2")); - sObj2Arr2 = response.getHits().getAt(0).field("s_obj2_arr2").value(); + sObj2Arr2 = response.getHits().getAt(0).field("s_obj2_arr2").values(); assertThat(sObj2Arr2.size(), equalTo(2)); assertThat(sObj2Arr2.get(0).toString(), equalTo("arr_value1")); assertThat(sObj2Arr2.get(1).toString(), equalTo("arr_value2")); - List sObj2Arr3 = response.getHits().getAt(0).field("s_arr3").value(); + List sObj2Arr3 = response.getHits().getAt(0).field("s_arr3").values(); assertThat(((Map) sObj2Arr3.get(0)).get("arr3_field1").toString(), equalTo("arr3_value1")); }