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 79bd575 [client] replace Equals() with operator==()
79bd575 is described below
commit 79bd57546996c6e3c130960ba099fb637d58d51f
Author: Alexey Serbin <[email protected]>
AuthorDate: Mon Jun 7 18:21:26 2021 -0700
[client] replace Equals() with operator==()
This patch replaces the 'bool Equals(const X&)' method for Partition and
PartitionSchema classes with 'bool operator==(const X&)'.
For KuduSchema and KuduColumnSchema, both operator==() and operator!=()
were introduced, and the Equals() method was marked as deprecated since
KuduSchema's and KuduColumnsSchema's API are public at this point.
I decided to perform minor refactoring while changing other code around:
the presence of Equals() method looks a bit lame in a C++ API.
This patch doesn't contain any functional modifications.
Change-Id: I4a95534835c93b1fb7a3ca1f311c530572c50ad2
Reviewed-on: http://gerrit.cloudera.org:8080/17558
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <[email protected]>
---
src/kudu/client/client-test.cc | 4 +--
src/kudu/client/client-unittest.cc | 11 +++---
src/kudu/client/schema.cc | 25 ++++++++++---
src/kudu/client/schema.h | 42 ++++++++++++++++++++--
src/kudu/common/partition-test.cc | 4 +--
src/kudu/common/partition.cc | 50 ++++++++++++++++++--------
src/kudu/common/partition.h | 8 ++---
src/kudu/integration-tests/alter_table-test.cc | 2 +-
src/kudu/integration-tests/test_workload.cc | 2 +-
src/kudu/tablet/tablet_metadata.cc | 4 +--
src/kudu/tools/table_scanner.cc | 9 ++---
11 files changed, 117 insertions(+), 44 deletions(-)
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index d0ee1c0..2dd82a0 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -4895,11 +4895,11 @@ TEST_F(ClientTest, TestGetTableSchema) {
// Verify the schema for the current table
ASSERT_OK(client_->GetTableSchema(kTableName, &schema));
- ASSERT_TRUE(schema_.Equals(schema));
+ ASSERT_EQ(schema_, schema);
// Verify that a get schema request for a missing table throws not found
Status s = client_->GetTableSchema("MissingTableName", &schema);
- ASSERT_TRUE(s.IsNotFound());
+ ASSERT_TRUE(s.IsNotFound()) << s.ToString();
ASSERT_STR_CONTAINS(s.ToString(), "the table does not exist");
}
diff --git a/src/kudu/client/client-unittest.cc
b/src/kudu/client/client-unittest.cc
index 5c80386..2c0fb01 100644
--- a/src/kudu/client/client-unittest.cc
+++ b/src/kudu/client/client-unittest.cc
@@ -318,21 +318,22 @@ TEST(ClientUnitTest, TestKuduSchemaToStringWithColumnIds)
{
TEST(KuduColumnSchemaTest, TestEquals) {
KuduColumnSchema a32("a", KuduColumnSchema::INT32);
- ASSERT_TRUE(a32.Equals(a32));
+ ASSERT_TRUE(a32 == a32);
KuduColumnSchema a32_2(a32);
- ASSERT_TRUE(a32.Equals(a32_2));
+ ASSERT_EQ(a32, a32_2);
KuduColumnSchema b32("b", KuduColumnSchema::INT32);
- ASSERT_FALSE(a32.Equals(b32));
+ ASSERT_FALSE(a32 == b32);
+ ASSERT_TRUE(a32 != b32);
KuduColumnSchema a16("a", KuduColumnSchema::INT16);
- ASSERT_FALSE(a32.Equals(a16));
+ ASSERT_NE(a32, a16);
const int kDefaultOf7 = 7;
KuduColumnSchema a32_dflt("a", KuduColumnSchema::INT32,
/*is_nullable=*/false,
/*default_value=*/&kDefaultOf7);
- ASSERT_FALSE(a32.Equals(a32_dflt));
+ ASSERT_NE(a32, a32_dflt);
}
} // namespace client
diff --git a/src/kudu/client/schema.cc b/src/kudu/client/schema.cc
index 66085bb..4a9f1c3 100644
--- a/src/kudu/client/schema.cc
+++ b/src/kudu/client/schema.cc
@@ -778,9 +778,16 @@ void KuduColumnSchema::CopyFrom(const KuduColumnSchema&
other) {
}
bool KuduColumnSchema::Equals(const KuduColumnSchema& other) const {
- return this == &other ||
- col_ == other.col_ ||
- (col_ != nullptr && col_->Equals(*other.col_, ColumnSchema::COMPARE_ALL));
+ return *this == other;
+}
+
+bool KuduColumnSchema::operator==(const KuduColumnSchema& rhs) const {
+ return this == &rhs || col_ == rhs.col_ ||
+ (col_ != nullptr && col_->Equals(*rhs.col_, ColumnSchema::COMPARE_ALL));
+}
+
+bool KuduColumnSchema::operator!=(const KuduColumnSchema& rhs) const {
+ return !(*this == rhs);
}
const string& KuduColumnSchema::name() const {
@@ -856,9 +863,17 @@ Status KuduSchema::Reset(const vector<KuduColumnSchema>&
columns, int key_column
return Status::OK();
}
+bool KuduSchema::operator==(const KuduSchema& rhs) const {
+ return this == &rhs ||
+ (schema_ && rhs.schema_ && schema_->Equals(*rhs.schema_));
+}
+
+bool KuduSchema::operator!=(const KuduSchema& rhs) const {
+ return !(this == &rhs);
+}
+
bool KuduSchema::Equals(const KuduSchema& other) const {
- return this == &other ||
- (schema_ && other.schema_ && schema_->Equals(*other.schema_));
+ return *this == other;
}
KuduColumnSchema KuduSchema::Column(size_t idx) const {
diff --git a/src/kudu/client/schema.h b/src/kudu/client/schema.h
index 7e3f2a9..086e7d9 100644
--- a/src/kudu/client/schema.h
+++ b/src/kudu/client/schema.h
@@ -265,10 +265,29 @@ class KUDU_EXPORT KuduColumnSchema {
/// Check whether the object is identical to the other one.
///
+ /// @deprecated use operator==(const KuduColumnSchema&) instead
+ ///
/// @param [in] other
/// The reference object to compare with.
/// @return @c true iff the object is identical to the specified one.
- bool Equals(const KuduColumnSchema& other) const;
+ bool Equals(const KuduColumnSchema& other) const
+ ATTRIBUTE_DEPRECATED("use operator==(const KuduColumnSchema&) instead");
+
+ /// Check whether the schema is identical to the other one.
+ ///
+ /// @param [in] rhs
+ /// KuduColumnSchema object to compare this one with.
+ /// @return @c true iff this KuduColumnSchema object is identical
+ /// to the specified one.
+ bool operator==(const KuduColumnSchema& rhs) const;
+
+ /// Check whether the schema is not identical to the other one.
+ ///
+ /// @param [in] rhs
+ /// KuduColumnSchema object to compare this one with.
+ /// @return @c true iff this KuduColumnSchema object is not identical
+ /// to the specified one.
+ bool operator!=(const KuduColumnSchema& rhs) const;
/// @name Getters to expose column schema information.
///
@@ -633,11 +652,30 @@ class KUDU_EXPORT KuduSchema {
/// Check whether the schema is identical to the other one.
///
+ /// @deprecated use operator==(const KuduSchema&) instead
+ ///
/// @param [in] other
/// The other KuduSchema object to compare with.
/// @return @c true iff this KuduSchema object is identical
/// to the specified one.
- bool Equals(const KuduSchema& other) const;
+ bool Equals(const KuduSchema& other) const
+ ATTRIBUTE_DEPRECATED("use operator==(const KuduSchema&) instead");
+
+ /// Check whether the schema is identical to the other one.
+ ///
+ /// @param [in] rhs
+ /// KuduSchema object to compare this one with.
+ /// @return @c true iff this KuduSchema object is identical
+ /// to the specified one.
+ bool operator==(const KuduSchema& rhs) const;
+
+ /// Check whether the schema is not identical to the other one.
+ ///
+ /// @param [in] rhs
+ /// KuduSchema object to compare this one with.
+ /// @return @c true iff this KuduSchema object is not identical
+ /// to the specified one.
+ bool operator!=(const KuduSchema& rhs) const;
/// @param [in] idx
/// Column index.
diff --git a/src/kudu/common/partition-test.cc
b/src/kudu/common/partition-test.cc
index e71cce0..5b63260 100644
--- a/src/kudu/common/partition-test.cc
+++ b/src/kudu/common/partition-test.cc
@@ -982,7 +982,6 @@ void CheckPartitions(const vector<Partition>& partitions) {
void CheckSerializationFunctions(const PartitionSchemaPB& pb,
const PartitionSchema& partition_schema,
const Schema& schema) {
-
PartitionSchemaPB pb1;
ASSERT_OK(partition_schema.ToPB(schema, &pb1));
@@ -991,8 +990,7 @@ void CheckSerializationFunctions(const PartitionSchemaPB&
pb,
PartitionSchema partition_schema1;
ASSERT_OK(PartitionSchema::FromPB(pb1, schema, &partition_schema1));
-
- ASSERT_TRUE(partition_schema.Equals(partition_schema1));
+ ASSERT_EQ(partition_schema, partition_schema1);
}
} // namespace
diff --git a/src/kudu/common/partition.cc b/src/kudu/common/partition.cc
index e7787d0..6aa5cb5 100644
--- a/src/kudu/common/partition.cc
+++ b/src/kudu/common/partition.cc
@@ -84,11 +84,19 @@ Slice Partition::range_key(const string& partition_key)
const {
}
}
-bool Partition::Equals(const Partition& other) const {
- if (this == &other) return true;
- if (partition_key_start() != other.partition_key_start()) return false;
- if (partition_key_end() != other.partition_key_end()) return false;
- if (hash_buckets_ != other.hash_buckets_) return false;
+bool Partition::operator==(const Partition& rhs) const {
+ if (this == &rhs) {
+ return true;
+ }
+ if (partition_key_start() != rhs.partition_key_start()) {
+ return false;
+ }
+ if (partition_key_end() != rhs.partition_key_end()) {
+ return false;
+ }
+ if (hash_buckets_ != rhs.hash_buckets_) {
+ return false;
+ }
return true;
}
@@ -1098,20 +1106,32 @@ string PartitionSchema::PartitionTableEntry(const
Schema& schema,
return entry;
}
-bool PartitionSchema::Equals(const PartitionSchema& other) const {
- if (this == &other) return true;
+bool PartitionSchema::operator==(const PartitionSchema& rhs) const {
+ if (this == &rhs) {
+ return true;
+ }
// Compare range component.
- if (range_schema_.column_ids != other.range_schema_.column_ids) return false;
+ if (range_schema_.column_ids != rhs.range_schema_.column_ids) {
+ return false;
+ }
// Compare hash bucket components.
- if (hash_bucket_schemas_.size() != other.hash_bucket_schemas_.size()) return
false;
- for (int i = 0; i < hash_bucket_schemas_.size(); i++) {
- if (hash_bucket_schemas_[i].seed != other.hash_bucket_schemas_[i].seed)
return false;
- if (hash_bucket_schemas_[i].num_buckets
- != other.hash_bucket_schemas_[i].num_buckets) return false;
- if (hash_bucket_schemas_[i].column_ids
- != other.hash_bucket_schemas_[i].column_ids) return false;
+ const auto& hb_schemas = hash_bucket_schemas_;
+ const auto& rhs_hb_schemas = rhs.hash_bucket_schemas_;
+ if (hb_schemas.size() != rhs_hb_schemas.size()) {
+ return false;
+ }
+ for (size_t i = 0; i < hb_schemas.size(); ++i) {
+ if (hb_schemas[i].seed != rhs_hb_schemas[i].seed) {
+ return false;
+ }
+ if (hb_schemas[i].num_buckets != rhs_hb_schemas[i].num_buckets) {
+ return false;
+ }
+ if (hb_schemas[i].column_ids != rhs_hb_schemas[i].column_ids) {
+ return false;
+ }
}
return true;
diff --git a/src/kudu/common/partition.h b/src/kudu/common/partition.h
index 2226e45..c37c7c5 100644
--- a/src/kudu/common/partition.h
+++ b/src/kudu/common/partition.h
@@ -73,8 +73,8 @@ class Partition {
return partition_key_end_;
}
- // Returns true if the other partition is equivalent to this one.
- bool Equals(const Partition& other) const;
+ // Returns true iff the given partition 'rhs' is equivalent to this one.
+ bool operator==(const Partition& rhs) const;
// Serializes a partition into a protobuf message.
void ToPB(PartitionPB* pb) const;
@@ -281,8 +281,8 @@ class PartitionSchema {
std::string PartitionTableHeader(const Schema& schema) const;
std::string PartitionTableEntry(const Schema& schema, const Partition&
partition) const;
- // Returns true if the other partition schema is equivalent to this one.
- bool Equals(const PartitionSchema& other) const;
+ // Returns 'true' iff the partition schema 'rhs' is equivalent to this one.
+ bool operator==(const PartitionSchema& rhs) const;
// Transforms an exclusive lower bound range partition key into an inclusive
// lower bound range partition key.
diff --git a/src/kudu/integration-tests/alter_table-test.cc
b/src/kudu/integration-tests/alter_table-test.cc
index 42e4818..09bcb05 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -556,7 +556,7 @@ TEST_F(AlterTableTest, TestAlterOnTSRestart) {
KuduSchema schema;
bool alter_in_progress = false;
ASSERT_OK(client_->GetTableSchema(kTableName, &schema));
- ASSERT_TRUE(schema_.Equals(schema));
+ ASSERT_EQ(schema_, schema);
ASSERT_OK(client_->IsAlterTableInProgress(kTableName, &alter_in_progress));
ASSERT_TRUE(alter_in_progress);
diff --git a/src/kudu/integration-tests/test_workload.cc
b/src/kudu/integration-tests/test_workload.cc
index 51e089a..d59b04e 100644
--- a/src/kudu/integration-tests/test_workload.cc
+++ b/src/kudu/integration-tests/test_workload.cc
@@ -392,7 +392,7 @@ void TestWorkload::Setup() {
} else {
KuduSchema existing_schema;
CHECK_OK(client_->GetTableSchema(table_name_, &existing_schema));
- CHECK(schema_.Equals(existing_schema))
+ CHECK(schema_ == existing_schema)
<< "Existing table's schema doesn't match ours";
LOG(INFO) << "TestWorkload: Skipping table creation because table "
<< table_name_ << " already exists";
diff --git a/src/kudu/tablet/tablet_metadata.cc
b/src/kudu/tablet/tablet_metadata.cc
index 6e635a5..809a5c9 100644
--- a/src/kudu/tablet/tablet_metadata.cc
+++ b/src/kudu/tablet/tablet_metadata.cc
@@ -415,11 +415,11 @@ Status TabletMetadata::LoadFromSuperBlock(const
TabletSuperBlockPB& superblock)
PartitionSchema partition_schema;
RETURN_NOT_OK(PartitionSchema::FromPB(superblock.partition_schema(),
*schema_, &partition_schema));
- CHECK(partition_schema_.Equals(partition_schema));
+ CHECK(partition_schema_ == partition_schema);
Partition partition;
Partition::FromPB(superblock.partition(), &partition);
- CHECK(partition_.Equals(partition));
+ CHECK(partition_ == partition);
}
tablet_data_state_ = superblock.tablet_data_state();
diff --git a/src/kudu/tools/table_scanner.cc b/src/kudu/tools/table_scanner.cc
index 3fa89cd..d88b3c1 100644
--- a/src/kudu/tools/table_scanner.cc
+++ b/src/kudu/tools/table_scanner.cc
@@ -337,7 +337,7 @@ Status CreateDstTableIfNeeded(const
client::sp::shared_ptr<KuduTable>& src_table
const string& dst_table_name) {
client::sp::shared_ptr<KuduTable> dst_table;
Status s = dst_client->OpenTable(dst_table_name, &dst_table);
- if (!s.IsNotFound() && !s.ok()) {
+ if (!s.IsNotFound()) {
return s;
}
@@ -349,9 +349,10 @@ Status CreateDstTableIfNeeded(const
client::sp::shared_ptr<KuduTable>& src_table
}
const KuduSchema& dst_table_schema = dst_table->schema();
- if (!src_table_schema.Equals(dst_table_schema)) {
- return Status::NotSupported(
- "Not support different schema of source table and destination
table.");
+ if (src_table_schema != dst_table_schema) {
+ return Status::NotSupported(Substitute(
+ "destination table's schema differs from the source one ($0 vs $1)",
+ dst_table_schema.ToString(), src_table_schema.ToString()));
}
return Status::OK();
