kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [2/2] kudu git commit: KUDU-1766: Java client partition pruning with MAX_VALUE equality predicate fails
Date Wed, 30 Nov 2016 02:26:03 GMT
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 <adar@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 <danburkert@apache.org>
Authored: Tue Nov 29 15:24:23 2016 -0800
Committer: Adar Dembo <adar@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);


Mime
View raw message