You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@orc.apache.org by do...@apache.org on 2020/10/17 00:27:31 UTC
[orc] branch branch-1.5 updated: ORC-673: PPD LTE Point equality
comparison is wrong when RG MIN==MAX (#551)
This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-1.5
in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/branch-1.5 by this push:
new ae4d4aa ORC-673: PPD LTE Point equality comparison is wrong when RG MIN==MAX (#551)
ae4d4aa is described below
commit ae4d4aa18342e1163009c561c410c57e568295e8
Author: Panagiotis Garefalakis <pg...@cloudera.com>
AuthorDate: Sat Oct 17 01:27:19 2020 +0100
ORC-673: PPD LTE Point equality comparison is wrong when RG MIN==MAX (#551)
### What changes were proposed in this pull request?
FIx PPD Less than Equals comparison for min==max corner case
### Why are the changes needed?
Better PPD evaluation
### How was this patch tested?
Extra JUnit test
---
.../java/org/apache/orc/impl/RecordReaderImpl.java | 3 +-
.../orc/impl/TestPredicatePushDownBounds.java | 374 +++++++++++++++++++++
.../org/apache/orc/impl/TestRecordReaderImpl.java | 2 +-
3 files changed, 377 insertions(+), 2 deletions(-)
diff --git a/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java b/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
index db9f35b..d5a1307 100644
--- a/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
+++ b/java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java
@@ -640,7 +640,8 @@ public class RecordReaderImpl implements RecordReader {
}
case LESS_THAN_EQUALS:
loc = compareToRange((Comparable) predObj, minValue, maxValue);
- if (loc == Location.AFTER || loc == Location.MAX) {
+ if (loc == Location.AFTER || loc == Location.MAX ||
+ (loc == Location.MIN && minValue.equals(maxValue))) {
return hasNull ? TruthValue.YES_NULL : TruthValue.YES;
} else if (loc == Location.BEFORE) {
return hasNull ? TruthValue.NO_NULL : TruthValue.NO;
diff --git a/java/core/src/test/org/apache/orc/impl/TestPredicatePushDownBounds.java b/java/core/src/test/org/apache/orc/impl/TestPredicatePushDownBounds.java
new file mode 100644
index 0000000..56f4194
--- /dev/null
+++ b/java/core/src/test/org/apache/orc/impl/TestPredicatePushDownBounds.java
@@ -0,0 +1,374 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.orc.impl;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.hadoop.hive.ql.io.sarg.PredicateLeaf;
+import org.apache.hadoop.hive.ql.io.sarg.SearchArgument;
+import org.apache.hadoop.hive.ql.io.sarg.SearchArgumentFactory;
+import org.apache.orc.IntegerColumnStatistics;
+import org.apache.orc.TypeDescription;
+import org.apache.orc.util.BloomFilter;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+
+import static junit.framework.Assert.assertEquals;
+import static org.apache.orc.impl.TestRecordReaderImpl.createPredicateLeaf;
+
+public class TestPredicatePushDownBounds {
+
+ /**
+ * This test case handles the Equals corner case where the predicate is equal
+ * to truncated upper and lower bounds.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testCornerCases() {
+
+ int stringLength = 1100;
+ byte[] utf8F;
+ byte[] utf8P;
+
+ final TypeDescription schema = TypeDescription.createString();
+ final ColumnStatisticsImpl stat = ColumnStatisticsImpl.create(schema);
+
+ BloomFilter bf = new BloomFilter(100);
+ // FFF... to PPP...
+ for (int i = 70; i <= 80; i++) {
+ final String inputString = StringUtils
+ .repeat(Character.toString((char) i), stringLength);
+ bf.addString(inputString);
+ }
+
+ final String longStringF = StringUtils
+ .repeat(Character.toString('F'), stringLength);
+ final String longStringP = StringUtils
+ .repeat(Character.toString('P'), stringLength);
+
+ /* String that matches the upperbound value after truncation */
+ final String upperboundString =
+ StringUtils.repeat(Character.toString('P'), 1023) + "Q";
+ /* String that matches the lower value after truncation */
+ final String lowerboundString = StringUtils
+ .repeat(Character.toString('F'), 1024);
+
+ final String shortStringF = StringUtils.repeat(Character.toString('F'), 50);
+ final String shortStringP =
+ StringUtils.repeat(Character.toString('P'), 50) + "Q";
+
+ /* Test for a case EQUALS where only upperbound is set */
+ final PredicateLeaf predicateUpperBoundEquals = TestRecordReaderImpl
+ .createPredicateLeaf(PredicateLeaf.Operator.EQUALS,
+ PredicateLeaf.Type.STRING, "x", upperboundString, null);
+
+ /* Test for a case LESS_THAN where only upperbound is set */
+ final PredicateLeaf predicateUpperBoundLessThan = TestRecordReaderImpl
+ .createPredicateLeaf(PredicateLeaf.Operator.LESS_THAN,
+ PredicateLeaf.Type.STRING, "x", upperboundString, null);
+
+ /* Test for a case LESS_THAN_EQUALS where only upperbound is set */
+ final PredicateLeaf predicateUpperBoundLessThanEquals = TestRecordReaderImpl
+ .createPredicateLeaf(PredicateLeaf.Operator.LESS_THAN_EQUALS,
+ PredicateLeaf.Type.STRING, "x", upperboundString, null);
+
+ utf8F = shortStringF.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8F, 0, utf8F.length, 1);
+
+ utf8P = longStringP.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8P, 0, utf8P.length, 1);
+
+ assertEquals(SearchArgument.TruthValue.NO, RecordReaderImpl
+ .evaluatePredicate(stat, predicateUpperBoundEquals, null));
+
+ assertEquals(SearchArgument.TruthValue.YES, RecordReaderImpl
+ .evaluatePredicate(stat, predicateUpperBoundLessThan, null));
+
+ assertEquals(SearchArgument.TruthValue.YES, RecordReaderImpl
+ .evaluatePredicate(stat, predicateUpperBoundLessThanEquals, null));
+
+ stat.reset();
+
+ utf8F = longStringF.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8F, 0, utf8F.length, 1);
+
+ utf8P = shortStringP.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8P, 0, utf8P.length, 1);
+
+ /* Test for a case Equals where only lowerbound is set */
+ final PredicateLeaf predicateLowerBoundEquals = createPredicateLeaf(
+ PredicateLeaf.Operator.EQUALS, PredicateLeaf.Type.STRING, "x",
+ lowerboundString, null);
+
+ /* Test for a case LESS_THAN where only lowerbound is set */
+ final PredicateLeaf predicateLowerBoundLessThan = createPredicateLeaf(
+ PredicateLeaf.Operator.LESS_THAN, PredicateLeaf.Type.STRING, "x",
+ lowerboundString, null);
+
+ /* Test for a case LESS_THAN_EQUALS where only lowerbound is set */
+ final PredicateLeaf predicateLowerBoundLessThanEquals = createPredicateLeaf(
+ PredicateLeaf.Operator.LESS_THAN_EQUALS, PredicateLeaf.Type.STRING, "x",
+ lowerboundString, null);
+
+ assertEquals(SearchArgument.TruthValue.NO, RecordReaderImpl
+ .evaluatePredicate(stat, predicateLowerBoundEquals, null));
+
+ assertEquals(SearchArgument.TruthValue.NO, RecordReaderImpl
+ .evaluatePredicate(stat, predicateLowerBoundLessThan, bf));
+
+ assertEquals(SearchArgument.TruthValue.NO, RecordReaderImpl
+ .evaluatePredicate(stat, predicateLowerBoundLessThanEquals, null));
+
+ }
+
+ /**
+ * A case where the search values fall within the upperbound and lower bound
+ * range.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testNormalCase() throws Exception {
+
+ int stringLength = 1100;
+ /* length of string in BF */
+ int bfStringLength = 50;
+ //int stringLength = 11;
+ byte[] utf8F;
+ byte[] utf8P;
+
+ final TypeDescription schema = TypeDescription.createString();
+ final ColumnStatisticsImpl stat = ColumnStatisticsImpl.create(schema);
+
+ BloomFilter bf = new BloomFilter(100);
+ // FFF... to PPP...
+ for (int i = 70; i <= 80; i++) {
+ final String inputString = StringUtils
+ .repeat(Character.toString((char) i), bfStringLength);
+ bf.addString(inputString);
+ }
+
+ final String longStringF = StringUtils
+ .repeat(Character.toString('F'), stringLength);
+ final String longStringP = StringUtils
+ .repeat(Character.toString('P'), stringLength);
+ final String predicateString = StringUtils
+ .repeat(Character.toString('I'), 50);
+
+
+ /* Test for a case where only upperbound is set */
+ final PredicateLeaf predicateEquals = createPredicateLeaf(
+ PredicateLeaf.Operator.EQUALS, PredicateLeaf.Type.STRING, "x",
+ predicateString, null);
+
+ /* trigger lower bound */
+ utf8F = longStringF.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8F, 0, utf8F.length, 1);
+
+ /* trigger upper bound */
+ utf8P = longStringP.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8P, 0, utf8P.length, 1);
+
+ assertEquals(SearchArgument.TruthValue.YES_NO,
+ RecordReaderImpl.evaluatePredicate(stat, predicateEquals, bf));
+
+ }
+
+ /**
+ * Test for IN search arg when upper and lower bounds are set.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testIN() throws Exception {
+ int stringLength = 1100;
+ byte[] utf8F;
+ byte[] utf8P;
+
+ final TypeDescription schema = TypeDescription.createString();
+ final ColumnStatisticsImpl stat = ColumnStatisticsImpl.create(schema);
+
+ final BloomFilter bf = new BloomFilter(100);
+ // FFF... to PPP...
+ for (int i = 70; i <= 80; i++) {
+ final String inputString = StringUtils
+ .repeat(Character.toString((char) i), stringLength);
+ bf.addString(inputString);
+ }
+
+ final String longStringF = StringUtils
+ .repeat(Character.toString('F'), stringLength);
+ final String longStringP = StringUtils
+ .repeat(Character.toString('P'), stringLength);
+
+ /* String that matches the upperbound value after truncation */
+ final String upperboundString =
+ StringUtils.repeat(Character.toString('P'), 1023) + "Q";
+ /* String that matches the lower value after truncation */
+ final String lowerboundString = StringUtils
+ .repeat(Character.toString('F'), 1024);
+
+ final String shortStringF = StringUtils.repeat(Character.toString('F'), 50);
+ final String shortStringP =
+ StringUtils.repeat(Character.toString('P'), 50) + "Q";
+
+ final List<Object> args = new ArrayList<Object>();
+ args.add(upperboundString);
+
+ /* set upper bound */
+ utf8F = shortStringF.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8F, 0, utf8F.length, 1);
+
+ utf8P = longStringP.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8P, 0, utf8P.length, 1);
+
+ /* Test for a case IN where only upper bound is set and test literal is equal to upperbound */
+ final PredicateLeaf predicateUpperBoundSet = TestRecordReaderImpl
+ .createPredicateLeaf(PredicateLeaf.Operator.IN,
+ PredicateLeaf.Type.STRING, "x", null, args);
+
+ assertEquals(SearchArgument.TruthValue.NO,
+ RecordReaderImpl.evaluatePredicate(stat, predicateUpperBoundSet, null));
+
+ /* Test for lower bound set only */
+ args.clear();
+ args.add(lowerboundString);
+
+ stat.reset();
+ /* set lower bound */
+ utf8F = longStringF.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8F, 0, utf8F.length, 1);
+
+ utf8P = shortStringP.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8P, 0, utf8P.length, 1);
+
+ /* Test for a case IN where only lower bound is set and the test literal is lowerbound string */
+ final PredicateLeaf predicateLowerBoundSet = TestRecordReaderImpl
+ .createPredicateLeaf(PredicateLeaf.Operator.IN,
+ PredicateLeaf.Type.STRING, "x", null, args);
+
+ assertEquals(SearchArgument.TruthValue.NO,
+ RecordReaderImpl.evaluatePredicate(stat, predicateLowerBoundSet, null));
+
+ /* Test the case were both upper and lower bounds are set */
+ args.clear();
+ args.add(lowerboundString);
+ args.add(upperboundString);
+
+ stat.reset();
+ /* set upper and lower bound */
+ utf8F = longStringF.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8F, 0, utf8F.length, 1);
+
+ utf8P = longStringP.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8P, 0, utf8P.length, 1);
+
+ final PredicateLeaf predicateUpperLowerBoundSet = TestRecordReaderImpl
+ .createPredicateLeaf(PredicateLeaf.Operator.IN,
+ PredicateLeaf.Type.STRING, "x", null, args);
+
+ assertEquals(SearchArgument.TruthValue.NO, RecordReaderImpl
+ .evaluatePredicate(stat, predicateUpperLowerBoundSet, null));
+
+ /* test the boundary condition */
+ args.clear();
+ args.add(longStringF);
+ args.add(longStringP);
+
+ stat.reset();
+ /* set upper and lower bound */
+ utf8F = longStringF.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8F, 0, utf8F.length, 1);
+
+ utf8P = longStringP.getBytes(StandardCharsets.UTF_8);
+ stat.increment();
+ stat.updateString(utf8P, 0, utf8P.length, 1);
+
+ final PredicateLeaf predicateUpperLowerBoundSetBoundary = TestRecordReaderImpl
+ .createPredicateLeaf(PredicateLeaf.Operator.IN,
+ PredicateLeaf.Type.STRING, "x", null, args);
+
+ assertEquals(SearchArgument.TruthValue.YES_NO, RecordReaderImpl
+ .evaluatePredicate(stat, predicateUpperLowerBoundSetBoundary, null));
+
+ }
+
+ /**
+ * Test for LESS_THAN_EQUALS search arg when upper and lower bounds are the same.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void testLessThanEquals() {
+ final TypeDescription schema = TypeDescription.createInt();
+ final ColumnStatisticsImpl stat = ColumnStatisticsImpl.create(schema);
+
+ stat.increment();
+ stat.updateInteger(1, 100);
+ stat.updateInteger(3, 100);
+
+ IntegerColumnStatistics typed = (IntegerColumnStatistics) stat;
+ Assert.assertEquals(1, typed.getMinimum());
+ Assert.assertEquals(3, typed.getMaximum());
+
+ SearchArgument sArg = SearchArgumentFactory.newBuilder()
+ .startAnd()
+ .lessThanEquals("c", PredicateLeaf.Type.LONG, 3L)
+ .end()
+ .build();
+
+ assertEquals(SearchArgument.TruthValue.YES, RecordReaderImpl
+ .evaluatePredicate(stat, sArg.getLeaves().get(0), null));
+
+ // Corner case where MIN == MAX == 3
+ final ColumnStatisticsImpl newStat = ColumnStatisticsImpl.create(schema);
+
+ newStat.increment();
+ newStat.updateInteger(3, 100);
+
+ typed = (IntegerColumnStatistics) newStat;
+ Assert.assertEquals(3, typed.getMinimum());
+ Assert.assertEquals(3, typed.getMaximum());
+
+ sArg = SearchArgumentFactory.newBuilder()
+ .startAnd()
+ .lessThanEquals("c", PredicateLeaf.Type.LONG, 3L)
+ .end()
+ .build();
+
+ assertEquals(SearchArgument.TruthValue.YES, RecordReaderImpl
+ .evaluatePredicate(newStat, sArg.getLeaves().get(0), null));
+ }
+}
diff --git a/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java b/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java
index e512304..77fb575 100644
--- a/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java
+++ b/java/core/src/test/org/apache/orc/impl/TestRecordReaderImpl.java
@@ -1020,7 +1020,7 @@ public class TestRecordReaderImpl {
evaluateInteger(createStringStats("c", "d", true), pred)); // min
assertEquals(TruthValue.YES_NO_NULL,
evaluateInteger(createStringStats("b", "d", true), pred)); // middle
- assertEquals(TruthValue.YES_NO_NULL,
+ assertEquals(TruthValue.YES_NULL,
evaluateInteger(createStringStats("c", "c", true), pred)); // same
}