From a18f47e62e426885de82584e5536f5f20d252d49 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Thu, 26 Sep 2013 22:49:38 +0200 Subject: [PATCH] Returning useful exception when sorting on a completion field Closes #3747 --- .../index/mapper/FieldMapper.java | 2 ++ .../mapper/core/AbstractFieldMapper.java | 4 ++++ .../mapper/core/CompletionFieldMapper.java | 4 ++++ .../search/sort/SortParseElement.java | 5 ++++ .../suggest/CompletionSuggestSearchTests.java | 23 +++++++++++++++++++ 5 files changed, 38 insertions(+) diff --git a/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index bb90a09b660..8f85fa1d2de 100644 --- a/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -202,4 +202,6 @@ public interface FieldMapper { PostingsFormatProvider postingsFormatProvider(); boolean isNumeric(); + + boolean isSortable(); } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java index 407b4802c7a..127077d7ca3 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java @@ -711,4 +711,8 @@ public abstract class AbstractFieldMapper implements FieldMapper, Mapper { return false; } + @Override + public boolean isSortable() { + return true; + } } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java index 7d26e2d934c..819ffa59e15 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java @@ -357,6 +357,10 @@ public class CompletionFieldMapper extends AbstractFieldMapper { return CONTENT_TYPE; } + @Override + public boolean isSortable() { + return false; + } @Override public FieldType defaultFieldType() { diff --git a/src/main/java/org/elasticsearch/search/sort/SortParseElement.java b/src/main/java/org/elasticsearch/search/sort/SortParseElement.java index b85dd8e8bfa..fc30437ff34 100644 --- a/src/main/java/org/elasticsearch/search/sort/SortParseElement.java +++ b/src/main/java/org/elasticsearch/search/sort/SortParseElement.java @@ -31,6 +31,7 @@ import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.fieldcomparator.SortMode; import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.ObjectMappers; +import org.elasticsearch.index.mapper.core.CompletionFieldMapper; import org.elasticsearch.index.mapper.core.NumberFieldMapper; import org.elasticsearch.index.mapper.object.ObjectMapper; import org.elasticsearch.index.query.ParsedFilter; @@ -198,6 +199,10 @@ public class SortParseElement implements SearchParseElement { throw new SearchParseException(context, "No mapping found for [" + fieldName + "] in order to sort on"); } + if (!fieldMapper.isSortable()) { + throw new SearchParseException(context, "Sorting not supported for field[" + fieldName + "]"); + } + // Enable when we also know how to detect fields that do tokenize, but only emit one token /*if (fieldMapper instanceof StringFieldMapper) { StringFieldMapper stringFieldMapper = (StringFieldMapper) fieldMapper; diff --git a/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java b/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java index 508824f9105..e571c05dd87 100644 --- a/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java +++ b/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestSearchTests.java @@ -27,6 +27,7 @@ import org.elasticsearch.action.admin.indices.segments.IndexShardSegments; import org.elasticsearch.action.admin.indices.segments.ShardSegments; import org.elasticsearch.action.admin.indices.stats.IndicesStatsResponse; import org.elasticsearch.action.index.IndexRequestBuilder; +import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.suggest.SuggestResponse; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; @@ -34,6 +35,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.mapper.MapperException; import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.core.CompletionFieldMapper; +import org.elasticsearch.search.sort.FieldSortBuilder; import org.elasticsearch.search.suggest.completion.CompletionStats; import org.elasticsearch.search.suggest.completion.CompletionSuggestion; import org.elasticsearch.search.suggest.completion.CompletionSuggestionBuilder; @@ -556,6 +558,27 @@ public class CompletionSuggestSearchTests extends AbstractIntegrationTest { assertThat(regexSizeInBytes, is(totalSizeInBytes)); } + @Test + public void testThatSortingOnCompletionFieldReturnsUsefulException() throws Exception { + createIndexAndMapping(); + + client().prepareIndex(INDEX, TYPE, "1").setSource(jsonBuilder() + .startObject().startObject(FIELD) + .startArray("input").value("Nirvana").endArray() + .endObject().endObject() + ).get(); + + refresh(); + try { + client().prepareSearch(INDEX).setTypes(TYPE).addSort(new FieldSortBuilder(FIELD)).execute().actionGet(); + fail("Expected an exception due to trying to sort on completion field, but did not happen"); + } catch (SearchPhaseExecutionException e) { + assertThat(e.status().getStatus(), is(400)); + assertThat(e.getMessage(), containsString("Sorting not supported for field[" + FIELD + "]")); + } + } + + public void assertSuggestions(String suggestion, String... suggestions) { String suggestionName = RandomStrings.randomAsciiOfLength(new Random(), 10); SuggestResponse suggestResponse = client().prepareSuggest(INDEX).addSuggestion(