huaxingao commented on a change in pull request #923:
URL: https://github.com/apache/parquet-mr/pull/923#discussion_r716278171
##########
File path:
parquet-column/src/main/java/org/apache/parquet/internal/column/columnindex/ColumnIndexBuilder.java
##########
@@ -326,12 +323,27 @@ boolean isNullPage(int pageIndex) {
// and >= the min value in the IN set, then the page might contain
// the values in the IN set.
getBoundaryOrder().ltEq(createValueComparator(max))
- .forEachRemaining((int index) -> matchingIndexes2.add(index));
+ .forEachRemaining((int index) ->
matchingIndexesLessThanMax.add(index));
getBoundaryOrder().gtEq(createValueComparator(min))
- .forEachRemaining((int index) -> matchingIndexes3.add(index));
- matchingIndexes2.retainAll(matchingIndexes3);
- matchingIndexes2.addAll(matchingIndexes1); // add the matching null
pages
- return IndexIterator.filter(getPageCount(), pageIndex ->
matchingIndexes2.contains(pageIndex));
+ .forEachRemaining((int index) ->
matchingIndexesLargerThanMin.add(index));
+ matchingIndexesLessThanMax.retainAll(matchingIndexesLargerThanMin);
+ IntSet matchingIndex = matchingIndexesLessThanMax;
+ matchingIndex.addAll(matchingIndexesForNull); // add the matching null
pages
+ return IndexIterator.filter(getPageCount(), pageIndex ->
matchingIndex.contains(pageIndex));
+ }
+
+ private <T extends Comparable<T>> T getMaxOrMin(boolean isMax, Set<T>
values) {
+ T res = null;
+ for (T element : values) {
+ if (res == null) {
+ res = element;
+ } else if (isMax && res != null && element != null &&
res.compareTo(element) < 0) {
Review comment:
I changed to `PrimitiveComparator`. I checked `instanceof` and then cast
to use the `PrimitiveComparator` for each of the type. Not sure if this is the
correct way. Please take a look.
##########
File path:
parquet-hadoop/src/main/java/org/apache/parquet/filter2/statisticslevel/StatisticsFilter.java
##########
@@ -186,26 +186,36 @@ private boolean hasNulls(ColumnChunkMetaData column) {
return BLOCK_MIGHT_MATCH;
}
+ if (stats.isNumNullsSet()) {
+ if (stats.getNumNulls() == 0) {
+ if (values.contains(null) && values.size() == 1) return
BLOCK_CANNOT_MATCH;
+ } else {
+ if (values.contains(null)) return BLOCK_MIGHT_MATCH;
+ }
+ }
+
// drop if all the element in value < min || all the element in value > max
- return allElementCanBeDropped(stats, values, meta);
+ if (stats.compareMinToValue(getMaxOrMin(true, values)) <= 0 &&
+ stats.compareMaxToValue(getMaxOrMin(false, values)) >= 0) {
+ return BLOCK_MIGHT_MATCH;
+ }
+ else {
+ return BLOCK_CANNOT_MATCH;
+ }
}
- private <T extends Comparable<T>> Boolean
allElementCanBeDropped(Statistics<T> stats, Set<T> values, ColumnChunkMetaData
meta) {
- for (T value : values) {
- if (value != null) {
- if (stats.compareMinToValue(value) <= 0 &&
stats.compareMaxToValue(value) >= 0)
- return false;
- } else {
- // numNulls is not set. We don't know anything about the nulls in this
chunk
- if (!stats.isNumNullsSet()) {
- return false;
- }
- if (hasNulls(meta)) {
- return false;
- }
+ private <T extends Comparable<T>> T getMaxOrMin(boolean isMax, Set<T>
values) {
Review comment:
Done. Thanks!
##########
File path:
parquet-column/src/main/java/org/apache/parquet/internal/column/columnindex/ColumnIndexBuilder.java
##########
@@ -291,32 +291,29 @@ boolean isNullPage(int pageIndex) {
@Override
public <T extends Comparable<T>> PrimitiveIterator.OfInt visit(In<T> in) {
Set<T> values = in.getValues();
- TreeSet<T> myTreeSet = new TreeSet<>();
- IntSet matchingIndexes1 = new IntOpenHashSet(); // for null
+ IntSet matchingIndexesForNull = new IntOpenHashSet(); // for null
Iterator<T> it = values.iterator();
while(it.hasNext()) {
T value = it.next();
- if (value != null) {
- myTreeSet.add(value);
- } else {
+ if (value == null) {
if (nullCounts == null) {
// Searching for nulls so if we don't have null related statistics
we have to return all pages
return IndexIterator.all(getPageCount());
} else {
for (int i = 0; i < nullCounts.length; i++) {
if (nullCounts[i] > 0) {
- matchingIndexes1.add(i);
+ matchingIndexesForNull.add(i);
}
}
}
}
}
- IntSet matchingIndexes2 = new IntOpenHashSet();
- IntSet matchingIndexes3 = new IntOpenHashSet();
+ IntSet matchingIndexesLessThanMax = new IntOpenHashSet();
+ IntSet matchingIndexesLargerThanMin = new IntOpenHashSet();
Review comment:
Changed.
##########
File path:
parquet-hadoop/src/main/java/org/apache/parquet/filter2/statisticslevel/StatisticsFilter.java
##########
@@ -186,26 +186,36 @@ private boolean hasNulls(ColumnChunkMetaData column) {
return BLOCK_MIGHT_MATCH;
}
+ if (stats.isNumNullsSet()) {
+ if (stats.getNumNulls() == 0) {
+ if (values.contains(null) && values.size() == 1) return
BLOCK_CANNOT_MATCH;
+ } else {
+ if (values.contains(null)) return BLOCK_MIGHT_MATCH;
+ }
+ }
+
// drop if all the element in value < min || all the element in value > max
- return allElementCanBeDropped(stats, values, meta);
+ if (stats.compareMinToValue(getMaxOrMin(true, values)) <= 0 &&
+ stats.compareMaxToValue(getMaxOrMin(false, values)) >= 0) {
+ return BLOCK_MIGHT_MATCH;
+ }
+ else {
Review comment:
Done.
##########
File path:
parquet-hadoop/src/test/java/org/apache/parquet/filter2/recordlevel/TestRecordLevelFilters.java
##########
@@ -146,6 +147,33 @@ public void testAllFilter() throws Exception {
assertEquals(new ArrayList<Group>(), found);
}
+ @Test
+ public void testInFilter() throws Exception {
+ BinaryColumn name = binaryColumn("name");
+
+ HashSet<Binary> nameSet = new HashSet<>();
+ nameSet.add(Binary.fromString("thing2"));
+ nameSet.add(Binary.fromString("thing1"));
+ for (int i = 100; i < 200; i++) {
+ nameSet.add(Binary.fromString("p" + i));
+ }
Review comment:
Added. Thanks!
--
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]
