Skip to content

Commit

Permalink
KUDU-2173: Partitions are incorrectly pruned when range-partitioned o…
Browse files Browse the repository at this point in the history
…n a PK prefix

The partition pruner mistakenly treated a range partition which is a
proper prefix of a primary key as an exclusive bound, when in fact it's
an inclusive bound if the remaining PK column constraints are greater
than the min value.

This is a C++-only bug; the Java client only attempts to use the PK as
the range partition bound when the primary key columns match the range
partition columns exactly (see KUDU-2178). Regardless, I added Java
regression tests in order to cover the case when the Java pruner is
improved.

Change-Id: I38752f50c0910cd157a912eaa272c76a1a0d9b59
Reviewed-on: http://gerrit.cloudera.org:8080/8222
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <[email protected]>
  • Loading branch information
danburkert authored and toddlipcon committed Oct 9, 2017
1 parent 29a7568 commit 0c82398
Show file tree
Hide file tree
Showing 6 changed files with 320 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ public void testPrimaryKeyRangePruning() throws Exception {

byte min = Byte.MIN_VALUE;


// No bounds
checkPartitionsPrimaryKey(3, table, partitions,
null, null);
Expand All @@ -217,6 +216,14 @@ public void testPrimaryKeyRangePruning() throws Exception {
checkPartitionsPrimaryKey(1, table, partitions,
null, new byte[] { -1, min, min });

// PK < (0, 0, 0)
checkPartitionsPrimaryKey(1, table, partitions,
null, new byte[] { 0, 0, 0 });

// PK < (0, 0, min)
checkPartitionsPrimaryKey(1, table, partitions,
null, new byte[] { 0, 0, min });

// PK < (10, 10, 10)
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { 10, 10, 10 });
Expand Down Expand Up @@ -256,7 +263,86 @@ public void testPrimaryKeyRangePruning() throws Exception {
// PK >= (10, 10, 11)
checkPartitionsPrimaryKey(0, table, partitions,
new byte[] { 10, 0, 0 }, new byte[] { 0, 0, 0 });
}

@Test
public void testPrimaryKeyPrefixRangePruning() throws Exception {
// CREATE TABLE t
// (a INT8, b INT8, c INT8)
// PRIMARY KEY (a, b, c))
// PARTITION BY RANGE (a, b)
// (PARTITION VALUES < (0, 0, 0));

ArrayList<ColumnSchema> columns = new ArrayList<>(3);
columns.add(new ColumnSchema.ColumnSchemaBuilder("a", Type.INT8).key(true).build());
columns.add(new ColumnSchema.ColumnSchemaBuilder("b", Type.INT8).key(true).build());
columns.add(new ColumnSchema.ColumnSchemaBuilder("c", Type.INT8).key(true).build());
Schema schema = new Schema(columns);

CreateTableOptions tableBuilder = new CreateTableOptions();
tableBuilder.setRangePartitionColumns(ImmutableList.of("a", "b"));

PartialRow split = schema.newPartialRow();
split.addByte("a", (byte) 0);
split.addByte("b", (byte) 0);
tableBuilder.addSplitRow(split);

String tableName = "testPrimaryKeyPrefixRangePruning-" + System.currentTimeMillis();

syncClient.createTable(tableName, schema, tableBuilder);
KuduTable table = syncClient.openTable(tableName);
List<Partition> partitions = getTablePartitions(table);

byte min = Byte.MIN_VALUE;
byte max = Byte.MAX_VALUE;

// No bounds
checkPartitionsPrimaryKey(2, table, partitions,
null, null);

// PK < (-1, min, min)
// TODO(KUDU-2178): prune the upper partition.
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { -1, min, min });

// PK < (0, 0, min)
// TODO(KUDU-2178): prune the upper partition.
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { 0, 0, min });

// PK < (0, 0, 0)
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { 0, 0, 0 });

// PK < (0, 1, min)
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { 0, 1, min });

// PK < (0, 1, 0)
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { 0, 1, 0 });

// PK < (max, max, min)
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { max, max, min });

// PK < (max, max, 0)
checkPartitionsPrimaryKey(2, table, partitions,
null, new byte[] { max, max, 0 });

// PK >= (0, 0, min)
// TODO(KUDU-2178): prune the lower partition.
checkPartitionsPrimaryKey(2, table, partitions,
new byte[] { 0, 0, min }, null);

// PK >= (0, 0, 0)
// TODO(KUDU-2178): prune the lower partition.
checkPartitionsPrimaryKey(2, table, partitions,
new byte[] { 0, 0, 0 }, null);

// PK >= (0, -1, 0)
checkPartitionsPrimaryKey(2, table, partitions,
new byte[] { 0, -1, 0 }, null);
}

@Test
Expand Down
7 changes: 5 additions & 2 deletions src/kudu/common/key_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,12 @@ int PushLowerBoundKeyPredicates(ColIdxIter first,
} // anonymous namespace

bool IncrementPrimaryKey(ContiguousRow* row, Arena* arena) {
int32_t num_pk_cols = row->schema()->num_key_columns();
return IncrementPrimaryKey(row, row->schema()->num_key_columns(), arena);
}

bool IncrementPrimaryKey(ContiguousRow* row, int32_t num_columns, Arena* arena) {
return IncrementKey(boost::make_counting_iterator(0),
boost::make_counting_iterator(num_pk_cols),
boost::make_counting_iterator(num_columns),
row,
arena);
}
Expand Down
4 changes: 4 additions & 0 deletions src/kudu/common/key_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ namespace key_util {
// REQUIRES: all primary key columns must be initialized.
bool IncrementPrimaryKey(ContiguousRow* row, Arena* arena) WARN_UNUSED_RESULT;

// Like 'IncrementPrimaryKey(ContiguousRow*, Arena*)', but only increments up to
// 'num_columns' columns of the primary key prefix.
bool IncrementPrimaryKey(ContiguousRow* row, int32_t num_columns, Arena* arena) WARN_UNUSED_RESULT;

// Increments the provided cell in place.
bool IncrementCell(const ColumnSchema& col, void* cell_ptr, Arena* arena);

Expand Down
3 changes: 2 additions & 1 deletion src/kudu/common/partial_row.h
Original file line number Diff line number Diff line change
Expand Up @@ -488,8 +488,9 @@ class KUDU_EXPORT KuduPartialRow {
template<typename KeyTypeWrapper> friend struct client::IntKeysTestSetup;
template<typename KeyTypeWrapper> friend struct tablet::SliceTypeRowOps;
template<typename KeyTypeWrapper> friend struct tablet::NumTypeRowOps;
FRIEND_TEST(PartitionPrunerTest, TestPrimaryKeyRangePruning);
FRIEND_TEST(PartitionPrunerTest, TestIntPartialPrimaryKeyRangePruning);
FRIEND_TEST(PartitionPrunerTest, TestPartialPrimaryKeyRangePruning);
FRIEND_TEST(PartitionPrunerTest, TestPrimaryKeyRangePruning);

template<typename T>
Status Set(const Slice& col_name, const typename T::cpp_type& val,
Expand Down
Loading

0 comments on commit 0c82398

Please sign in to comment.