gszadovszky commented on a change in pull request #923:
URL: https://github.com/apache/parquet-mr/pull/923#discussion_r717354327
##########
File path: parquet-column/src/main/java/org/apache/parquet/column/MinMax.java
##########
@@ -18,25 +18,17 @@
*/
package org.apache.parquet.column;
-import java.util.Iterator;
-import java.util.Set;
-
-import org.apache.parquet.io.api.Binary;
import org.apache.parquet.schema.PrimitiveComparator;
/**
- * This class calculates the max and min values of a Set.
+ * This class calculates the max and min values of an iterable collection.
*/
public final class MinMax<T> {
- private PrimitiveComparator comparator;
- private Iterator<T> iterator;
private T min = null;
private T max = null;
- public MinMax(PrimitiveComparator comparator, Iterator<T> iterator) {
- this.comparator = comparator;
- this.iterator = iterator;
- getMinAndMax();
+ public MinMax(PrimitiveComparator comparator, Iterable<T> iterable) {
Review comment:
I think, you should get warnings if you use a generic class without the
generic type. `PrimitiveComparator<T>` or even `Comparator<T>` should work fine.
##########
File path: parquet-column/src/main/java/org/apache/parquet/column/MinMax.java
##########
@@ -47,43 +39,22 @@ public T getMax() {
return max;
}
- private void getMinAndMax() {
- while(iterator.hasNext()) {
- T element = iterator.next();
+ private void getMinAndMax(PrimitiveComparator comparator, Iterable<T>
iterable) {
+ iterable.forEach(element -> {
if (max == null) {
max = element;
- } else if (max != null && element != null) {
- if ((element instanceof Integer &&
- ((PrimitiveComparator<Integer>)comparator).compare((Integer)max,
(Integer)element) < 0) ||
- (element instanceof Binary &&
- ((PrimitiveComparator<Binary>)comparator).compare((Binary)max,
(Binary)element) < 0) ||
- (element instanceof Double &&
- ((PrimitiveComparator<Double>)comparator).compare((Double)max,
(Double)element) < 0) ||
- (element instanceof Float &&
- ((PrimitiveComparator<Float>)comparator).compare((Float)max,
(Float)element) < 0) ||
- (element instanceof Boolean &&
- ((PrimitiveComparator<Boolean>)comparator).compare((Boolean)max,
(Boolean)element) < 0) ||
- (element instanceof Long &&
- ((PrimitiveComparator<Long>)comparator).compare((Long) max,
(Long)element) < 0))
+ } else if (element != null) {
+ if (comparator.compare(max, element) < 0) {
Review comment:
nit: You may combine the two with an `&&`.
##########
File path: parquet-column/src/main/java/org/apache/parquet/column/MinMax.java
##########
@@ -47,43 +39,22 @@ public T getMax() {
return max;
}
- private void getMinAndMax() {
- while(iterator.hasNext()) {
- T element = iterator.next();
+ private void getMinAndMax(PrimitiveComparator comparator, Iterable<T>
iterable) {
+ iterable.forEach(element -> {
if (max == null) {
max = element;
- } else if (max != null && element != null) {
- if ((element instanceof Integer &&
- ((PrimitiveComparator<Integer>)comparator).compare((Integer)max,
(Integer)element) < 0) ||
- (element instanceof Binary &&
- ((PrimitiveComparator<Binary>)comparator).compare((Binary)max,
(Binary)element) < 0) ||
- (element instanceof Double &&
- ((PrimitiveComparator<Double>)comparator).compare((Double)max,
(Double)element) < 0) ||
- (element instanceof Float &&
- ((PrimitiveComparator<Float>)comparator).compare((Float)max,
(Float)element) < 0) ||
- (element instanceof Boolean &&
- ((PrimitiveComparator<Boolean>)comparator).compare((Boolean)max,
(Boolean)element) < 0) ||
- (element instanceof Long &&
- ((PrimitiveComparator<Long>)comparator).compare((Long) max,
(Long)element) < 0))
+ } else if (element != null) {
+ if (comparator.compare(max, element) < 0) {
max = element;
+ }
}
if (min == null) {
min = element;
- } else if (min != null && element != null) {
- if ((element instanceof Integer &&
- ((PrimitiveComparator<Integer>)comparator).compare((Integer)min,
(Integer)element) > 0) ||
- (element instanceof Binary &&
- ((PrimitiveComparator<Binary>)comparator).compare((Binary)min,
(Binary)element) > 0) ||
- (element instanceof Double &&
- ((PrimitiveComparator<Double>)comparator).compare((Double)min,
(Double)element) > 0) ||
- (element instanceof Float &&
- ((PrimitiveComparator<Float>)comparator).compare((Float)min,
(Float)element) > 0) ||
- (element instanceof Boolean &&
- ((PrimitiveComparator<Boolean>)comparator).compare((Boolean)min,
(Boolean)element) > 0) ||
- (element instanceof Long &&
- ((PrimitiveComparator<Long>)comparator).compare((Long)min,
(Long)element) > 0))
+ } else if (element != null) {
+ if (comparator.compare(min, element) > 0) {
Review comment:
See above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
