Decouple MultiValueMode. (#31075)

Currently this class takes care of moth selecting the relevant value, and replacing missing values if any. This is fine for sorting, which always needs to do both at the same time, but we also have a number of aggregations and script utils that need to retain information about missing values so this change proposes to decouple selection of the relevant value and replacement of missing values.
elastic · Jun 5, 2018 · 0d89d80 · 0d89d80
1 parent 62b4faa
commit 0d89d80
Show file tree

Hide file tree

Showing 16 changed files with 260 additions and 165 deletions.
diff --git a/...-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java b/...-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java
@@ -47,7 +47,7 @@ public NumericDoubleValues getField(final int ordinal, LeafReaderContext ctx) th
             if (ordinal > names.length) {
                 throw new IndexOutOfBoundsException("ValuesSource array index " + ordinal + " out of bounds");
             }
-            return multiValueMode.select(values[ordinal].doubleValues(ctx), Double.NEGATIVE_INFINITY);
+            return multiValueMode.select(values[ordinal].doubleValues(ctx));
         }
     }
 

diff --git a/...g-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java b/...g-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java
@@ -54,7 +54,7 @@ class DateMethodValueSource extends FieldDataValueSource {
     public FunctionValues getValues(Map context, LeafReaderContext leaf) throws IOException {
         AtomicNumericFieldData leafData = (AtomicNumericFieldData) fieldData.load(leaf);
         final Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.ROOT);
-        NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues(), 0d);
+        NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues());
         return new DoubleDocValues(this) {
             @Override
             public double doubleVal(int docId) throws IOException {

diff --git a/...g-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java b/...g-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java
@@ -56,7 +56,7 @@ class DateObjectValueSource extends FieldDataValueSource {
     public FunctionValues getValues(Map context, LeafReaderContext leaf) throws IOException {
         AtomicNumericFieldData leafData = (AtomicNumericFieldData) fieldData.load(leaf);
         MutableDateTime joda = new MutableDateTime(0, DateTimeZone.UTC);
-        NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues(), 0d);
+        NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues());
         return new DoubleDocValues(this) {
             @Override
             public double doubleVal(int docId) throws IOException {

diff --git a/...ng-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java b/...ng-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java
@@ -68,7 +68,7 @@ public int hashCode() {
     @SuppressWarnings("rawtypes") // ValueSource uses a rawtype
     public FunctionValues getValues(Map context, LeafReaderContext leaf) throws IOException {
         AtomicNumericFieldData leafData = (AtomicNumericFieldData) fieldData.load(leaf);
-        NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues(), 0d);
+        NumericDoubleValues docValues = multiValueMode.select(leafData.getDoubleValues());
         return new DoubleDocValues(this) {
           @Override
           public double doubleVal(int doc) throws IOException {

diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/FieldData.java
@@ -291,38 +291,6 @@ public static boolean isMultiValued(SortedSetDocValues values) {
         return DocValues.unwrapSingleton(values) == null;
     }
 
-    /**
-     * Returns whether the provided values *might* be multi-valued. There is no
-     * guarantee that this method will return {@code false} in the single-valued case.
-     */
-    public static boolean isMultiValued(SortedNumericDocValues values) {
-        return DocValues.unwrapSingleton(values) == null;
-    }
-
-    /**
-     * Returns whether the provided values *might* be multi-valued. There is no
-     * guarantee that this method will return {@code false} in the single-valued case.
-     */
-    public static boolean isMultiValued(SortedNumericDoubleValues values) {
-        return unwrapSingleton(values) == null;
-    }
-
-    /**
-     * Returns whether the provided values *might* be multi-valued. There is no
-     * guarantee that this method will return {@code false} in the single-valued case.
-     */
-    public static boolean isMultiValued(SortedBinaryDocValues values) {
-        return unwrapSingleton(values) != null;
-    }
-
-    /**
-     * Returns whether the provided values *might* be multi-valued. There is no
-     * guarantee that this method will return {@code false} in the single-valued case.
-     */
-    public static boolean isMultiValued(MultiGeoPointValues values) {
-        return unwrapSingleton(values) == null;
-    }
-
     /**
      * Return a {@link String} representation of the provided values. That is
      * typically used for scripts or for the `map` execution mode of terms aggs.
@@ -555,4 +523,63 @@ public long nextValue() throws IOException {
         }
 
     }
+
+    /**
+     * Return a {@link NumericDocValues} instance that has a value for every
+     * document, returns the same value as {@code values} if there is a value
+     * for the current document and {@code missing} otherwise.
+     */
+    public static NumericDocValues replaceMissing(NumericDocValues values, long missing) {
+        return new AbstractNumericDocValues() {
+
+            private long value;
+
+            @Override
+            public int docID() {
+                return values.docID();
+            }
+
+            @Override
+            public boolean advanceExact(int target) throws IOException {
+                if (values.advanceExact(target)) {
+                    value = values.longValue();
+                } else {
+                    value = missing;
+                }
+                return true;
+            }
+
+            @Override
+            public long longValue() throws IOException {
+                return value;
+            }
+        };
+    }
+
+    /**
+     * Return a {@link NumericDoubleValues} instance that has a value for every
+     * document, returns the same value as {@code values} if there is a value
+     * for the current document and {@code missing} otherwise.
+     */
+    public static NumericDoubleValues replaceMissing(NumericDoubleValues values, double missing) {
+        return new NumericDoubleValues() {
+
+            private double value;
+
+            @Override
+            public boolean advanceExact(int target) throws IOException {
+                if (values.advanceExact(target)) {
+                    value = values.doubleValue();
+                } else {
+                    value = missing;
+                }
+                return true;
+            }
+
+            @Override
+            public double doubleValue() throws IOException {
+                return value;
+            }
+        };
+    }
 }
diff --git a/.../java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java b/.../java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java
@@ -27,6 +27,7 @@
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BitSet;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.index.fielddata.NumericDoubleValues;
@@ -71,7 +72,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String
                 final SortedNumericDoubleValues values = getValues(context);
                 final NumericDoubleValues selectedValues;
                 if (nested == null) {
-                    selectedValues = sortMode.select(values, dMissingValue);
+                    selectedValues = FieldData.replaceMissing(sortMode.select(values), dMissingValue);
                 } else {
                     final BitSet rootDocs = nested.rootDocs(context);
                     final DocIdSetIterator innerDocs = nested.innerDocs(context);

diff --git a/...n/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java b/...n/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java
@@ -25,6 +25,7 @@
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BitSet;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.index.fielddata.NumericDoubleValues;
@@ -63,7 +64,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String
                 final SortedNumericDoubleValues values = indexFieldData.load(context).getDoubleValues();
                 final NumericDoubleValues selectedValues;
                 if (nested == null) {
-                    selectedValues = sortMode.select(values, dMissingValue);
+                    selectedValues = FieldData.replaceMissing(sortMode.select(values), dMissingValue);
                 } else {
                     final BitSet rootDocs = nested.rootDocs(context);
                     final DocIdSetIterator innerDocs = nested.innerDocs(context);

diff --git a/...in/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/...in/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java
@@ -26,6 +26,7 @@
 import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BitSet;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.search.MultiValueMode;
@@ -62,7 +63,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String
                 final SortedNumericDocValues values = indexFieldData.load(context).getLongValues();
                 final NumericDocValues selectedValues;
                 if (nested == null) {
-                    selectedValues = sortMode.select(values, dMissingValue);
+                    selectedValues = FieldData.replaceMissing(sortMode.select(values), dMissingValue);
                 } else {
                     final BitSet rootDocs = nested.rootDocs(context);
                     final DocIdSetIterator innerDocs = nested.innerDocs(context);

diff --git a/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java b/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java
@@ -40,6 +40,7 @@
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData;
 import org.elasticsearch.index.fielddata.MultiGeoPointValues;
@@ -354,7 +355,7 @@ public boolean needsScores() {
         @Override
         protected NumericDoubleValues distance(LeafReaderContext context) {
             final MultiGeoPointValues geoPointValues = fieldData.load(context).getGeoPointValues();
-            return mode.select(new SortingNumericDoubleValues() {
+            return FieldData.replaceMissing(mode.select(new SortingNumericDoubleValues() {
                 @Override
                 public boolean advanceExact(int docId) throws IOException {
                     if (geoPointValues.advanceExact(docId)) {
@@ -372,7 +373,7 @@ public boolean advanceExact(int docId) throws IOException {
                         return false;
                     }
                 }
-            }, 0.0);
+            }), 0);
         }
 
         @Override
@@ -436,7 +437,7 @@ public boolean needsScores() {
         @Override
         protected NumericDoubleValues distance(LeafReaderContext context) {
             final SortedNumericDoubleValues doubleValues = fieldData.load(context).getDoubleValues();
-            return mode.select(new SortingNumericDoubleValues() {
+            return FieldData.replaceMissing(mode.select(new SortingNumericDoubleValues() {
                 @Override
                 public boolean advanceExact(int docId) throws IOException {
                     if (doubleValues.advanceExact(docId)) {
@@ -451,7 +452,7 @@ public boolean advanceExact(int docId) throws IOException {
                         return false;
                     }
                 }
-            }, 0.0);
+            }), 0);
         }
 
         @Override

diff --git a/server/src/main/java/org/elasticsearch/search/MultiValueMode.java b/server/src/main/java/org/elasticsearch/search/MultiValueMode.java
@@ -411,38 +411,22 @@ public static MultiValueMode fromString(String sortMode) {
      *
      * Allowed Modes: SUM, AVG, MEDIAN, MIN, MAX
      */
-    public NumericDocValues select(final SortedNumericDocValues values, final long missingValue) {
+    public NumericDocValues select(final SortedNumericDocValues values) {
         final NumericDocValues singleton = DocValues.unwrapSingleton(values);
         if (singleton != null) {
-            return new AbstractNumericDocValues() {
-
-                private long value;
-
-                @Override
-                public boolean advanceExact(int target) throws IOException {
-                    this.value = singleton.advanceExact(target) ? singleton.longValue() : missingValue;
-                    return true;
-                }
-
-                @Override
-                public int docID() {
-                    return singleton.docID();
-                }
-
-                @Override
-                public long longValue() throws IOException {
-                    return this.value;
-                }
-            };
+            return singleton;
         } else {
             return new AbstractNumericDocValues() {
 
                 private long value;
 
                 @Override
                 public boolean advanceExact(int target) throws IOException {
-                    this.value = values.advanceExact(target) ? pick(values) : missingValue;
-                    return true;
+                    if (values.advanceExact(target)) {
+                        value = pick(values);
+                        return true;
+                    }
+                    return false;
                 }
 
                 @Override
@@ -476,7 +460,7 @@ protected long pick(SortedNumericDocValues values) throws IOException {
      */
     public NumericDocValues select(final SortedNumericDocValues values, final long missingValue, final BitSet parentDocs, final DocIdSetIterator childDocs, int maxDoc) throws IOException {
         if (parentDocs == null || childDocs == null) {
-            return select(DocValues.emptySortedNumeric(maxDoc), missingValue);
+            return FieldData.replaceMissing(DocValues.emptyNumeric(), missingValue);
         }
 
         return new AbstractNumericDocValues() {
@@ -529,32 +513,22 @@ protected long pick(SortedNumericDocValues values, long missingValue, DocIdSetIt
      *
      * Allowed Modes: SUM, AVG, MEDIAN, MIN, MAX
      */
-    public NumericDoubleValues select(final SortedNumericDoubleValues values, final double missingValue) {
+    public NumericDoubleValues select(final SortedNumericDoubleValues values) {
         final NumericDoubleValues singleton = FieldData.unwrapSingleton(values);
         if (singleton != null) {
-            return new NumericDoubleValues() {
-                private double value;
-
-                @Override
-                public boolean advanceExact(int doc) throws IOException {
-                    this.value = singleton.advanceExact(doc) ? singleton.doubleValue() : missingValue;
-                    return true;
-                }
-
-                @Override
-                public double doubleValue() throws IOException {
-                    return this.value;
-                }
-            };
+            return singleton;
         } else {
             return new NumericDoubleValues() {
 
                 private double value;
 
                 @Override
                 public boolean advanceExact(int target) throws IOException {
-                    value = values.advanceExact(target) ? pick(values) : missingValue;
-                    return true;
+                    if (values.advanceExact(target)) {
+                        value = pick(values);
+                        return true;
+                    }
+                    return false;
                 }
 
                 @Override
@@ -583,7 +557,7 @@ protected double pick(SortedNumericDoubleValues values) throws IOException {
      */
     public NumericDoubleValues select(final SortedNumericDoubleValues values, final double missingValue, final BitSet parentDocs, final DocIdSetIterator childDocs, int maxDoc) throws IOException {
         if (parentDocs == null || childDocs == null) {
-            return select(FieldData.emptySortedNumericDoubles(), missingValue);
+            return FieldData.replaceMissing(FieldData.emptyNumericDouble(), missingValue);
         }
 
         return new NumericDoubleValues() {

diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/max/MaxAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/max/MaxAggregator.java
@@ -72,7 +72,7 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx,
     }
         final BigArrays bigArrays = context.bigArrays();
         final SortedNumericDoubleValues allValues = valuesSource.doubleValues(ctx);
-        final NumericDoubleValues values = MultiValueMode.MAX.select(allValues, Double.NEGATIVE_INFINITY);
+        final NumericDoubleValues values = MultiValueMode.MAX.select(allValues);
         return new LeafBucketCollectorBase(sub, allValues) {
 
             @Override

diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/min/MinAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/min/MinAggregator.java
@@ -71,7 +71,7 @@ public LeafBucketCollector getLeafCollector(LeafReaderContext ctx,
         }
         final BigArrays bigArrays = context.bigArrays();
         final SortedNumericDoubleValues allValues = valuesSource.doubleValues(ctx);
-        final NumericDoubleValues values = MultiValueMode.MIN.select(allValues, Double.POSITIVE_INFINITY);
+        final NumericDoubleValues values = MultiValueMode.MIN.select(allValues);
         return new LeafBucketCollectorBase(sub, allValues) {
 
             @Override

diff --git a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java
@@ -41,6 +41,7 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentParser.Token;
+import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested;
 import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
@@ -637,7 +638,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String
                                 localPoints);
                         final NumericDoubleValues selectedValues;
                         if (nested == null) {
-                            selectedValues = finalSortMode.select(distanceValues, Double.POSITIVE_INFINITY);
+                            selectedValues = FieldData.replaceMissing(finalSortMode.select(distanceValues), Double.POSITIVE_INFINITY);
                         } else {
                             final BitSet rootDocs = nested.rootDocs(context);
                             final DocIdSetIterator innerDocs = nested.innerDocs(context);