This is an automated email from the ASF dual-hosted git repository.
larsh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/master by this push:
new ae945a8 PHOENIX-5176 KeyRange.compareUpperRange(KeyRang 1, KeyRang 2)
returns wrong result when two key ranges have the same upper bound values but
one is inclusive and another is exclusive
ae945a8 is described below
commit ae945a84dd21da8d3547419436eaba67f137dd91
Author: Bin Shi <[email protected]>
AuthorDate: Mon Apr 8 16:40:30 2019 -0700
PHOENIX-5176 KeyRange.compareUpperRange(KeyRang 1, KeyRang 2) returns wrong
result when two key ranges have the same upper bound values but one is
inclusive and another is exclusive
---
.../java/org/apache/phoenix/query/KeyRange.java | 4 +-
.../org/apache/phoenix/query/KeyRangeMoreTest.java | 136 ++++++++++++---------
2 files changed, 77 insertions(+), 63 deletions(-)
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
index 2b66061..4229dfa 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
@@ -548,7 +548,7 @@ public class KeyRange implements Writable {
return Lists.transform(keys, POINT);
}
- private static int compareUpperRange(KeyRange rowKeyRange1,KeyRange
rowKeyRange2) {
+ public static int compareUpperRange(KeyRange rowKeyRange1,KeyRange
rowKeyRange2) {
int result = Boolean.compare(rowKeyRange1.upperUnbound(),
rowKeyRange2.upperUnbound());
if (result != 0) {
return result;
@@ -557,7 +557,7 @@ public class KeyRange implements Writable {
if (result != 0) {
return result;
}
- return Boolean.compare(rowKeyRange2.isUpperInclusive(),
rowKeyRange1.isUpperInclusive());
+ return Boolean.compare(rowKeyRange1.isUpperInclusive(),
rowKeyRange2.isUpperInclusive());
}
public static List<KeyRange> intersect(List<KeyRange> rowKeyRanges1,
List<KeyRange> rowKeyRanges2) {
diff --git
a/phoenix-core/src/test/java/org/apache/phoenix/query/KeyRangeMoreTest.java
b/phoenix-core/src/test/java/org/apache/phoenix/query/KeyRangeMoreTest.java
index 9710bf5..6f0c4c7 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/query/KeyRangeMoreTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/query/KeyRangeMoreTest.java
@@ -23,6 +23,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.List;
+import com.google.common.collect.Lists;
import org.apache.hadoop.hbase.util.Bytes;
import org.apache.phoenix.schema.types.PInteger;
import org.junit.Test;
@@ -126,8 +127,7 @@ public class KeyRangeMoreTest extends TestCase {
assertResult(result, maxStart,minEnd);
Collections.shuffle(rowKeyRanges1);
Collections.shuffle(rowKeyRanges2);
-
- };
+ }
}
private void assertResult(List<KeyRange> result,int start,int end) {
@@ -192,72 +192,86 @@ public class KeyRangeMoreTest extends TestCase {
listIntersectAndAssert(Arrays.asList(KeyRange.EMPTY_RANGE),Arrays.asList(KeyRange.EVERYTHING_RANGE),Arrays.asList(KeyRange.EMPTY_RANGE));
- rowKeyRanges1=Arrays.asList(
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(2),
- true,
- PInteger.INSTANCE.toBytes(5),
- true),
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(8),
- true,
- KeyRange.UNBOUND,
- false));
- rowKeyRanges2=Arrays.asList(
- PInteger.INSTANCE.getKeyRange(
- KeyRange.UNBOUND,
- false,
- PInteger.INSTANCE.toBytes(4),
- true),
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(7),
- true,
- PInteger.INSTANCE.toBytes(10),
- true),
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(13),
- true,
- PInteger.INSTANCE.toBytes(14),
- true),
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(19),
- true,
- KeyRange.UNBOUND,
- false)
- );
- expected=Arrays.asList(
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(2),
- true,
- PInteger.INSTANCE.toBytes(4),
- true),
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(8),
- true,
- PInteger.INSTANCE.toBytes(10),
- true),
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(13),
- true,
- PInteger.INSTANCE.toBytes(14),
- true),
- PInteger.INSTANCE.getKeyRange(
- PInteger.INSTANCE.toBytes(19),
- true,
- KeyRange.UNBOUND,
- false)
- );
+ rowKeyRanges1 = createKeyRangeList(
+ Arrays.asList(2, 5, 8, Integer.MAX_VALUE),
+ Arrays.asList(true, true, true, false));
+ rowKeyRanges2 = createKeyRangeList(
+ Arrays.asList(Integer.MIN_VALUE, 4, 7, 10, 13, 14, 19,
Integer.MAX_VALUE),
+ Arrays.asList(false, true, true, true, true, true, true,
false));
+ expected = createKeyRangeList(
+ Arrays.asList(2, 4, 8, 10, 13, 14, 19, Integer.MAX_VALUE),
+ Arrays.asList(true, true, true, true, true, true, true,
false));
+ listIntersectAndAssert(rowKeyRanges1, rowKeyRanges2, expected);
+
+ rowKeyRanges1 = createKeyRangeList(
+ Arrays.asList(3, 5, 5, 6),
+ Arrays.asList(true, false, true, false));
+ rowKeyRanges2 = createKeyRangeList(
+ Arrays.asList(3, 5, 6, 7),
+ Arrays.asList(true, true, true, true));
+ expected = createKeyRangeList(
+ Arrays.asList(3, 5),
+ Arrays.asList(true, true));
listIntersectAndAssert(rowKeyRanges1, rowKeyRanges2, expected);
}
+ @Test
+ public void testKeyRangeCompareUpperRange() throws Exception {
+ List<KeyRange> rowKeyRanges1 = createKeyRangeListWithFixedLowerRange(
+ Arrays.asList(Integer.MAX_VALUE, Integer.MAX_VALUE, 10000,
1001, 1000, 1000, 1000, 1000, 1000),
+ Arrays.asList(false, false, true, true, true, true, false,
true, false));
+ List<KeyRange> rowKeyRanges2 = createKeyRangeListWithFixedLowerRange(
+ Arrays.asList(Integer.MAX_VALUE, 10000, Integer.MAX_VALUE,
1000, 1001, 1000, 1000, 1000, 1000),
+ Arrays.asList(false, false, false, true, true, true, false,
false, true));
+ List<Integer> expectedResults = Arrays.asList(0, 1, -1, 1, -1, 0, 0,
1, -1);
+ assertEquals(rowKeyRanges1.size(), rowKeyRanges2.size());
+ assertEquals(rowKeyRanges1.size(), expectedResults.size());
+
+ for (int i = 0; i < expectedResults.size(); i++) {
+ int compareResult =
KeyRange.compareUpperRange(rowKeyRanges1.get(i), rowKeyRanges2.get(i));
+ assertEquals(expectedResults.get(i).intValue(), compareResult);
+ }
+ }
+
+ private static List<KeyRange>
createKeyRangeListWithFixedLowerRange(List<Integer> keys, List<Boolean>
boundaryConditions) {
+ assertEquals(keys.size(), boundaryConditions.size());
+ List<Integer> newKeys = Lists.newArrayListWithCapacity(keys.size() *
2);
+ List<Boolean> newBoundaryConditions =
Lists.newArrayListWithCapacity(boundaryConditions.size() * 2);
+
+ for (int i = 0; i < keys.size(); i++) {
+ newKeys.add(0);
+ newBoundaryConditions.add(true);
+ newKeys.add(keys.get(i));
+ newBoundaryConditions.add(boundaryConditions.get(i));
+ }
+
+ return createKeyRangeList(newKeys, newBoundaryConditions);
+ }
+
+ private static List<KeyRange> createKeyRangeList(List<Integer> keys,
List<Boolean> boundaryConditions) {
+ assertEquals(keys.size(), boundaryConditions.size());
+ assertTrue(keys.size() % 2 == 0);
+
+ int size = keys.size() / 2;
+ List<KeyRange> keyRangeList = Lists.newArrayListWithCapacity(size);
+
+ for (int i = 0; i < size; i++) {
+ byte[] startKey = keys.get(2*i).equals(Integer.MIN_VALUE) ?
KeyRange.UNBOUND : PInteger.INSTANCE.toBytes(keys.get(2*i));
+ byte[] endKey = keys.get(2*i + 1).equals(Integer.MAX_VALUE) ?
KeyRange.UNBOUND : PInteger.INSTANCE.toBytes(keys.get(2*i + 1));
+ keyRangeList.add(PInteger.INSTANCE.getKeyRange(startKey,
boundaryConditions.get(2*i), endKey, boundaryConditions.get(2*i+1)));
+ }
+
+ return keyRangeList;
+ }
+
private static void listIntersectAndAssert(List<KeyRange>
rowKeyRanges1,List<KeyRange> rowKeyRanges2,List<KeyRange> expected) {
- for(int i=0;i<200;i++) {
- List<KeyRange> result=KeyRange.intersect(rowKeyRanges1,
rowKeyRanges2);
+ for (int i = 0; i < 200; i++) {
+ List<KeyRange> result = KeyRange.intersect(rowKeyRanges1,
rowKeyRanges2);
assertEquals(expected, result);
- result=KeyRange.intersect(rowKeyRanges2, rowKeyRanges1);
+ result = KeyRange.intersect(rowKeyRanges2, rowKeyRanges1);
assertEquals(expected, result);
Collections.shuffle(rowKeyRanges1);
Collections.shuffle(rowKeyRanges2);
- };
+ }
}
}
