From 8a05b670ca1ae160319de6cf60cf0e95581ea691 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 3 Mar 2020 16:08:10 +0100 Subject: [PATCH] Address MinAndMax generics warnings (#52642) `MinAndMax` encapsulates min and max values for a shard. It uses generics to make sure that the values are of the same type and are also comparable. Though there are warnings whenever this class is currently used, which are addressed with this commit. Relates to #49092 --- .../search/CanMatchPreFilterSearchPhase.java | 2 +- .../search/sort/FieldSortBuilder.java | 112 ++++++++---------- .../elasticsearch/search/sort/MinAndMax.java | 21 ++-- .../CanMatchPreFilterSearchPhaseTests.java | 2 +- 4 files changed, 63 insertions(+), 74 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java index 59debedbcf8..9d8b159b8e0 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -133,7 +133,7 @@ final class CanMatchPreFilterSearchPhase extends AbstractSearchAsyncAction shardComparator(GroupShardsIterator shardsIts, MinAndMax[] minAndMaxes, SortOrder order) { - final Comparator comparator = Comparator.comparing(index -> minAndMaxes[index], MinAndMax.getComparator(order)); + final Comparator comparator = Comparator.comparing(index -> minAndMaxes[index], MinAndMax.getComparator(order)); return comparator.thenComparing(index -> shardsIts.get(index).shardId()); } diff --git a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java index 510ee4bf580..b6cd3b665e8 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -499,46 +499,6 @@ public class FieldSortBuilder extends SortBuilder { return source.sorts().get(0) instanceof FieldSortBuilder ? (FieldSortBuilder) source.sorts().get(0) : null; } - /** - * Return a {@link Function} that converts a serialized point into a {@link Number} according to the provided - * {@link SortField}. This is needed for {@link SortField} that converts values from one type to another using - * {@link FieldSortBuilder#setNumericType(String)} )} (e.g.: long to double). - */ - private static Function numericPointConverter(SortField sortField, NumberFieldType numberFieldType) { - switch (IndexSortConfig.getSortFieldType(sortField)) { - case LONG: - return v -> numberFieldType.parsePoint(v).longValue(); - - case INT: - return v -> numberFieldType.parsePoint(v).intValue(); - - case DOUBLE: - return v -> numberFieldType.parsePoint(v).doubleValue(); - - case FLOAT: - return v -> numberFieldType.parsePoint(v).floatValue(); - - default: - return v -> null; - } - } - - /** - * Return a {@link Function} that converts a serialized date point into a {@link Long} according to the provided - * {@link NumericType}. - */ - private static Function datePointConverter(DateFieldType dateFieldType, String numericTypeStr) { - if (numericTypeStr != null) { - NumericType numericType = resolveNumericType(numericTypeStr); - if (dateFieldType.resolution() == MILLISECONDS && numericType == NumericType.DATE_NANOSECONDS) { - return v -> DateUtils.toNanoSeconds(LongPoint.decodeDimension(v, 0)); - } else if (dateFieldType.resolution() == NANOSECONDS && numericType == NumericType.DATE) { - return v -> DateUtils.toMilliSeconds(LongPoint.decodeDimension(v, 0)); - } - } - return v -> LongPoint.decodeDimension(v, 0); - } - /** * Return the {@link MinAndMax} indexed value from the provided {@link FieldSortBuilder} or null if unknown. * The value can be extracted on non-nested indexed mapped fields of type keyword, numeric or date, other fields @@ -555,41 +515,73 @@ public class FieldSortBuilder extends SortBuilder { if (reader == null || (fieldType == null || fieldType.indexOptions() == IndexOptions.NONE)) { return null; } - String fieldName = fieldType.name(); switch (IndexSortConfig.getSortFieldType(sortField)) { case LONG: case INT: case DOUBLE: case FLOAT: - final Function converter; - if (fieldType instanceof NumberFieldType) { - converter = numericPointConverter(sortField, (NumberFieldType) fieldType); - } else if (fieldType instanceof DateFieldType) { - converter = datePointConverter((DateFieldType) fieldType, sortBuilder.getNumericType()); - } else { - return null; - } - if (PointValues.size(reader, fieldName) == 0) { - return null; - } - final Comparable min = converter.apply(PointValues.getMinPackedValue(reader, fieldName)); - final Comparable max = converter.apply(PointValues.getMaxPackedValue(reader, fieldName)); - return MinAndMax.newMinMax(min, max); - + return extractNumericMinAndMax(reader, sortField, fieldType, sortBuilder); case STRING: case STRING_VAL: if (fieldType instanceof KeywordFieldMapper.KeywordFieldType) { - Terms terms = MultiTerms.getTerms(reader, fieldName); + Terms terms = MultiTerms.getTerms(reader, fieldType.name()); if (terms == null) { return null; } - return terms.getMin() != null ? MinAndMax.newMinMax(terms.getMin(), terms.getMax()) : null; + return terms.getMin() != null ? new MinAndMax<>(terms.getMin(), terms.getMax()) : null; } break; } return null; } + private static MinAndMax extractNumericMinAndMax(IndexReader reader, + SortField sortField, + MappedFieldType fieldType, + FieldSortBuilder sortBuilder) throws IOException { + String fieldName = fieldType.name(); + if (PointValues.size(reader, fieldName) == 0) { + return null; + } + if (fieldType instanceof NumberFieldType) { + NumberFieldType numberFieldType = (NumberFieldType) fieldType; + Number minPoint = numberFieldType.parsePoint(PointValues.getMinPackedValue(reader, fieldName)); + Number maxPoint = numberFieldType.parsePoint(PointValues.getMaxPackedValue(reader, fieldName)); + switch (IndexSortConfig.getSortFieldType(sortField)) { + case LONG: + return new MinAndMax<>(minPoint.longValue(), maxPoint.longValue()); + case INT: + return new MinAndMax<>(minPoint.intValue(), maxPoint.intValue()); + case DOUBLE: + return new MinAndMax<>(minPoint.doubleValue(), maxPoint.doubleValue()); + case FLOAT: + return new MinAndMax<>(minPoint.floatValue(), maxPoint.floatValue()); + default: + return null; + } + } else if (fieldType instanceof DateFieldType) { + DateFieldType dateFieldType = (DateFieldType) fieldType; + Function dateConverter = createDateConverter(sortBuilder, dateFieldType); + Long min = dateConverter.apply(PointValues.getMinPackedValue(reader, fieldName)); + Long max = dateConverter.apply(PointValues.getMaxPackedValue(reader, fieldName)); + return new MinAndMax<>(min, max); + } + return null; + } + + private static Function createDateConverter(FieldSortBuilder sortBuilder, DateFieldType dateFieldType) { + String numericTypeStr = sortBuilder.getNumericType(); + if (numericTypeStr != null) { + NumericType numericType = resolveNumericType(numericTypeStr); + if (dateFieldType.resolution() == MILLISECONDS && numericType == NumericType.DATE_NANOSECONDS) { + return v -> DateUtils.toNanoSeconds(LongPoint.decodeDimension(v, 0)); + } else if (dateFieldType.resolution() == NANOSECONDS && numericType == NumericType.DATE) { + return v -> DateUtils.toMilliSeconds(LongPoint.decodeDimension(v, 0)); + } + } + return v -> LongPoint.decodeDimension(v, 0); + } + /** * Throws an exception if max children is not located at top level nested sort. */ @@ -647,7 +639,7 @@ public class FieldSortBuilder extends SortBuilder { private static final ObjectParser PARSER = new ObjectParser<>(NAME); static { - PARSER.declareField(FieldSortBuilder::missing, p -> p.objectText(), MISSING, ValueType.VALUE); + PARSER.declareField(FieldSortBuilder::missing, XContentParser::objectText, MISSING, ValueType.VALUE); PARSER.declareString((fieldSortBuilder, nestedPath) -> { deprecationLogger.deprecated("[nested_path] has been deprecated in favor of the [nested] parameter"); fieldSortBuilder.setNestedPath(nestedPath); @@ -660,7 +652,7 @@ public class FieldSortBuilder extends SortBuilder { return SortBuilder.parseNestedFilter(p); }, NESTED_FILTER_FIELD); PARSER.declareObject(FieldSortBuilder::setNestedSort, (p, c) -> NestedSortBuilder.fromXContent(p), NESTED_FIELD); - PARSER.declareString((b, v) -> b.setNumericType(v), NUMERIC_TYPE); + PARSER.declareString(FieldSortBuilder::setNumericType, NUMERIC_TYPE); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java b/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java index 28e07c8863b..ab02d6df799 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java +++ b/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java @@ -29,20 +29,21 @@ import java.util.Comparator; import java.util.Objects; /** - * A class that encapsulates a minimum and a maximum {@link Comparable}. + * A class that encapsulates a minimum and a maximum, that are of the same type and {@link Comparable}. */ public class MinAndMax> implements Writeable { private final T minValue; private final T maxValue; - private MinAndMax(T minValue, T maxValue) { + public MinAndMax(T minValue, T maxValue) { this.minValue = Objects.requireNonNull(minValue); this.maxValue = Objects.requireNonNull(maxValue); } + @SuppressWarnings("unchecked") public MinAndMax(StreamInput in) throws IOException { - this.minValue = (T) Lucene.readSortValue(in); - this.maxValue = (T) Lucene.readSortValue(in); + this.minValue = (T)Lucene.readSortValue(in); + this.maxValue = (T)Lucene.readSortValue(in); } @Override @@ -54,27 +55,23 @@ public class MinAndMax> implements Writeable { /** * Return the minimum value. */ - public T getMin() { + T getMin() { return minValue; } /** * Return the maximum value. */ - public T getMax() { + T getMax() { return maxValue; } - public static > MinAndMax newMinMax(T min, T max) { - return new MinAndMax<>(min, max); - } - /** * Return a {@link Comparator} for {@link MinAndMax} values according to the provided {@link SortOrder}. */ public static Comparator> getComparator(SortOrder order) { - Comparator cmp = order == SortOrder.ASC ? - Comparator.comparing(v -> (Comparable) v.getMin()) : Comparator.comparing(v -> (Comparable) v.getMax()); + Comparator> cmp = order == SortOrder.ASC ? + Comparator.comparing(MinAndMax::getMin) : Comparator.comparing(MinAndMax::getMax); if (order == SortOrder.DESC) { cmp = cmp.reversed(); } diff --git a/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java index 05231e1c5ab..844fbe8e865 100644 --- a/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java @@ -298,7 +298,7 @@ public class CanMatchPreFilterSearchPhaseTests extends ESTestCase { ActionListener listener) { Long min = rarely() ? null : randomLong(); Long max = min == null ? null : randomLongBetween(min, Long.MAX_VALUE); - MinAndMax minMax = min == null ? null : MinAndMax.newMinMax(min, max); + MinAndMax minMax = min == null ? null : new MinAndMax<>(min, max); boolean canMatch = frequently(); synchronized (shardIds) { shardIds.add(request.shardId());