From 409f76059279a0e952bb743ced3472ef9da3c6ac Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Fri, 21 Feb 2020 15:21:52 +0100 Subject: [PATCH 1/4] Address MinAndMax generics warnings The 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 | 22 ++++++++++++------- .../elasticsearch/search/sort/MinAndMax.java | 20 +++++++++-------- 3 files changed, 26 insertions(+), 18 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 aba32d2c850a0..cc9282ff114e3 100644 --- a/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java @@ -122,7 +122,7 @@ private static List sortShards(GroupShardsIterator shardsIts.get(ord)) + .map(shardsIts::get) .collect(Collectors.toList()); } 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 a2ffa632f5ca8..d4ff2b66259d6 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -434,7 +434,7 @@ public static FieldSortBuilder getPrimaryFieldSortOrNull(SearchSourceBuilder sou * {@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) { + private static Function> numericPointConverter(SortField sortField, NumberFieldType numberFieldType) { switch (IndexSortConfig.getSortFieldType(sortField)) { case LONG: return v -> numberFieldType.parsePoint(v).longValue(); @@ -457,7 +457,7 @@ private static Function numericPointConverter(SortField sort * 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) { + private static Function> datePointConverter(DateFieldType dateFieldType, String numericTypeStr) { if (numericTypeStr != null) { NumericType numericType = resolveNumericType(numericTypeStr); if (dateFieldType.resolution() == MILLISECONDS && numericType == NumericType.DATE_NANOSECONDS) { @@ -491,7 +491,7 @@ public static MinAndMax getMinMaxOrNull(QueryShardContext context, FieldSortB case INT: case DOUBLE: case FLOAT: - final Function converter; + final Function> converter; if (fieldType instanceof NumberFieldType) { converter = numericPointConverter(sortField, (NumberFieldType) fieldType); } else if (fieldType instanceof DateFieldType) { @@ -502,9 +502,7 @@ public static MinAndMax getMinMaxOrNull(QueryShardContext context, FieldSortB 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 extractMinAndMax(reader, fieldName, converter); case STRING: case STRING_VAL: @@ -520,6 +518,14 @@ public static MinAndMax getMinMaxOrNull(QueryShardContext context, FieldSortB return null; } + @SuppressWarnings("unchecked") + private static > MinAndMax extractMinAndMax(IndexReader reader, String fieldName, + Function> converter) throws IOException { + final T min = (T)converter.apply(PointValues.getMinPackedValue(reader, fieldName)); + final T max = (T)converter.apply(PointValues.getMaxPackedValue(reader, fieldName)); + return MinAndMax.newMinMax(min, max); + } + /** * Throws an exception if max children is not located at top level nested sort. */ @@ -601,12 +607,12 @@ public static FieldSortBuilder fromXContent(XContentParser parser, String fieldN 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::unmappedType , UNMAPPED_TYPE); PARSER.declareString((b, v) -> b.order(SortOrder.fromString(v)) , ORDER_FIELD); PARSER.declareString((b, v) -> b.sortMode(SortMode.fromString(v)), SORT_MODE); 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 28e07c8863b8a..942f572cb3727 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java +++ b/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java @@ -29,9 +29,9 @@ 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 { +public class MinAndMax> implements Writeable { private final T minValue; private final T maxValue; @@ -40,9 +40,10 @@ private MinAndMax(T minValue, T maxValue) { 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,18 +55,18 @@ public void writeTo(StreamOutput out) throws IOException { /** * 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) { + public static > MinAndMax newMinMax(T min, T max) { return new MinAndMax<>(min, max); } @@ -73,8 +74,9 @@ public static > MinAndMax newMinMax(T min, T * 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(); } From 38c03edbc319482799c5375aab6167fa0681f351 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 25 Feb 2020 12:17:56 +0100 Subject: [PATCH 2/4] iter --- .../search/sort/FieldSortBuilder.java | 110 ++++++++---------- .../elasticsearch/search/sort/MinAndMax.java | 7 +- 2 files changed, 49 insertions(+), 68 deletions(-) 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 d4ff2b66259d6..efd359538ac88 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java @@ -429,46 +429,6 @@ public static FieldSortBuilder getPrimaryFieldSortOrNull(SearchSourceBuilder sou 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 @@ -485,45 +445,71 @@ public static MinAndMax getMinMaxOrNull(QueryShardContext context, FieldSortB 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; - } - return extractMinAndMax(reader, fieldName, converter); - + 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; } - @SuppressWarnings("unchecked") - private static > MinAndMax extractMinAndMax(IndexReader reader, String fieldName, - Function> converter) throws IOException { - final T min = (T)converter.apply(PointValues.getMinPackedValue(reader, fieldName)); - final T max = (T)converter.apply(PointValues.getMaxPackedValue(reader, fieldName)); - return MinAndMax.newMinMax(min, max); + 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); } /** 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 942f572cb3727..19986f171f5fa 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java +++ b/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java @@ -35,7 +35,7 @@ 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); } @@ -66,17 +66,12 @@ 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(MinAndMax::getMin) : Comparator.comparing(MinAndMax::getMax); - if (order == SortOrder.DESC) { cmp = cmp.reversed(); } From 7ab696bc5ea8842148b395d3f60f5f25b5b3031a Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 25 Feb 2020 17:41:32 +0100 Subject: [PATCH 3/4] fix compile --- .../action/search/CanMatchPreFilterSearchPhase.java | 2 +- .../action/search/CanMatchPreFilterSearchPhaseTests.java | 2 +- 2 files changed, 2 insertions(+), 2 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 cc9282ff114e3..03851a7217a6d 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 @@ private static boolean shouldSortShards(MinAndMax[] minAndMaxes) { private static Comparator 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/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java b/server/src/test/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhaseTests.java index 06e63f7b32936..c38f77207daf8 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 void sendCanMatch(Transport.Connection connection, ShardSearchRequest req 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()); From 4a3123cdbf39ba31e133cc56746415a6c402ee45 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Fri, 28 Feb 2020 13:46:20 +0100 Subject: [PATCH 4/4] iter --- .../src/main/java/org/elasticsearch/search/sort/MinAndMax.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 19986f171f5fa..ab02d6df7993b 100644 --- a/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java +++ b/server/src/main/java/org/elasticsearch/search/sort/MinAndMax.java @@ -31,7 +31,7 @@ /** * A class that encapsulates a minimum and a maximum, that are of the same type and {@link Comparable}. */ -public class MinAndMax> implements Writeable { +public class MinAndMax> implements Writeable { private final T minValue; private final T maxValue;