Remove MappedFieldType.useTermQueryWithQueryString() and isNumeric(). #17599

In both cases, what elasticsearch is really interested in is whether the field
is an analyzed string field. So it can just check `tokenized()` instead.

core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java

@@ -215,7 +215,8 @@ public class MapperQueryParser extends QueryParser {
                 }
                 if (currentFieldType != null) {
                     Query query = null;
-                    if (currentFieldType.useTermQueryWithQueryString()) {
+                    if (currentFieldType.tokenized() == false) {
+                        // this might be a structured field like a numeric
                         try {
                             query = currentFieldType.termQuery(queryText, context);
                         } catch (RuntimeException e) {
@@ -326,7 +327,7 @@ public class MapperQueryParser extends QueryParser {
     private Query getRangeQuerySingle(String field, String part1, String part2, boolean startInclusive, boolean endInclusive) {
         currentFieldType = context.fieldMapper(field);
         if (currentFieldType != null) {
-            if (lowercaseExpandedTerms && !currentFieldType.isNumeric()) {
+            if (lowercaseExpandedTerms && currentFieldType.tokenized()) {
                 part1 = part1 == null ? null : part1.toLowerCase(locale);
                 part2 = part2 == null ? null : part2.toLowerCase(locale);
             }
@@ -463,7 +464,7 @@ public class MapperQueryParser extends QueryParser {
                     setAnalyzer(context.getSearchAnalyzer(currentFieldType));
                 }
                 Query query = null;
-                if (currentFieldType.useTermQueryWithQueryString()) {
+                if (currentFieldType.tokenized() == false) {
                     query = currentFieldType.prefixQuery(termStr, multiTermRewriteMethod, context);
                 }
                 if (query == null) {
@@ -722,7 +723,7 @@ public class MapperQueryParser extends QueryParser {
                     setAnalyzer(context.getSearchAnalyzer(currentFieldType));
                 }
                 Query query = null;
-                if (currentFieldType.useTermQueryWithQueryString()) {
+                if (currentFieldType.tokenized() == false) {
                     query = currentFieldType.regexpQuery(termStr, RegExp.ALL, maxDeterminizedStates, multiTermRewriteMethod, context);
                 }
                 if (query == null) {

core/src/main/java/org/elasticsearch/action/admin/indices/analyze/TransportAnalyzeAction.java

@@ -128,8 +128,8 @@ public class TransportAnalyzeAction extends TransportSingleShardAction<AnalyzeRe
                 }
                 MappedFieldType fieldType = indexService.mapperService().fullName(request.field());
                 if (fieldType != null) {
-                    if (fieldType.isNumeric()) {
+                    if (fieldType.tokenized() == false) {
-                        throw new IllegalArgumentException("Can't process field [" + request.field() + "], Analysis requests are not supported on numeric fields");
+                        throw new IllegalArgumentException("Can't process field [" + request.field() + "], Analysis requests are only supported on tokenized fields");
                     }
                     analyzer = fieldType.indexAnalyzer();
                     field = fieldType.name();

core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java

@@ -222,10 +222,6 @@ public abstract class MappedFieldType extends FieldType {
         }
     }
 
-    public boolean isNumeric() {
-        return false;
-    }
-
     public boolean isSortable() {
         return true;
     }
@@ -325,14 +321,6 @@ public abstract class MappedFieldType extends FieldType {
         return BytesRefs.toBytesRef(value);
     }
 
-    /**
-     * Should the field query {@link #termQuery(Object, org.elasticsearch.index.query.QueryShardContext)}  be used when detecting this
-     * field in query string.
-     */
-    public boolean useTermQueryWithQueryString() {
-        return false;
-    }
-
     /**
      * Creates a term associated with the field of this mapper for the given
      * value. Its important to use termQuery when building term queries because

core/src/main/java/org/elasticsearch/index/mapper/core/BooleanFieldMapper.java

@@ -191,11 +191,6 @@ public class BooleanFieldMapper extends FieldMapper {
             return value(value);
         }
 
-        @Override
-        public boolean useTermQueryWithQueryString() {
-            return true;
-        }
-
         @Override
         public IndexFieldData.Builder fielddataBuilder() {
             failIfNoDocValues();

core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java

@@ -172,16 +172,6 @@ public abstract class NumberFieldMapper extends FieldMapper implements AllFieldM
         @Override
         public abstract Query fuzzyQuery(Object value, Fuzziness fuzziness, int prefixLength, int maxExpansions, boolean transpositions);
 
-        @Override
-        public boolean useTermQueryWithQueryString() {
-            return true;
-        }
-
-        @Override
-        public boolean isNumeric() {
-            return true;
-        }
-
         @Override
         public DocValueFormat docValueFormat(@Nullable String format, DateTimeZone timeZone) {
             if (timeZone != null) {
@@ -205,6 +195,7 @@ public abstract class NumberFieldMapper extends FieldMapper implements AllFieldM
                                 Explicit<Boolean> ignoreMalformed, Explicit<Boolean> coerce, Settings indexSettings,
                                 MultiFields multiFields, CopyTo copyTo) {
         super(simpleName, fieldType, defaultFieldType, indexSettings, multiFields, copyTo);
+        assert fieldType.tokenized() == false;
         this.ignoreMalformed = ignoreMalformed;
         this.coerce = coerce;
     }

core/src/main/java/org/elasticsearch/index/mapper/internal/FieldNamesFieldMapper.java

@@ -188,11 +188,6 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper {
             return value.toString();
         }
 
-        @Override
-        public boolean useTermQueryWithQueryString() {
-            return true;
-        }
-
         @Override
         public Query termQuery(Object value, QueryShardContext context) {
             if (isEnabled() == false) {

core/src/main/java/org/elasticsearch/index/mapper/internal/IdFieldMapper.java

@@ -66,6 +66,7 @@ public class IdFieldMapper extends MetadataFieldMapper {
         public static final MappedFieldType FIELD_TYPE = new IdFieldType();
 
         static {
+            FIELD_TYPE.setTokenized(false);
             FIELD_TYPE.setIndexOptions(IndexOptions.NONE);
             FIELD_TYPE.setStored(false);
             FIELD_TYPE.setOmitNorms(true);
@@ -135,11 +136,6 @@ public class IdFieldMapper extends MetadataFieldMapper {
             return value.toString();
         }
 
-        @Override
-        public boolean useTermQueryWithQueryString() {
-            return true;
-        }
-
         @Override
         public Query termQuery(Object value, @Nullable QueryShardContext context) {
             if (indexOptions() != IndexOptions.NONE || context == null) {

core/src/main/java/org/elasticsearch/index/mapper/internal/IndexFieldMapper.java

@@ -123,16 +123,6 @@ public class IndexFieldMapper extends MetadataFieldMapper {
             return CONTENT_TYPE;
         }
 
-        @Override
-        public boolean useTermQueryWithQueryString() {
-            // As we spoof the presence of an indexed field we have to override
-            // the default of returning false which otherwise leads MatchQuery
-            // et al to run an analyzer over the query string and then try to
-            // hit the search index. We need them to use our termQuery(..)
-            // method which checks index names
-            return true;
-        }
-
         /**
          * This termQuery impl looks at the context to determine the index that
          * is being queried and then returns a MATCH_ALL_QUERY or MATCH_NO_QUERY

core/src/main/java/org/elasticsearch/index/mapper/internal/ParentFieldMapper.java

@@ -72,6 +72,7 @@ public class ParentFieldMapper extends MetadataFieldMapper {
         public static final ParentFieldType FIELD_TYPE = new ParentFieldType();
 
         static {
+            FIELD_TYPE.setTokenized(false);
             FIELD_TYPE.setIndexOptions(IndexOptions.NONE);
             FIELD_TYPE.setHasDocValues(true);
             FIELD_TYPE.setDocValuesType(DocValuesType.SORTED);
@@ -185,14 +186,6 @@ public class ParentFieldMapper extends MetadataFieldMapper {
             return CONTENT_TYPE;
         }
 
-        /**
-         * We don't need to analyzer the text, and we need to convert it to UID...
-         */
-        @Override
-        public boolean useTermQueryWithQueryString() {
-            return true;
-        }
-
         @Override
         public Query termQuery(Object value, @Nullable QueryShardContext context) {
             return termsQuery(Collections.singletonList(value), context);

core/src/main/java/org/elasticsearch/index/mapper/internal/TypeFieldMapper.java

@@ -126,11 +126,6 @@ public class TypeFieldMapper extends MetadataFieldMapper {
             return value.toString();
         }
 
-        @Override
-        public boolean useTermQueryWithQueryString() {
-            return true;
-        }
-
         @Override
         public Query termQuery(Object value, @Nullable QueryShardContext context) {
             if (indexOptions() == IndexOptions.NONE) {

core/src/main/java/org/elasticsearch/index/search/MatchQuery.java

@@ -244,7 +244,7 @@ public class MatchQuery {
          * passing through QueryBuilder.
          */
         boolean noForcedAnalyzer = this.analyzer == null;
-        if (fieldType != null && fieldType.useTermQueryWithQueryString() && noForcedAnalyzer) {
+        if (fieldType != null && fieldType.tokenized() == false && noForcedAnalyzer) {
             return termQuery(fieldType, value);
         }
 

core/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java

@@ -28,6 +28,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested;
+import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryParseContext;
@@ -271,17 +272,18 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
                 localSortMode = MultiValueMode.fromString(sortMode.toString());
             }
 
-            if (fieldType.isNumeric() == false && (sortMode == SortMode.SUM || sortMode == SortMode.AVG || sortMode == SortMode.MEDIAN)) {
-                throw new QueryShardException(context, "we only support AVG, MEDIAN and SUM on number based fields");
-            }
-
             boolean reverse = (order == SortOrder.DESC);
             if (localSortMode == null) {
                 localSortMode = reverse ? MultiValueMode.MAX : MultiValueMode.MIN;
             }
 
             final Nested nested = resolveNested(context, nestedPath, nestedFilter);
-            IndexFieldData.XFieldComparatorSource fieldComparatorSource = context.getForField(fieldType)
+            IndexFieldData<?> fieldData = context.getForField(fieldType);
+            if (fieldData instanceof IndexNumericFieldData == false
+                    && (sortMode == SortMode.SUM || sortMode == SortMode.AVG || sortMode == SortMode.MEDIAN)) {
+                throw new QueryShardException(context, "we only support AVG, MEDIAN and SUM on number based fields");
+            }
+            IndexFieldData.XFieldComparatorSource fieldComparatorSource = fieldData
                     .comparatorSource(missing, localSortMode, nested);
             return new SortField(fieldType.name(), fieldComparatorSource, reverse);
         }

modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScriptEngineService.java

@@ -32,10 +32,10 @@ import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.fielddata.IndexFieldData;
+import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.core.DateFieldMapper;
-import org.elasticsearch.index.mapper.core.NumberFieldMapper;
 import org.elasticsearch.script.ClassPermission;
 import org.elasticsearch.script.CompiledScript;
 import org.elasticsearch.script.ExecutableScript;
@@ -193,12 +193,12 @@ public class ExpressionScriptEngineService extends AbstractComponent implements
                     if (fieldType == null) {
                         throw new ScriptException("Field [" + fieldname + "] used in expression does not exist in mappings");
                     }
-                    if (fieldType.isNumeric() == false) {
+
+                    IndexFieldData<?> fieldData = lookup.doc().fieldDataService().getForField(fieldType);
+                    if (fieldData instanceof IndexNumericFieldData == false) {
                         // TODO: more context (which expression?)
                         throw new ScriptException("Field [" + fieldname + "] used in expression must be numeric");
                     }
-
-                    IndexFieldData<?> fieldData = lookup.doc().fieldDataService().getForField((NumberFieldMapper.NumberFieldType) fieldType);
                     if (methodname == null) {
                         bindings.add(variable, new FieldDataValueSource(fieldData, MultiValueMode.MIN));
                     } else {

modules/lang-expression/src/test/java/org/elasticsearch/script/expression/MoreExpressionTests.java

@@ -319,7 +319,7 @@ public class MoreExpressionTests extends ESIntegTestCase {
     public void testNonNumericField() {
         client().prepareIndex("test", "doc", "1").setSource("text", "this is not a number").setRefresh(true).get();
         try {
-            buildRequest("doc['text']").get();
+            buildRequest("doc['text.keyword']").get();
             fail("Expected text field to cause execution failure");
         } catch (SearchPhaseExecutionException e) {
             assertThat(e.toString() + "should have contained ScriptException",