Remove getValue and getValues from Field (elastic#79516)

With duck-typing in Painless and potentially non-dynamic languages requiring a cast under the
current design, this change removes unnecessary methods from the Field base class. The removal of
getValue and getValues allows sub classes to return primitives from these methods and opens up
potential design space for performance improvements.

Note this removes getLong from the unsigned long field; however, this is currently undocumented and
unused behavior.

Author: jdconrad

modules/lang-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt

@@ -13,9 +13,6 @@ class org.elasticsearch.script.field.Field @dynamic_type {
   String getName()
   boolean isEmpty()
   int size()
-  List getValues()
-  def getValue(def)
-  def getValue(int, def)
 }
 
 class org.elasticsearch.script.DocBasedScript {

server/src/main/java/org/elasticsearch/index/fielddata/LeafFieldData.java

@@ -28,7 +28,7 @@ public interface LeafFieldData extends Accountable, Releasable {
     /**
      * Returns an {@code Field} for use in accessing field values in scripting.
      */
-    default DocValuesField<?> getScriptField(String name) {
+    default DocValuesField getScriptField(String name) {
         throw new UnsupportedOperationException();
     }
 

server/src/main/java/org/elasticsearch/script/DocBasedScript.java

@@ -23,14 +23,14 @@ public DocBasedScript(DocReader docReader) {
         this.docReader = docReader;
     }
 
-    public Field<?> field(String fieldName) {
+    public Field field(String fieldName) {
         if (docReader == null) {
-            return new EmptyField<>(fieldName);
+            return new EmptyField(fieldName);
         }
         return docReader.field(fieldName);
     }
 
-    public Stream<Field<?>> fields(String fieldGlob) {
+    public Stream<Field> fields(String fieldGlob) {
         if (docReader == null) {
             return Stream.empty();
         }

server/src/main/java/org/elasticsearch/script/DocReader.java

@@ -22,10 +22,10 @@
  */
 public interface DocReader {
     /** New-style field access */
-    Field<?> field(String fieldName);
+    Field field(String fieldName);
 
     /** New-style field iterator */
-    Stream<Field<?>> fields(String fieldGlob);
+    Stream<Field> fields(String fieldGlob);
 
     /** Set the underlying docId */
     void setDocument(int docID);

server/src/main/java/org/elasticsearch/script/DocValuesDocReader.java

@@ -39,19 +39,19 @@ public DocValuesDocReader(SearchLookup searchLookup, LeafReaderContext leafConte
     }
 
     @Override
-    public Field<?> field(String fieldName) {
+    public Field field(String fieldName) {
         LeafDocLookup leafDocLookup = leafSearchLookup.doc();
 
         if (leafDocLookup.containsKey(fieldName) == false) {
-            return new EmptyField<>(fieldName);
+            return new EmptyField(fieldName);
         }
 
         return leafDocLookup.getScriptField(fieldName);
     }
 
 
     @Override
-    public Stream<Field<?>> fields(String fieldGlob) {
+    public Stream<Field> fields(String fieldGlob) {
         throw new UnsupportedOperationException("not implemented");
     }
 

server/src/main/java/org/elasticsearch/script/field/DocValuesField.java

@@ -10,7 +10,7 @@
 
 import java.io.IOException;
 
-public interface DocValuesField<T> extends Field<T> {
+public interface DocValuesField extends Field {
 
     /** Set the current document ID. */
     void setNextDocId(int docId) throws IOException;

server/src/main/java/org/elasticsearch/script/field/EmptyField.java

@@ -8,13 +8,10 @@
 
 package org.elasticsearch.script.field;
 
-import java.util.Collections;
-import java.util.List;
-
 /**
  * Script field with no mapping, always returns {@code defaultValue}.
  */
-public class EmptyField<T> implements Field<T> {
+public class EmptyField implements Field {
 
     private final String name;
 
@@ -36,19 +33,4 @@ public boolean isEmpty() {
     public int size() {
         return 0;
     }
-
-    @Override
-    public List<T> getValues() {
-        return Collections.emptyList();
-    }
-
-    @Override
-    public T getValue(T defaultValue) {
-        return defaultValue;
-    }
-
-    @Override
-    public T getValue(int index, T defaultValue) {
-        return defaultValue;
-    }
 }

server/src/main/java/org/elasticsearch/script/field/Field.java

@@ -9,10 +9,8 @@
 
 package org.elasticsearch.script.field;
 
-import java.util.List;
-
 /** A field in a document accessible via scripting. */
-public interface Field<T> {
+public interface Field {
 
     /** Returns the name of this field. */
     String getName();
@@ -22,13 +20,4 @@ public interface Field<T> {
 
     /** Returns the number of values this field has. */
     int size();
-
-    /** Get all values of a multivalued field.  If {@code isEmpty()} this returns an empty list. */
-    List<T> getValues();
-
-    /** Get the first value of a field, if {@code isEmpty()} return defaultValue instead */
-    T getValue(T defaultValue);
-
-    /** Get the value of a field as the specified index. */
-    T getValue(int index, T defaultValue);
 }

server/src/main/java/org/elasticsearch/search/lookup/LeafDocLookup.java

@@ -26,13 +26,13 @@
 import java.util.function.Function;
 
 public class LeafDocLookup implements Map<String, ScriptDocValues<?>> {
-
+    
     private static final DeprecationLogger DEPRECATION_LOGGER =  DeprecationLogger.getLogger(LeafDocLookup.class);
     static final String TYPES_DEPRECATION_KEY = "type-field-doc-lookup";
     static final String TYPES_DEPRECATION_MESSAGE =
             "[types removal] Looking up doc types [_type] in scripts is deprecated.";
 
-    private final Map<String, DocValuesField<?>> localCacheScriptFieldData = new HashMap<>(4);
+    private final Map<String, DocValuesField> localCacheScriptFieldData = new HashMap<>(4);
     private final Map<String, ScriptDocValues<?>> localCacheFieldData = new HashMap<>(4);
     private final Function<String, MappedFieldType> fieldTypeLookup;
     private final Function<MappedFieldType, IndexFieldData<?>> fieldDataLookup;
@@ -52,8 +52,8 @@ public void setDocument(int docId) {
         this.docId = docId;
     }
 
-    public DocValuesField<?> getScriptField(String fieldName) {
-        DocValuesField<?> field = localCacheScriptFieldData.get(fieldName);
+    public DocValuesField getScriptField(String fieldName) {
+        DocValuesField field = localCacheScriptFieldData.get(fieldName);
 
         if (field == null) {
             final MappedFieldType fieldType = fieldTypeLookup.apply(fieldName);
@@ -64,9 +64,9 @@ public DocValuesField<?> getScriptField(String fieldName) {
 
             // Load the field data on behalf of the script. Otherwise, it would require
             // additional permissions to deal with pagedbytes/ramusagestimator/etc.
-            field = AccessController.doPrivileged(new PrivilegedAction<DocValuesField<?>>() {
+            field = AccessController.doPrivileged(new PrivilegedAction<DocValuesField>() {
                 @Override
-                public DocValuesField<?> run() {
+                public DocValuesField run() {
                     return fieldDataLookup.apply(fieldType).load(reader).getScriptField(fieldName);
                 }
             });

x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongDocValuesField.java

@@ -20,7 +20,7 @@
 import static org.elasticsearch.search.DocValueFormat.MASK_2_63;
 import static org.elasticsearch.xpack.unsignedlong.UnsignedLongFieldMapper.BIGINTEGER_2_64_MINUS_ONE;
 
-public class UnsignedLongDocValuesField implements UnsignedLongField, DocValuesField<Long> {
+public class UnsignedLongDocValuesField implements UnsignedLongField, DocValuesField {
 
     private final SortedNumericDocValues input;
     private long[] values = new long[0];
@@ -89,26 +89,12 @@ public List<Long> getValues() {
     }
 
     @Override
-    public Long getValue(Long defaultValue) {
+    public long getValue(long defaultValue) {
         return getValue(0, defaultValue);
     }
 
     @Override
-    public Long getValue(int index, Long defaultValue) {
-        if (isEmpty() || index < 0 || index >= count) {
-            return defaultValue;
-        }
-
-        return toFormatted(index);
-    }
-
-    @Override
-    public long getLong(long defaultValue) {
-        return getLong(0, defaultValue);
-    }
-
-    @Override
-    public long getLong(int index, long defaultValue) {
+    public long getValue(int index, long defaultValue) {
         if (isEmpty() || index < 0 || index >= count) {
             return defaultValue;
         }

x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongField.java

@@ -12,13 +12,16 @@
 import java.math.BigInteger;
 import java.util.List;
 
-public interface UnsignedLongField extends Field<Long> {
+public interface UnsignedLongField extends Field {
 
     /** Returns the 0th index value as an {@code long} if it exists, otherwise {@code defaultValue}. */
-    long getLong(long defaultValue);
+    long getValue(long defaultValue);
 
     /** Returns the value at {@code index} as an {@code long} if it exists, otherwise {@code defaultValue}. */
-    long getLong(int index, long defaultValue);
+    long getValue(int index, long defaultValue);
+
+    /** Return all the values as a {@code List}. */
+    List<Long> getValues();
 
     /** Returns the 0th index value as a {@code BigInteger} if it exists, otherwise {@code defaultValue}. */
     BigInteger getBigInteger(BigInteger defaultValue);

x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java

@@ -79,7 +79,7 @@ public ScriptDocValues<?> getScriptValues() {
     }
 
     @Override
-    public DocValuesField<?> getScriptField(String name) {
+    public DocValuesField getScriptField(String name) {
         return new UnsignedLongDocValuesField(getLongValues(), name);
     }
 

x-pack/plugin/mapper-unsigned-long/src/main/resources/org/elasticsearch/xpack/unsignedlong/org.elasticsearch.xpack.unsignedlong.txt

@@ -12,8 +12,9 @@ class org.elasticsearch.xpack.unsignedlong.UnsignedLongScriptDocValues {
 }
 
 class org.elasticsearch.xpack.unsignedlong.UnsignedLongField @dynamic_type {
-  long getLong(long)
-  long getLong(int, long)
+  long getValue(long)
+  long getValue(int, long)
+  List getValues()
   BigInteger getBigInteger(BigInteger)
   BigInteger getBigInteger(int, BigInteger)
   List getBigIntegers()

x-pack/plugin/mapper-unsigned-long/src/yamlRestTest/resources/rest-api-spec/test/50_script_values.yml

@@ -46,21 +46,7 @@ setup:
   - match: { hits.hits.2.fields.scripted_ul.0: -9223372036854775808 }
   - match: { hits.hits.3.fields.scripted_ul.0: 9223372036854775807 }
   - match: { hits.hits.4.fields.scripted_ul.0: 0 }
-  - do:
-      search:
-        index: test1
-        body:
-          sort: [ { ul: desc } ]
-          script_fields:
-            scripted_ul:
-              script:
-                source: "field('ul').getLong(1000L)"
 
-  - match: { hits.hits.0.fields.scripted_ul.0: -1 }
-  - match: { hits.hits.1.fields.scripted_ul.0: -3 }
-  - match: { hits.hits.2.fields.scripted_ul.0: -9223372036854775808 }
-  - match: { hits.hits.3.fields.scripted_ul.0: 9223372036854775807 }
-  - match: { hits.hits.4.fields.scripted_ul.0: 0 }
   - do:
       search:
         index: test1