From 8e3e5bf7eb7f5e0ef6edf032c4da2626e8ecb0db Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 4 Jun 2018 16:06:47 +0200 Subject: [PATCH 1/4] Decouple MultiValueMode. 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. --- .../support/MultiValuesSource.java | 2 +- .../expression/DateMethodValueSource.java | 2 +- .../expression/DateObjectValueSource.java | 2 +- .../expression/FieldDataValueSource.java | 2 +- .../index/fielddata/FieldData.java | 91 +++++++---- .../DoubleValuesComparatorSource.java | 3 +- .../FloatValuesComparatorSource.java | 3 +- .../LongValuesComparatorSource.java | 3 +- .../functionscore/DecayFunctionBuilder.java | 4 +- .../elasticsearch/search/MultiValueMode.java | 58 ++----- .../metrics/max/MaxAggregator.java | 2 +- .../metrics/min/MinAggregator.java | 2 +- .../search/sort/GeoDistanceSortBuilder.java | 2 +- .../index/fielddata/FieldDataTests.java | 87 ++++++++++ .../ordinals/MultiOrdinalsTests.java | 3 +- .../search/MultiValueModeTests.java | 153 +++++++++--------- 16 files changed, 256 insertions(+), 163 deletions(-) diff --git a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java index 0274c1748dde5..86d1836721f10 100644 --- a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSource.java +++ b/modules/aggs-matrix-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/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java index b0bc7c203b63b..13845a88fe3f0 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateMethodValueSource.java +++ b/modules/lang-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/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java index 86abcbcbefabd..ee59892a7024f 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/DateObjectValueSource.java +++ b/modules/lang-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/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java index f2aaced37658e..3e49797bbacc8 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/FieldDataValueSource.java +++ b/modules/lang-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 index b5e1608957e4c..68b8f2c85325f 100644 --- 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/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java index 338d903a390b3..43bc19a12a384 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/DoubleValuesComparatorSource.java +++ b/server/src/main/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/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java index a61a715547a24..b271dd54bd7fd 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/FloatValuesComparatorSource.java +++ b/server/src/main/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/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java b/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java index aa206fe1baebe..362dde6099680 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/fieldcomparator/LongValuesComparatorSource.java +++ b/server/src/main/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 index 3712040b8de03..584a495cc5d22 100644 --- a/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java @@ -372,7 +372,7 @@ public boolean advanceExact(int docId) throws IOException { return false; } } - }, 0.0); + }); } @Override @@ -451,7 +451,7 @@ public boolean advanceExact(int docId) throws IOException { return false; } } - }, 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 index b2ee4b8ffbd5f..eaaa5f74fa4d5 100644 --- a/server/src/main/java/org/elasticsearch/search/MultiValueMode.java +++ b/server/src/main/java/org/elasticsearch/search/MultiValueMode.java @@ -411,29 +411,10 @@ 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() { @@ -441,8 +422,11 @@ public long longValue() throws IOException { @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,23 +513,10 @@ 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() { @@ -553,8 +524,11 @@ public double doubleValue() throws IOException { @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 index 8ef4d0b7e29d1..ff76e6637baf4 100644 --- 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 index f355f55139c04..e4b371514bdf9 100644 --- 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 index 1d488a58857df..d540bfcb3364d 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java @@ -637,7 +637,7 @@ protected NumericDocValues getNumericDocValues(LeafReaderContext context, String localPoints); final NumericDoubleValues selectedValues; if (nested == null) { - selectedValues = finalSortMode.select(distanceValues, Double.POSITIVE_INFINITY); + selectedValues = finalSortMode.select(distanceValues); } else { final BitSet rootDocs = nested.rootDocs(context); final DocIdSetIterator innerDocs = nested.innerDocs(context); diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java index 6236517dde0be..ac924aa83e45a 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/FieldDataTests.java @@ -137,4 +137,91 @@ public int docValueCount() { assertEquals(valueBits, asMultiLongs.nextValue()); assertSame(multiValues, FieldData.sortableLongBitsToDoubles(asMultiLongs)); } + + private static NumericDocValues asNumericDocValues(Long... values) { + return new AbstractNumericDocValues() { + + int docID = -1; + + @Override + public int docID() { + return docID; + } + + @Override + public boolean advanceExact(int target) throws IOException { + docID = target; + return target < values.length && values[target] != null; + } + + @Override + public long longValue() throws IOException { + return values[docID]; + } + }; + } + + public void testReplaceMissingLongs() throws IOException { + final NumericDocValues values = asNumericDocValues(null, 3L, 2L, null, 5L, null); + final NumericDocValues replaced = FieldData.replaceMissing(values, 4); + + assertTrue(replaced.advanceExact(0)); + assertEquals(4L, replaced.longValue()); + + assertTrue(replaced.advanceExact(1)); + assertEquals(3L, replaced.longValue()); + + assertTrue(replaced.advanceExact(2)); + assertEquals(2L, replaced.longValue()); + + assertTrue(replaced.advanceExact(3)); + assertEquals(4L, replaced.longValue()); + + assertTrue(replaced.advanceExact(4)); + assertEquals(5L, replaced.longValue()); + + assertTrue(replaced.advanceExact(5)); + assertEquals(4L, replaced.longValue()); + } + + private static NumericDoubleValues asNumericDoubleValues(Double... values) { + return new NumericDoubleValues() { + + int docID = -1; + + @Override + public boolean advanceExact(int target) throws IOException { + docID = target; + return target < values.length && values[target] != null; + } + + @Override + public double doubleValue() throws IOException { + return values[docID]; + } + }; + } + + public void testReplaceMissingDoubles() throws IOException { + final NumericDoubleValues values = asNumericDoubleValues(null, 1.3, 1.2, null, 1.5, null); + final NumericDoubleValues replaced = FieldData.replaceMissing(values, 1.4); + + assertTrue(replaced.advanceExact(0)); + assertEquals(1.4, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(1)); + assertEquals(1.3, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(2)); + assertEquals(1.2, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(3)); + assertEquals(1.4, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(4)); + assertEquals(1.5, replaced.doubleValue(), 0d); + + assertTrue(replaced.advanceExact(5)); + assertEquals(1.4, replaced.doubleValue(), 0d); + } } diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java index 1ae6197c547da..3656d59e788e4 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/ordinals/MultiOrdinalsTests.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.index.fielddata.ordinals; +import org.apache.lucene.index.DocValues; import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.index.SortedSetDocValues; import org.apache.lucene.util.packed.PackedInts; @@ -261,7 +262,7 @@ private void assertEquals(SortedSetDocValues docs, long[][] ordinalPlan) throws } } assertThat(docs.getValueCount(), equalTo(maxOrd)); - assertThat(FieldData.isMultiValued(docs), equalTo(true)); + assertNull(DocValues.unwrapSingleton(docs)); for (int doc = 0; doc < ordinalPlan.length; ++doc) { long[] ords = ordinalPlan[doc]; assertEquals(ords.length > 0, docs.advanceExact(doc)); diff --git a/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java b/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java index d9eb45013263d..b64f6ee0ee3d1 100644 --- a/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java +++ b/server/src/test/java/org/elasticsearch/search/MultiValueModeTests.java @@ -151,54 +151,55 @@ public int docValueCount() { } private void verifySortedNumeric(Supplier supplier, int maxDoc) throws IOException { - for (long missingValue : new long[] { 0, randomLong() }) { - for (MultiValueMode mode : MultiValueMode.values()) { - SortedNumericDocValues values = supplier.get(); - final NumericDocValues selected = mode.select(values, missingValue); - for (int i = 0; i < maxDoc; ++i) { - assertTrue(selected.advanceExact(i)); - final long actual = selected.longValue(); + for (MultiValueMode mode : MultiValueMode.values()) { + SortedNumericDocValues values = supplier.get(); + final NumericDocValues selected = mode.select(values); + for (int i = 0; i < maxDoc; ++i) { + Long actual = null; + if (selected.advanceExact(i)) { + actual = selected.longValue(); verifyLongValueCanCalledMoreThanOnce(selected, actual); + } - long expected = 0; - if (values.advanceExact(i) == false) { - expected = missingValue; + + Long expected = null; + if (values.advanceExact(i)) { + int numValues = values.docValueCount(); + if (mode == MultiValueMode.MAX) { + expected = Long.MIN_VALUE; + } else if (mode == MultiValueMode.MIN) { + expected = Long.MAX_VALUE; } else { - int numValues = values.docValueCount(); - if (mode == MultiValueMode.MAX) { - expected = Long.MIN_VALUE; + expected = 0L; + } + for (int j = 0; j < numValues; ++j) { + if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { + expected += values.nextValue(); } else if (mode == MultiValueMode.MIN) { - expected = Long.MAX_VALUE; + expected = Math.min(expected, values.nextValue()); + } else if (mode == MultiValueMode.MAX) { + expected = Math.max(expected, values.nextValue()); } - for (int j = 0; j < numValues; ++j) { - if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { - expected += values.nextValue(); - } else if (mode == MultiValueMode.MIN) { - expected = Math.min(expected, values.nextValue()); - } else if (mode == MultiValueMode.MAX) { - expected = Math.max(expected, values.nextValue()); + } + if (mode == MultiValueMode.AVG) { + expected = numValues > 1 ? Math.round((double)expected/(double)numValues) : expected; + } else if (mode == MultiValueMode.MEDIAN) { + int value = numValues/2; + if (numValues % 2 == 0) { + for (int j = 0; j < value - 1; ++j) { + values.nextValue(); } - } - if (mode == MultiValueMode.AVG) { - expected = numValues > 1 ? Math.round((double)expected/(double)numValues) : expected; - } else if (mode == MultiValueMode.MEDIAN) { - int value = numValues/2; - if (numValues % 2 == 0) { - for (int j = 0; j < value - 1; ++j) { - values.nextValue(); - } - expected = Math.round(((double) values.nextValue() + values.nextValue())/2.0); - } else { - for (int j = 0; j < value; ++j) { - values.nextValue(); - } - expected = values.nextValue(); + expected = Math.round(((double) values.nextValue() + values.nextValue())/2.0); + } else { + for (int j = 0; j < value; ++j) { + values.nextValue(); } + expected = values.nextValue(); } } - - assertEquals(mode.toString() + " docId=" + i, expected, actual); } + + assertEquals(mode.toString() + " docId=" + i, expected, actual); } } } @@ -326,54 +327,54 @@ public int docValueCount() { } private void verifySortedNumericDouble(Supplier supplier, int maxDoc) throws IOException { - for (long missingValue : new long[] { 0, randomLong() }) { - for (MultiValueMode mode : MultiValueMode.values()) { - SortedNumericDoubleValues values = supplier.get(); - final NumericDoubleValues selected = mode.select(values, missingValue); - for (int i = 0; i < maxDoc; ++i) { - assertTrue(selected.advanceExact(i)); - final double actual = selected.doubleValue(); + for (MultiValueMode mode : MultiValueMode.values()) { + SortedNumericDoubleValues values = supplier.get(); + final NumericDoubleValues selected = mode.select(values); + for (int i = 0; i < maxDoc; ++i) { + Double actual = null; + if (selected.advanceExact(i)) { + actual = selected.doubleValue(); verifyDoubleValueCanCalledMoreThanOnce(selected, actual); + } - double expected = 0.0; - if (values.advanceExact(i) == false) { - expected = missingValue; + Double expected = null; + if (values.advanceExact(i)) { + int numValues = values.docValueCount(); + if (mode == MultiValueMode.MAX) { + expected = Double.NEGATIVE_INFINITY; + } else if (mode == MultiValueMode.MIN) { + expected = Double.POSITIVE_INFINITY; } else { - int numValues = values.docValueCount(); - if (mode == MultiValueMode.MAX) { - expected = Long.MIN_VALUE; + expected = 0d; + } + for (int j = 0; j < numValues; ++j) { + if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { + expected += values.nextValue(); } else if (mode == MultiValueMode.MIN) { - expected = Long.MAX_VALUE; + expected = Math.min(expected, values.nextValue()); + } else if (mode == MultiValueMode.MAX) { + expected = Math.max(expected, values.nextValue()); } - for (int j = 0; j < numValues; ++j) { - if (mode == MultiValueMode.SUM || mode == MultiValueMode.AVG) { - expected += values.nextValue(); - } else if (mode == MultiValueMode.MIN) { - expected = Math.min(expected, values.nextValue()); - } else if (mode == MultiValueMode.MAX) { - expected = Math.max(expected, values.nextValue()); + } + if (mode == MultiValueMode.AVG) { + expected = expected/numValues; + } else if (mode == MultiValueMode.MEDIAN) { + int value = numValues/2; + if (numValues % 2 == 0) { + for (int j = 0; j < value - 1; ++j) { + values.nextValue(); } - } - if (mode == MultiValueMode.AVG) { - expected = expected/numValues; - } else if (mode == MultiValueMode.MEDIAN) { - int value = numValues/2; - if (numValues % 2 == 0) { - for (int j = 0; j < value - 1; ++j) { - values.nextValue(); - } - expected = (values.nextValue() + values.nextValue())/2.0; - } else { - for (int j = 0; j < value; ++j) { - values.nextValue(); - } - expected = values.nextValue(); + expected = (values.nextValue() + values.nextValue())/2.0; + } else { + for (int j = 0; j < value; ++j) { + values.nextValue(); } + expected = values.nextValue(); } } - - assertEquals(mode.toString() + " docId=" + i, expected, actual, 0.1); } + + assertEquals(mode.toString() + " docId=" + i, expected, actual); } } } From b8a9132f7c65cd5d93c22e18b4305e130dfcb13a Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 4 Jun 2018 16:52:44 +0200 Subject: [PATCH 2/4] iter --- .../org/elasticsearch/search/sort/GeoDistanceSortBuilder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java index d540bfcb3364d..cfa5a240dea53 100644 --- 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); + selectedValues = FieldData.replaceMissing(finalSortMode.select(distanceValues), Double.POSITIVE_INFINITY); } else { final BitSet rootDocs = nested.rootDocs(context); final DocIdSetIterator innerDocs = nested.innerDocs(context); From 02f6313453ff621ac335d87212277057d5a6cd91 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 4 Jun 2018 16:54:52 +0200 Subject: [PATCH 3/4] iter --- .../index/query/functionscore/DecayFunctionBuilder.java | 9 +++++---- .../search/functionscore/DecayFunctionScoreIT.java | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) 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 index 584a495cc5d22..54c25b40501d2 100644 --- 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); } @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); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java b/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java index d6acdf11cb2ab..26b843c52dcf8 100644 --- a/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java +++ b/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java @@ -547,7 +547,7 @@ public void testValueMissingLin() throws Exception { new FilterFunctionBuilder(linearDecayFunction("num2", "0.0", "1")) }).scoreMode(FunctionScoreQuery.ScoreMode.MULTIPLY)))); - SearchResponse sr = response.actionGet(); + SearchResponse sr = response.actionGet();System.out.println(sr); assertNoFailures(sr); SearchHits sh = sr.getHits(); From f006e05d88b0cb7e09ff6be0612060942693fbfc Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 4 Jun 2018 17:59:25 +0200 Subject: [PATCH 4/4] iter --- .../search/functionscore/DecayFunctionScoreIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java b/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java index 26b843c52dcf8..d6acdf11cb2ab 100644 --- a/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java +++ b/server/src/test/java/org/elasticsearch/search/functionscore/DecayFunctionScoreIT.java @@ -547,7 +547,7 @@ public void testValueMissingLin() throws Exception { new FilterFunctionBuilder(linearDecayFunction("num2", "0.0", "1")) }).scoreMode(FunctionScoreQuery.ScoreMode.MULTIPLY)))); - SearchResponse sr = response.actionGet();System.out.println(sr); + SearchResponse sr = response.actionGet(); assertNoFailures(sr); SearchHits sh = sr.getHits();