You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2016/11/30 02:26:03 UTC
[2/2] kudu git commit: KUDU-1766: Java client partition pruning with
MAX_VALUE equality predicate fails
KUDU-1766: Java client partition pruning with MAX_VALUE equality predicate fails
The bug orginated in a mistransliteration of the c++ partition pruning
algorithm; the original version has always had the missing if statement:
https://github.com/apache/kudu/blob/53e67e9eb7b4fc940a14a17119041871e80bcc3f/src/kudu/common/key_util.cc#L152-L155
Change-Id: Iad6d00d9f401c510e14709323e7686bfa6d1b449
Reviewed-on: http://gerrit.cloudera.org:8080/5266
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f441b45b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f441b45b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f441b45b
Branch: refs/heads/master
Commit: f441b45bfb6f34aec7757f3e0fb143c4f9063fe5
Parents: 5de4837
Author: Dan Burkert <da...@apache.org>
Authored: Tue Nov 29 15:24:23 2016 -0800
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Wed Nov 30 02:19:56 2016 +0000
----------------------------------------------------------------------
.../org/apache/kudu/client/PartitionPruner.java | 5 +++-
.../apache/kudu/client/TestPartitionPruner.java | 24 ++++++++++++++++++--
src/kudu/common/partition_pruner-test.cc | 21 +++++++++++++++++
3 files changed, 47 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/f441b45b/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java b/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
index 1535173..e5786f3 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/PartitionPruner.java
@@ -476,7 +476,10 @@ public class PartitionPruner {
// key to convert it to an exclusive upper bound.
if (finalPredicate.getType() == KuduPredicate.PredicateType.EQUALITY ||
finalPredicate.getType() == KuduPredicate.PredicateType.IN_LIST) {
- incrementKey(row, rangePartitionColumnIdxs.subList(0, pushedPredicates));
+ // If the increment fails then this bound is is not constraining the keyspace.
+ if (!incrementKey(row, rangePartitionColumnIdxs.subList(0, pushedPredicates))) {
+ return AsyncKuduClient.EMPTY_ARRAY;
+ }
}
// Step 3: Fill the remaining columns without predicates with the min value.
http://git-wip-us.apache.org/repos/asf/kudu/blob/f441b45b/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
index c05083b..457457e 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartitionPruner.java
@@ -246,8 +246,7 @@ public class TestPartitionPruner extends BaseKuduTest {
// CREATE TABLE t
// (a INT8, b STRING, c INT8)
// PRIMARY KEY (a, b, c))
- // DISTRIBUTE BY RANGE(c, b);
- // PARTITION BY RANGE (a, b, c)
+ // PARTITION BY RANGE (c, b)
// (PARTITION VALUES < (0, "m"),
// PARTITION (0, "m") <= VALUES < (10, "r")
// PARTITION (10, "r") <= VALUES);
@@ -292,6 +291,13 @@ public class TestPartitionPruner extends BaseKuduTest {
assertEquals(3, countPartitions(table, partitions,
KuduPredicate.newComparisonPredicate(c, ComparisonOp.LESS, 100)));
+ // c < MIN
+ // TODO(dan): this should prune all partitions.
+ assertEquals(1, countPartitions(table, partitions,
+ KuduPredicate.newComparisonPredicate(c, ComparisonOp.LESS, Byte.MIN_VALUE)));
+ // c < MAX
+ assertEquals(3, countPartitions(table, partitions,
+ KuduPredicate.newComparisonPredicate(c, ComparisonOp.LESS, Byte.MAX_VALUE)));
// c >= -10
assertEquals(3, countPartitions(table, partitions,
@@ -313,6 +319,13 @@ public class TestPartitionPruner extends BaseKuduTest {
assertEquals(1, countPartitions(table, partitions,
KuduPredicate.newComparisonPredicate(c, ComparisonOp.GREATER_EQUAL, 100)));
+ // c >= MIN
+ assertEquals(3, countPartitions(table, partitions,
+ KuduPredicate.newComparisonPredicate(c, ComparisonOp.GREATER_EQUAL, Byte.MIN_VALUE)));
+ // c >= MAX
+ assertEquals(1, countPartitions(table, partitions,
+ KuduPredicate.newComparisonPredicate(c, ComparisonOp.GREATER_EQUAL, Byte.MAX_VALUE)));
+
// c >= -10
// c < 0
assertEquals(1, countPartitions(table, partitions,
@@ -397,6 +410,13 @@ public class TestPartitionPruner extends BaseKuduTest {
KuduPredicate.newComparisonPredicate(c, ComparisonOp.EQUAL, 0),
KuduPredicate.newComparisonPredicate(c, ComparisonOp.EQUAL, 2)));
+ // c = MIN
+ assertEquals(1, countPartitions(table, partitions,
+ KuduPredicate.newComparisonPredicate(c, ComparisonOp.EQUAL, Byte.MIN_VALUE)));
+ // c = MAX
+ assertEquals(1, countPartitions(table, partitions,
+ KuduPredicate.newComparisonPredicate(c, ComparisonOp.EQUAL, Byte.MAX_VALUE)));
+
// a IN (1, 2)
assertEquals(1, countPartitions(table, partitions,
KuduPredicate.newInListPredicate(c, ImmutableList.of((byte) 1, (byte) 2))));
http://git-wip-us.apache.org/repos/asf/kudu/blob/f441b45b/src/kudu/common/partition_pruner-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/partition_pruner-test.cc b/src/kudu/common/partition_pruner-test.cc
index 9c85420..b9c5fa2 100644
--- a/src/kudu/common/partition_pruner-test.cc
+++ b/src/kudu/common/partition_pruner-test.cc
@@ -327,6 +327,8 @@ TEST(TestPartitionPruner, TestRangePruning) {
int8_t five = 5;
int8_t ten = 10;
int8_t hundred = 100;
+ int8_t min = INT8_MIN;
+ int8_t max = INT8_MAX;
Slice empty = "";
Slice a = "a";
@@ -350,6 +352,13 @@ TEST(TestPartitionPruner, TestRangePruning) {
// c < 100
Check({ ColumnPredicate::Range(schema.column(2), nullptr, &hundred) }, 3);
+ // c < MIN
+ // TODO(dan): this should prune all partitions.
+ Check({ ColumnPredicate::Range(schema.column(2), nullptr, &min) }, 1);
+
+ // c < MAX
+ Check({ ColumnPredicate::Range(schema.column(2), nullptr, &max) }, 3);
+
// c >= -10
Check({ ColumnPredicate::Range(schema.column(0), &neg_ten, nullptr) }, 3);
@@ -365,6 +374,18 @@ TEST(TestPartitionPruner, TestRangePruning) {
// c >= 100
Check({ ColumnPredicate::Range(schema.column(2), &hundred, nullptr) }, 1);
+ // c >= MIN
+ Check({ ColumnPredicate::Range(schema.column(2), &min, nullptr) }, 3);
+
+ // c >= MAX
+ Check({ ColumnPredicate::Range(schema.column(2), &max, nullptr) }, 1);
+
+ // c = MIN
+ Check({ ColumnPredicate::Equality(schema.column(2), &min) }, 1);
+
+ // c = MAX
+ Check({ ColumnPredicate::Equality(schema.column(2), &max) }, 1);
+
// c >= -10
// c < 0
Check({ ColumnPredicate::Range(schema.column(2), &neg_ten, &zero) }, 1);