This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new da7cbcf Fix the wrong unbounded values in
RangePredicateEvaluatorFactory (#5586)
da7cbcf is described below
commit da7cbcfd84b12331c18ef90a8b73995cfdebe338
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Wed Jun 17 22:37:12 2020 -0700
Fix the wrong unbounded values in RangePredicateEvaluatorFactory (#5586)
The unbounded values for LONG, FLOAT, DOUBLE type are not set based on the
type, but always as Integer.MIN_VALUE and Integer.MAX_VALUE
This can cause wrong result if the passed in value is smaller than
Integer.MIN_VALUE or larger than Integer.MAX_VALUE
Minor improvements:
- Replace "*" with constant `RangePredicate.UNBOUNDED` for readability
- For string type, use null to represent unbounded to save the per-value
string comparison
---
.../predicate/PredicateEvaluatorProvider.java | 4 +-
.../predicate/RangePredicateEvaluatorFactory.java | 66 +++++++++++-----------
.../NoDictionaryRangePredicateEvaluatorTest.java | 17 +++---
3 files changed, 46 insertions(+), 41 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/PredicateEvaluatorProvider.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/PredicateEvaluatorProvider.java
index 119687d..00f3d09 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/PredicateEvaluatorProvider.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/PredicateEvaluatorProvider.java
@@ -77,8 +77,8 @@ public class PredicateEvaluatorProvider {
throw new UnsupportedOperationException("Unsupported predicate
type: " + predicate.getType());
}
}
- } catch (NumberFormatException e) {
- // This NumberFormatException is caused by passing in a non-numeric
string as numeric number in query
+ } catch (Exception e) {
+ // Exception here is caused by mismatch between the column data type and
the predicate value in the query
throw new BadQueryRequestException(e);
}
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/RangePredicateEvaluatorFactory.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/RangePredicateEvaluatorFactory.java
index fee24ae..751e977 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/RangePredicateEvaluatorFactory.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/predicate/RangePredicateEvaluatorFactory.java
@@ -94,7 +94,7 @@ public class RangePredicateEvaluatorFactory {
boolean includeLowerBoundary = rangePredicate.includeLowerBoundary();
boolean includeUpperBoundary = rangePredicate.includeUpperBoundary();
- if (lowerBoundary.equals("*")) {
+ if (lowerBoundary.equals(RangePredicate.UNBOUNDED)) {
_startDictId = 0;
} else {
int insertionIndex = dictionary.insertionIndexOf(lowerBoundary);
@@ -108,7 +108,7 @@ public class RangePredicateEvaluatorFactory {
}
}
}
- if (upperBoundary.equals("*")) {
+ if (upperBoundary.equals(RangePredicate.UNBOUNDED)) {
_endDictId = dictionary.length();
} else {
int insertionIndex = dictionary.insertionIndexOf(upperBoundary);
@@ -269,12 +269,14 @@ public class RangePredicateEvaluatorFactory {
final boolean _includeUpperBoundary;
IntRawValueBasedRangePredicateEvaluator(RangePredicate rangePredicate) {
- _includeLowerBoundary = rangePredicate.includeLowerBoundary();
- _includeUpperBoundary = rangePredicate.includeUpperBoundary();
String lowerBoundary = rangePredicate.getLowerBoundary();
String upperBoundary = rangePredicate.getUpperBoundary();
- _lowerBoundary = lowerBoundary.equals("*") ? Integer.MIN_VALUE :
Integer.parseInt(lowerBoundary);
- _upperBoundary = upperBoundary.equals("*") ? Integer.MAX_VALUE :
Integer.parseInt(upperBoundary);
+ _lowerBoundary =
+ lowerBoundary.equals(RangePredicate.UNBOUNDED) ? Integer.MIN_VALUE :
Integer.parseInt(lowerBoundary);
+ _upperBoundary =
+ upperBoundary.equals(RangePredicate.UNBOUNDED) ? Integer.MAX_VALUE :
Integer.parseInt(upperBoundary);
+ _includeLowerBoundary = rangePredicate.includeLowerBoundary();
+ _includeUpperBoundary = rangePredicate.includeUpperBoundary();
}
@Override
@@ -311,12 +313,12 @@ public class RangePredicateEvaluatorFactory {
final boolean _includeUpperBoundary;
LongRawValueBasedRangePredicateEvaluator(RangePredicate rangePredicate) {
- _includeLowerBoundary = rangePredicate.includeLowerBoundary();
- _includeUpperBoundary = rangePredicate.includeUpperBoundary();
String lowerBoundary = rangePredicate.getLowerBoundary();
String upperBoundary = rangePredicate.getUpperBoundary();
- _lowerBoundary = lowerBoundary.equals("*") ? Integer.MIN_VALUE :
Long.parseLong(lowerBoundary);
- _upperBoundary = upperBoundary.equals("*") ? Integer.MAX_VALUE :
Long.parseLong(upperBoundary);
+ _lowerBoundary = lowerBoundary.equals(RangePredicate.UNBOUNDED) ?
Long.MIN_VALUE : Long.parseLong(lowerBoundary);
+ _upperBoundary = upperBoundary.equals(RangePredicate.UNBOUNDED) ?
Long.MAX_VALUE : Long.parseLong(upperBoundary);
+ _includeLowerBoundary = rangePredicate.includeLowerBoundary();
+ _includeUpperBoundary = rangePredicate.includeUpperBoundary();
}
@Override
@@ -353,12 +355,14 @@ public class RangePredicateEvaluatorFactory {
final boolean _includeUpperBoundary;
FloatRawValueBasedRangePredicateEvaluator(RangePredicate rangePredicate) {
- _includeLowerBoundary = rangePredicate.includeLowerBoundary();
- _includeUpperBoundary = rangePredicate.includeUpperBoundary();
String lowerBoundary = rangePredicate.getLowerBoundary();
String upperBoundary = rangePredicate.getUpperBoundary();
- _lowerBoundary = lowerBoundary.equals("*") ? Integer.MIN_VALUE :
Float.parseFloat(lowerBoundary);
- _upperBoundary = upperBoundary.equals("*") ? Integer.MAX_VALUE :
Float.parseFloat(upperBoundary);
+ _lowerBoundary =
+ lowerBoundary.equals(RangePredicate.UNBOUNDED) ?
Float.NEGATIVE_INFINITY : Float.parseFloat(lowerBoundary);
+ _upperBoundary =
+ upperBoundary.equals(RangePredicate.UNBOUNDED) ?
Float.POSITIVE_INFINITY : Float.parseFloat(upperBoundary);
+ _includeLowerBoundary = rangePredicate.includeLowerBoundary();
+ _includeUpperBoundary = rangePredicate.includeUpperBoundary();
}
@Override
@@ -395,12 +399,14 @@ public class RangePredicateEvaluatorFactory {
final boolean _includeUpperBoundary;
DoubleRawValueBasedRangePredicateEvaluator(RangePredicate rangePredicate) {
- _includeLowerBoundary = rangePredicate.includeLowerBoundary();
- _includeUpperBoundary = rangePredicate.includeUpperBoundary();
String lowerBoundary = rangePredicate.getLowerBoundary();
String upperBoundary = rangePredicate.getUpperBoundary();
- _lowerBoundary = lowerBoundary.equals("*") ? Integer.MIN_VALUE :
Double.parseDouble(lowerBoundary);
- _upperBoundary = upperBoundary.equals("*") ? Integer.MAX_VALUE :
Double.parseDouble(upperBoundary);
+ _lowerBoundary =
+ lowerBoundary.equals(RangePredicate.UNBOUNDED) ?
Double.NEGATIVE_INFINITY : Double.parseDouble(lowerBoundary);
+ _upperBoundary =
+ upperBoundary.equals(RangePredicate.UNBOUNDED) ?
Double.POSITIVE_INFINITY : Double.parseDouble(upperBoundary);
+ _includeLowerBoundary = rangePredicate.includeLowerBoundary();
+ _includeUpperBoundary = rangePredicate.includeUpperBoundary();
}
@Override
@@ -437,8 +443,10 @@ public class RangePredicateEvaluatorFactory {
final boolean _includeUpperBoundary;
StringRawValueBasedRangePredicateEvaluator(RangePredicate rangePredicate) {
- _lowerBoundary = rangePredicate.getLowerBoundary();
- _upperBoundary = rangePredicate.getUpperBoundary();
+ String lowerBoundary = rangePredicate.getLowerBoundary();
+ String upperBoundary = rangePredicate.getUpperBoundary();
+ _lowerBoundary = lowerBoundary.equals(RangePredicate.UNBOUNDED) ? null :
lowerBoundary;
+ _upperBoundary = upperBoundary.equals(RangePredicate.UNBOUNDED) ? null :
upperBoundary;
_includeLowerBoundary = rangePredicate.includeLowerBoundary();
_includeUpperBoundary = rangePredicate.includeUpperBoundary();
}
@@ -456,14 +464,14 @@ public class RangePredicateEvaluatorFactory {
@Override
public boolean applySV(String value) {
boolean result = true;
- if (!_lowerBoundary.equals("*")) {
+ if (_lowerBoundary != null) {
if (_includeLowerBoundary) {
result = _lowerBoundary.compareTo(value) <= 0;
} else {
result = _lowerBoundary.compareTo(value) < 0;
}
}
- if (!_upperBoundary.equals("*")) {
+ if (_upperBoundary != null) {
if (_includeUpperBoundary) {
result &= _upperBoundary.compareTo(value) >= 0;
} else {
@@ -481,16 +489,10 @@ public class RangePredicateEvaluatorFactory {
final boolean _includeUpperBoundary;
BytesRawValueBasedRangePredicateEvaluator(RangePredicate rangePredicate) {
- if (!"*".equals(rangePredicate.getLowerBoundary())) {
- _lowerBoundary = BytesUtils.toBytes(rangePredicate.getLowerBoundary());
- } else {
- _lowerBoundary = null;
- }
- if (!"*".equals(rangePredicate.getUpperBoundary())) {
- _upperBoundary = BytesUtils.toBytes(rangePredicate.getUpperBoundary());
- } else {
- _upperBoundary = null;
- }
+ String lowerBoundary = rangePredicate.getLowerBoundary();
+ String upperBoundary = rangePredicate.getUpperBoundary();
+ _lowerBoundary = lowerBoundary.equals(RangePredicate.UNBOUNDED) ? null :
BytesUtils.toBytes(lowerBoundary);
+ _upperBoundary = upperBoundary.equals(RangePredicate.UNBOUNDED) ? null :
BytesUtils.toBytes(upperBoundary);
_includeLowerBoundary = rangePredicate.includeLowerBoundary();
_includeUpperBoundary = rangePredicate.includeUpperBoundary();
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/predicate/NoDictionaryRangePredicateEvaluatorTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/predicate/NoDictionaryRangePredicateEvaluatorTest.java
index 1c60083..e325ab6 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/predicate/NoDictionaryRangePredicateEvaluatorTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/predicate/NoDictionaryRangePredicateEvaluatorTest.java
@@ -19,11 +19,11 @@
package org.apache.pinot.core.predicate;
import java.util.Collections;
-import org.apache.pinot.spi.data.FieldSpec;
-import org.apache.pinot.spi.utils.ByteArray;
import org.apache.pinot.core.common.predicate.RangePredicate;
import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
import
org.apache.pinot.core.operator.filter.predicate.RangePredicateEvaluatorFactory;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.utils.ByteArray;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -75,11 +75,12 @@ public class NoDictionaryRangePredicateEvaluatorTest {
for (int i = -20; i < 20; i++) {
Assert.assertTrue(predicateEvaluator.applySV(i));
}
+ Assert.assertTrue(predicateEvaluator.applySV(Integer.MIN_VALUE));
+ Assert.assertTrue(predicateEvaluator.applySV(Integer.MAX_VALUE));
}
@Test
public void testLongPredicateEvaluator() {
-
PredicateEvaluator predicateEvaluator = buildRangePredicate("[-10\t\t10]",
FieldSpec.DataType.LONG);
for (int i = -20; i < 20; i++) {
Assert.assertEquals(predicateEvaluator.applySV((long) i), (i >= -10 && i
<= 10));
@@ -119,11 +120,12 @@ public class NoDictionaryRangePredicateEvaluatorTest {
for (int i = -20; i < 20; i++) {
Assert.assertTrue(predicateEvaluator.applySV((long) i));
}
+ Assert.assertTrue(predicateEvaluator.applySV(Long.MIN_VALUE));
+ Assert.assertTrue(predicateEvaluator.applySV(Long.MAX_VALUE));
}
@Test
public void testFloatPredicateEvaluator() {
-
PredicateEvaluator predicateEvaluator = buildRangePredicate("[-10\t\t10]",
FieldSpec.DataType.FLOAT);
for (int i = -20; i < 20; i++) {
Assert.assertEquals(predicateEvaluator.applySV((float) i), (i >= -10 &&
i <= 10));
@@ -163,11 +165,12 @@ public class NoDictionaryRangePredicateEvaluatorTest {
for (int i = -20; i < 20; i++) {
Assert.assertTrue(predicateEvaluator.applySV((float) i));
}
+ Assert.assertTrue(predicateEvaluator.applySV(Float.NEGATIVE_INFINITY));
+ Assert.assertTrue(predicateEvaluator.applySV(Float.POSITIVE_INFINITY));
}
@Test
public void testDoublePredicateEvaluator() {
-
PredicateEvaluator predicateEvaluator = buildRangePredicate("[-10\t\t10]",
FieldSpec.DataType.DOUBLE);
for (int i = -20; i < 20; i++) {
Assert.assertEquals(predicateEvaluator.applySV((double) i), (i >= -10 &&
i <= 10));
@@ -207,11 +210,12 @@ public class NoDictionaryRangePredicateEvaluatorTest {
for (int i = -20; i < 20; i++) {
Assert.assertTrue(predicateEvaluator.applySV((double) i));
}
+ Assert.assertTrue(predicateEvaluator.applySV(Double.NEGATIVE_INFINITY));
+ Assert.assertTrue(predicateEvaluator.applySV(Double.POSITIVE_INFINITY));
}
@Test
public void testStringPredicateEvaluator() {
-
PredicateEvaluator predicateEvaluator = buildRangePredicate("[-10\t\t10]",
FieldSpec.DataType.STRING);
for (int i = -20; i < 20; i++) {
String value = Integer.toString(i);
@@ -264,7 +268,6 @@ public class NoDictionaryRangePredicateEvaluatorTest {
@Test
public void testBytesPredicateEvaluator() {
-
PredicateEvaluator predicateEvaluator = buildRangePredicate("[10\t\t20]",
FieldSpec.DataType.BYTES);
for (int i = 0x00; i < 0x30; i++) {
byte[] value = Integer.toString(i).getBytes();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
