This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 6fdd91905 [client] KUDU-2671 move KuduRangePartition out of
KuduTableCreator
6fdd91905 is described below
commit 6fdd9190592fe47c2202e75edd2773832bf29968
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Wed Jul 20 17:27:40 2022 -0700
[client] KUDU-2671 move KuduRangePartition out of KuduTableCreator
This patch moves the KuduRangePartition class out of the
KuduTableCreator class. The reasoning behind this change is two-fold:
* make the API of KuduTableAlterer more consistent with the added
AddRangePartition(KuduRangePartition* partition) method
* avoid issues with embedded classes while cythonizing C++ client API
while adding support for range-specific hash schemas
This change might break ABI compatibility, but we haven't announced
support for range-specific hash schemas in Kudu 1.16 since it was not
ready yet. Any Kudu C++ client application that started using that
experimental API (which is very unlikely) should be recompiled with
updated headers and the kudu_client library.
Change-Id: I5af14e7e802baca496e13e05860d66685914dd29
Reviewed-on: http://gerrit.cloudera.org:8080/18765
Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
Reviewed-by: Mahesh Reddy <mre...@cloudera.com>
Tested-by: Alexey Serbin <ale...@apache.org>
Reviewed-by: Attila Bukor <abu...@apache.org>
---
src/kudu/client/client.cc | 32 +++---
src/kudu/client/client.h | 131 ++++++++++++-----------
src/kudu/client/flex_partitioning_client-test.cc | 12 +--
src/kudu/client/scan_token-test.cc | 18 ++--
src/kudu/client/table_alterer-internal.h | 2 +-
src/kudu/client/table_creator-internal.cc | 8 +-
src/kudu/client/table_creator-internal.h | 10 +-
7 files changed, 103 insertions(+), 110 deletions(-)
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 7adc33800..d31214371 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -1048,28 +1048,26 @@ Status KuduTableCreator::Create() {
return Status::OK();
}
-KuduTableCreator::KuduRangePartition::KuduRangePartition(
+KuduRangePartition::KuduRangePartition(
KuduPartialRow* lower_bound,
KuduPartialRow* upper_bound,
- RangePartitionBound lower_bound_type,
- RangePartitionBound upper_bound_type)
+ KuduTableCreator::RangePartitionBound lower_bound_type,
+ KuduTableCreator::RangePartitionBound upper_bound_type)
: data_(new Data(lower_bound, upper_bound, lower_bound_type,
upper_bound_type)) {
}
-KuduTableCreator::KuduRangePartition::~KuduRangePartition() {
+KuduRangePartition::~KuduRangePartition() {
delete data_;
}
-Status KuduTableCreator::KuduRangePartition::add_hash_partitions(
+Status KuduRangePartition::add_hash_partitions(
const vector<string>& columns,
int32_t num_buckets,
int32_t seed) {
if (seed < 0) {
- // TODO(aserbin): change the signature of
- // KuduRangePartition::add_hash_partitions() to use uint32_t
- // for the 'seed' parameter while it's still possible since
- // the client API hasn't been released yet
- return Status::InvalidArgument("hash seed must non-negative");
+ // int32_t, not uint32_t for seed is used to be "compatible" with the type
+ // of the 'seed' parameter for KuduTableCreator::add_hash_partitions().
+ return Status::InvalidArgument("hash seed must be non-negative");
}
return data_->add_hash_partitions(columns, num_buckets, seed);
}
@@ -1543,9 +1541,8 @@ KuduTableAlterer*
KuduTableAlterer::AddRangePartitionWithDimension(
Data::Step s { AlterTableRequestPB::ADD_RANGE_PARTITION,
nullptr,
- std::unique_ptr<KuduTableCreator::KuduRangePartition>(
- new KuduTableCreator::KuduRangePartition(
- lower_bound, upper_bound, lower_bound_type,
upper_bound_type)),
+ std::unique_ptr<KuduRangePartition>(new KuduRangePartition(
+ lower_bound, upper_bound, lower_bound_type,
upper_bound_type)),
dimension_label.empty() ? nullopt :
make_optional(dimension_label) };
data_->steps_.emplace_back(std::move(s));
data_->has_alter_partitioning_steps = true;
@@ -1553,7 +1550,7 @@ KuduTableAlterer*
KuduTableAlterer::AddRangePartitionWithDimension(
}
KuduTableAlterer* KuduTableAlterer::AddRangePartition(
- KuduTableCreator::KuduRangePartition* partition) {
+ KuduRangePartition* partition) {
CHECK(partition);
if (partition->data_->lower_bound_ == nullptr ||
partition->data_->upper_bound_ == nullptr) {
data_->status_ = Status::InvalidArgument("range partition bounds may not
be null");
@@ -1572,7 +1569,7 @@ KuduTableAlterer* KuduTableAlterer::AddRangePartition(
Data::Step s { AlterTableRequestPB::ADD_RANGE_PARTITION,
nullptr,
-
std::unique_ptr<KuduTableCreator::KuduRangePartition>(partition),
+ std::unique_ptr<KuduRangePartition>(partition),
nullopt };
data_->steps_.emplace_back(std::move(s));
data_->has_alter_partitioning_steps = true;
@@ -1604,9 +1601,8 @@ KuduTableAlterer* KuduTableAlterer::DropRangePartition(
Data::Step s { AlterTableRequestPB::DROP_RANGE_PARTITION,
nullptr,
- std::unique_ptr<KuduTableCreator::KuduRangePartition>(
- new KuduTableCreator::KuduRangePartition(
- lower_bound, upper_bound, lower_bound_type,
upper_bound_type)) };
+ std::unique_ptr<KuduRangePartition>(new KuduRangePartition(
+ lower_bound, upper_bound, lower_bound_type,
upper_bound_type)) };
data_->steps_.emplace_back(std::move(s));
data_->has_alter_partitioning_steps = true;
return this;
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index f2d07e2d5..26cdf9cb8 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -1220,68 +1220,6 @@ class KUDU_EXPORT KuduTableCreator {
INCLUSIVE_BOUND, ///< An inclusive bound.
};
- /// A helper class to represent a Kudu range partition with a custom hash
- /// bucket schema. The hash sub-partitioning for a range partition might be
- /// different from the default table-wide hash bucket schema specified during
- /// the creation of a table (see KuduTableCreator::add_hash_partitions()).
- /// Correspondingly, this class provides a means to specify a custom hash
- /// bucket structure for the data in a range partition.
- class KuduRangePartition {
- public:
- /// Create an object representing the range defined by the given
parameters.
- ///
- /// @param [in] lower_bound
- /// The lower bound for the range.
- /// The KuduRangePartition object takes ownership of the parameter.
- /// @param [in] upper_bound
- /// The upper bound for the range.
- /// The KuduRangePartition object takes ownership of the parameter.
- /// @param [in] lower_bound_type
- /// The type of the lower_bound: inclusive or exclusive; inclusive if the
- /// parameter is omitted.
- /// @param [in] upper_bound_type
- /// The type of the upper_bound: inclusive or exclusive; exclusive if the
- /// parameter is omitted.
- KuduRangePartition(KuduPartialRow* lower_bound,
- KuduPartialRow* upper_bound,
- RangePartitionBound lower_bound_type = INCLUSIVE_BOUND,
- RangePartitionBound upper_bound_type = EXCLUSIVE_BOUND);
-
- ~KuduRangePartition();
-
- /// Add a level of hash sub-partitioning for this range partition.
- ///
- /// The hash schema for the range partition is defined by the whole set of
- /// its hash sub-partitioning levels. A range partition can have multiple
- /// levels of hash sub-partitioning: this method can be called multiple
- /// times to define a multi-dimensional hash bucketing structure for the
- /// range. Alternatively, a range partition can have zero levels of hash
- /// sub-partitioning: simply don't call this method on a newly created
- /// @c KuduRangePartition object to have no hash sub-partitioning for the
- /// range represented by the object.
- ///
- /// @param [in] columns
- /// Names of columns to use for partitioning.
- /// @param [in] num_buckets
- /// Number of buckets for the hashing.
- /// @param [in] seed
- /// Hash seed for mapping rows to hash buckets.
- /// @return Operation result status.
- Status add_hash_partitions(const std::vector<std::string>& columns,
- int32_t num_buckets,
- int32_t seed = 0);
- private:
- class KUDU_NO_EXPORT Data;
-
- friend class KuduTableCreator;
- friend class KuduTableAlterer;
-
- // Owned.
- Data* data_;
-
- DISALLOW_COPY_AND_ASSIGN(KuduRangePartition);
- };
-
/// Add a range partition with table-wide hash bucket schema.
///
/// Multiple range partitions may be added, but they must not overlap. All
@@ -1331,7 +1269,7 @@ class KUDU_EXPORT KuduTableCreator {
/// The KuduTableCreator object takes ownership of the partition object.
/// @return Reference to the modified table creator.
KuduTableCreator& add_custom_range_partition(
- KuduRangePartition* partition);
+ class KuduRangePartition* partition);
/// Add a range partition split at the provided row.
///
@@ -1449,6 +1387,70 @@ class KUDU_EXPORT KuduTableCreator {
DISALLOW_COPY_AND_ASSIGN(KuduTableCreator);
};
+/// A helper class to represent a Kudu range partition with a custom hash
+/// bucket schema. The hash sub-partitioning for a range partition might be
+/// different from the default table-wide hash bucket schema specified during
+/// the creation of a table (see KuduTableCreator::add_hash_partitions()).
+/// Correspondingly, this class provides a means to specify a custom hash
+/// bucket structure for the data in a range partition.
+class KUDU_EXPORT KuduRangePartition {
+ public:
+ /// Create an object representing the range defined by the given parameters.
+ ///
+ /// @param [in] lower_bound
+ /// The lower bound for the range.
+ /// The KuduRangePartition object takes ownership of the parameter.
+ /// @param [in] upper_bound
+ /// The upper bound for the range.
+ /// The KuduRangePartition object takes ownership of the parameter.
+ /// @param [in] lower_bound_type
+ /// The type of the lower_bound: inclusive or exclusive; inclusive if the
+ /// parameter is omitted.
+ /// @param [in] upper_bound_type
+ /// The type of the upper_bound: inclusive or exclusive; exclusive if the
+ /// parameter is omitted.
+ KuduRangePartition(KuduPartialRow* lower_bound,
+ KuduPartialRow* upper_bound,
+ KuduTableCreator::RangePartitionBound lower_bound_type =
+ KuduTableCreator::INCLUSIVE_BOUND,
+ KuduTableCreator::RangePartitionBound upper_bound_type =
+ KuduTableCreator::EXCLUSIVE_BOUND);
+
+ ~KuduRangePartition();
+
+ /// Add a level of hash sub-partitioning for this range partition.
+ ///
+ /// The hash schema for the range partition is defined by the whole set of
+ /// its hash sub-partitioning levels. A range partition can have multiple
+ /// levels of hash sub-partitioning: this method can be called multiple
+ /// times to define a multi-dimensional hash bucketing structure for the
+ /// range. Alternatively, a range partition can have zero levels of hash
+ /// sub-partitioning: simply don't call this method on a newly created
+ /// @c KuduRangePartition object to have no hash sub-partitioning for the
+ /// range represented by the object.
+ ///
+ /// @param [in] columns
+ /// Names of columns to use for partitioning.
+ /// @param [in] num_buckets
+ /// Number of buckets for the hashing.
+ /// @param [in] seed
+ /// Hash seed for mapping rows to hash buckets.
+ /// @return Operation result status.
+ Status add_hash_partitions(const std::vector<std::string>& columns,
+ int32_t num_buckets,
+ int32_t seed = 0);
+ private:
+ class KUDU_NO_EXPORT Data;
+
+ friend class KuduTableCreator;
+ friend class KuduTableAlterer;
+
+ // Owned.
+ Data* data_;
+
+ DISALLOW_COPY_AND_ASSIGN(KuduRangePartition);
+};
+
/// @brief In-memory statistics of table.
class KUDU_EXPORT KuduTableStatistics {
public:
@@ -1906,8 +1908,7 @@ class KUDU_EXPORT KuduTableAlterer {
/// @param [in] partition
/// The range partition to be created: it can have a custom hash schema.
/// @return Raw pointer to this alterer object.
- KuduTableAlterer* AddRangePartition(
- KuduTableCreator::KuduRangePartition* partition);
+ KuduTableAlterer* AddRangePartition(KuduRangePartition* partition);
/// Add a range partition to the table with dimension label.
///
diff --git a/src/kudu/client/flex_partitioning_client-test.cc
b/src/kudu/client/flex_partitioning_client-test.cc
index c9d65ca72..77e67d990 100644
--- a/src/kudu/client/flex_partitioning_client-test.cc
+++ b/src/kudu/client/flex_partitioning_client-test.cc
@@ -129,7 +129,7 @@ class FlexPartitioningTest : public KuduTest {
}
protected:
- typedef unique_ptr<KuduTableCreator::KuduRangePartition> RangePartition;
+ typedef unique_ptr<KuduRangePartition> RangePartition;
typedef vector<RangePartition> RangePartitions;
static Status ApplyInsert(KuduSession* session,
@@ -211,9 +211,8 @@ class FlexPartitioningTest : public KuduTest {
CHECK_OK(lower->SetInt32(key_column, lower_bound));
unique_ptr<KuduPartialRow> upper(schema.NewRow());
CHECK_OK(upper->SetInt32(key_column, upper_bound));
- return unique_ptr<KuduTableCreator::KuduRangePartition>(
- new KuduTableCreator::KuduRangePartition(lower.release(),
- upper.release()));
+ return unique_ptr<KuduRangePartition>(
+ new KuduRangePartition(lower.release(), upper.release()));
}
RangePartition CreateRangePartition(int32_t lower_bound = 0,
@@ -224,9 +223,8 @@ class FlexPartitioningTest : public KuduTest {
RangePartition CreateRangePartitionNoUpperBound(int32_t lower_bound) {
unique_ptr<KuduPartialRow> lower(schema_.NewRow());
CHECK_OK(lower->SetInt32(kKeyColumn, lower_bound));
- return unique_ptr<KuduTableCreator::KuduRangePartition>(
- new KuduTableCreator::KuduRangePartition(lower.release(),
- schema_.NewRow()));
+ return unique_ptr<KuduRangePartition>(
+ new KuduRangePartition(lower.release(), schema_.NewRow()));
}
void CheckTabletCount(const char* table_name, int expected_count) {
diff --git a/src/kudu/client/scan_token-test.cc
b/src/kudu/client/scan_token-test.cc
index 4ac87d8ce..fca2ccbf6 100644
--- a/src/kudu/client/scan_token-test.cc
+++ b/src/kudu/client/scan_token-test.cc
@@ -604,9 +604,8 @@ TEST_F(ScanTokenTest,
ScanTokensWithCustomHashSchemasPerRange) {
unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
ASSERT_OK(lower_bound->SetInt64("col", 0));
ASSERT_OK(upper_bound->SetInt64("col", 100));
- unique_ptr<KuduTableCreator::KuduRangePartition> range_partition(
- new KuduTableCreator::KuduRangePartition(lower_bound.release(),
- upper_bound.release()));
+ unique_ptr<KuduRangePartition> range_partition(
+ new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
range_partition->add_hash_partitions({ "col" }, 4);
table_creator->add_custom_range_partition(range_partition.release());
}
@@ -616,9 +615,8 @@ TEST_F(ScanTokenTest,
ScanTokensWithCustomHashSchemasPerRange) {
unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
ASSERT_OK(lower_bound->SetInt64("col", 100));
ASSERT_OK(upper_bound->SetInt64("col", 200));
- unique_ptr<KuduTableCreator::KuduRangePartition> range_partition(
- new KuduTableCreator::KuduRangePartition(lower_bound.release(),
- upper_bound.release()));
+ unique_ptr<KuduRangePartition> range_partition(
+ new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
range_partition->add_hash_partitions({ "col" }, 2);
table_creator->add_custom_range_partition(range_partition.release());
}
@@ -752,8 +750,8 @@ TEST_F(ScanTokenTest,
TestScanTokensWithCustomHashSchemasPerNonCoveringRange) {
unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
ASSERT_OK(lower_bound->SetInt64("col", 0));
ASSERT_OK(upper_bound->SetInt64("col", 100));
- unique_ptr<KuduTableCreator::KuduRangePartition> range_partition(
- new KuduTableCreator::KuduRangePartition(lower_bound.release(),
upper_bound.release()));
+ unique_ptr<KuduRangePartition> range_partition(
+ new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
range_partition->add_hash_partitions({ "col" }, 4);
table_creator->add_custom_range_partition(range_partition.release());
}
@@ -763,8 +761,8 @@ TEST_F(ScanTokenTest,
TestScanTokensWithCustomHashSchemasPerNonCoveringRange) {
unique_ptr<KuduPartialRow> upper_bound(schema.NewRow());
ASSERT_OK(lower_bound->SetInt64("col", 200));
ASSERT_OK(upper_bound->SetInt64("col", 300));
- unique_ptr<KuduTableCreator::KuduRangePartition> range_partition(
- new KuduTableCreator::KuduRangePartition(lower_bound.release(),
upper_bound.release()));
+ unique_ptr<KuduRangePartition> range_partition(
+ new KuduRangePartition(lower_bound.release(),
upper_bound.release()));
range_partition->add_hash_partitions({ "col" }, 2);
table_creator->add_custom_range_partition(range_partition.release());
}
diff --git a/src/kudu/client/table_alterer-internal.h
b/src/kudu/client/table_alterer-internal.h
index c9ce547d2..b186fa890 100644
--- a/src/kudu/client/table_alterer-internal.h
+++ b/src/kudu/client/table_alterer-internal.h
@@ -57,7 +57,7 @@ class KuduTableAlterer::Data {
// The Kudu range partition to add or drop. Only set when the StepType is
// [ADD|DROP]_RANGE_PARTITION.
- std::unique_ptr<KuduTableCreator::KuduRangePartition> range_partition;
+ std::unique_ptr<KuduRangePartition> range_partition;
// The dimension label for tablet. Only set when the StepType is
ADD_RANGE_PARTITION.
std::optional<std::string> dimension_label;
diff --git a/src/kudu/client/table_creator-internal.cc
b/src/kudu/client/table_creator-internal.cc
index 85757afc1..d0dd218c2 100644
--- a/src/kudu/client/table_creator-internal.cc
+++ b/src/kudu/client/table_creator-internal.cc
@@ -32,11 +32,11 @@ KuduTableCreator::Data::Data(KuduClient* client)
wait_(true) {
}
-KuduTableCreator::KuduRangePartition::Data::Data(
+KuduRangePartition::Data::Data(
KuduPartialRow* lower_bound,
KuduPartialRow* upper_bound,
- RangePartitionBound lower_bound_type,
- RangePartitionBound upper_bound_type)
+ KuduTableCreator::RangePartitionBound lower_bound_type,
+ KuduTableCreator::RangePartitionBound upper_bound_type)
: lower_bound_type_(lower_bound_type),
upper_bound_type_(upper_bound_type),
lower_bound_(lower_bound),
@@ -44,7 +44,7 @@ KuduTableCreator::KuduRangePartition::Data::Data(
is_table_wide_hash_schema_(false) {
}
-Status KuduTableCreator::KuduRangePartition::Data::add_hash_partitions(
+Status KuduRangePartition::Data::add_hash_partitions(
const vector<string>& column_names,
int32_t num_buckets,
uint32_t seed) {
diff --git a/src/kudu/client/table_creator-internal.h
b/src/kudu/client/table_creator-internal.h
index 1d32a8bc5..0380d6d00 100644
--- a/src/kudu/client/table_creator-internal.h
+++ b/src/kudu/client/table_creator-internal.h
@@ -91,20 +91,20 @@ class KuduTableCreator::Data {
DISALLOW_COPY_AND_ASSIGN(Data);
};
-class KuduTableCreator::KuduRangePartition::Data {
+class KuduRangePartition::Data {
public:
Data(KuduPartialRow* lower_bound,
KuduPartialRow* upper_bound,
- RangePartitionBound lower_bound_type,
- RangePartitionBound upper_bound_type);
+ KuduTableCreator::RangePartitionBound lower_bound_type,
+ KuduTableCreator::RangePartitionBound upper_bound_type);
~Data() = default;
Status add_hash_partitions(const std::vector<std::string>& column_names,
int32_t num_buckets,
uint32_t seed);
- const RangePartitionBound lower_bound_type_;
- const RangePartitionBound upper_bound_type_;
+ const KuduTableCreator::RangePartitionBound lower_bound_type_;
+ const KuduTableCreator::RangePartitionBound upper_bound_type_;
std::unique_ptr<KuduPartialRow> lower_bound_;
std::unique_ptr<KuduPartialRow> upper_bound_;
