From d9de1299958aa07a2b112bc24a79a15f110f88c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20B=C3=BCscher?= <christoph@elastic.co>
Date: Thu, 17 Mar 2016 18:57:45 +0100
Subject: [PATCH] Adressing review comments

---
 .../elasticsearch/search/sort/FieldSortBuilder.java   | 11 ++---------
 .../search/sort/GeoDistanceSortBuilder.java           |  4 ++--
 .../elasticsearch/search/sort/ScriptSortBuilder.java  |  1 -
 .../elasticsearch/search/sort/SortParseElement.java   |  9 ---------
 4 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java
index 827fbbd86bf..aad68c3f700 100644
--- a/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java
+++ b/core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java
@@ -255,22 +255,15 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> implements S
                 throw new QueryShardException(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;
-                if (stringFieldMapper.fieldType().tokenized()) {
-                    // Fail early
-                    throw new SearchParseException(context, "Can't sort on tokenized string field[" + fieldName + "]");
-                }
-            }*/
-
             MultiValueMode localSortMode = null;
             if (sortMode != null) {
                 localSortMode = MultiValueMode.fromString(sortMode.toString());
             }
+
             if (fieldType.isNumeric() == false && (sortMode == SortMode.SUM || sortMode == SortMode.AVG)) {
                 throw new QueryShardException(context, "we only support AVG and SUM on number based fields");
             }
+
             boolean reverse = (order == SortOrder.DESC);
             if (localSortMode == null) {
                 localSortMode = reverse ? MultiValueMode.MAX : MultiValueMode.MIN;
diff --git a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java
index 35d1c4ae043..8bd97b003b7 100644
--- a/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java
+++ b/core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java
@@ -488,10 +488,10 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
 
         if (!indexCreatedBeforeV2_0 && !ignoreMalformed) {
             for (GeoPoint point : localPoints) {
-                if (point.lat() > 90.0 || point.lat() < -90.0) {
+                if (GeoUtils.isValidLatitude(point.lat()) == false) {
                     throw new ElasticsearchParseException("illegal latitude value [{}] for [GeoDistanceSort]", point.lat());
                 }
-                if (point.lon() > 180.0 || point.lon() < -180) {
+                if (GeoUtils.isValidLongitude(point.lon()) == false) {
                     throw new ElasticsearchParseException("illegal longitude value [{}] for [GeoDistanceSort]", point.lon());
                 }
             }
diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java
index aa592a7eee7..791eba99b81 100644
--- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java
+++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java
@@ -285,7 +285,6 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> implements
         }
 
         final Nested nested = resolveNested(context, nestedPath, nestedFilter);
-
         final IndexFieldData.XFieldComparatorSource fieldComparatorSource;
         switch (type) {
             case STRING:
diff --git a/core/src/main/java/org/elasticsearch/search/sort/SortParseElement.java b/core/src/main/java/org/elasticsearch/search/sort/SortParseElement.java
index 24f69cd0b61..1ed2a457a5f 100644
--- a/core/src/main/java/org/elasticsearch/search/sort/SortParseElement.java
+++ b/core/src/main/java/org/elasticsearch/search/sort/SortParseElement.java
@@ -217,15 +217,6 @@ public class SortParseElement implements SearchParseElement {
                 throw new QueryShardException(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;
-                if (stringFieldMapper.fieldType().tokenized()) {
-                    // Fail early
-                    throw new SearchParseException(context, "Can't sort on tokenized string field[" + fieldName + "]");
-                }
-            }*/
-
             // We only support AVG and SUM on number based fields
             if (fieldType.isNumeric() == false && (sortMode == MultiValueMode.SUM || sortMode == MultiValueMode.AVG)) {
                 sortMode = null;