pitrou commented on code in PR #13567:
URL: https://github.com/apache/arrow/pull/13567#discussion_r918650561
##########
cpp/src/arrow/dataset/partition_test.cc:
##########
@@ -426,6 +453,37 @@ TEST_F(TestPartitioning, HivePartitioning) {
AssertParseError("/alpha=0.0/beta=3.25/"); // conversion of "0.0" to int32
fails
}
+TEST_F(TestPartitioning, HivePartitioningEquals) {
+ const auto& array_vector = ArrayVector();
+ const auto& other_vector = {ArrayFromJSON(utf8(), R"(["foo", "bar",
"baz"])"),
+ ArrayFromJSON(utf8(), R"(["bar", "foo",
"baz"])")};
+ auto part = std::make_shared<HivePartitioning>(
+ schema({field("alpha", int32()), field("beta", float32())}),
array_vector, "xyz");
+ auto other_part = std::make_shared<HivePartitioning>(
+ schema({field("sigma", int32()), field("beta", float32())}),
array_vector, "xyz");
+ auto another_part = std::make_shared<HivePartitioning>(
+ schema({field("alpha", int32()), field("beta", float32())}),
other_vector, "xyz");
+ auto some_part = std::make_shared<HivePartitioning>(
+ schema({field("alpha", int32()), field("beta", float32())}),
array_vector, "abc");
Review Comment:
Same here.
##########
cpp/src/arrow/dataset/partition.h:
##########
@@ -63,13 +64,18 @@ struct ARROW_DS_EXPORT PartitionPathFormat {
/// Paths are consumed from left to right. Paths must be relative to
/// the root of a partition; path prefixes must be removed before passing
/// the path to a partitioning for parsing.
-class ARROW_DS_EXPORT Partitioning {
+class ARROW_DS_EXPORT Partitioning : public
util::EqualityComparable<Partitioning> {
public:
virtual ~Partitioning() = default;
/// \brief The name identifying the kind of partitioning
virtual std::string type_name() const = 0;
+ //// \brief Return whether the partitionings are equal
+ virtual bool Equals(const Partitioning& other) const {
+ return schema_->Equals(other.schema_, false);
Review Comment:
Nit for code clarity
```suggestion
return schema_->Equals(other.schema_, /*check_metadata=*/ false);
```
##########
cpp/src/arrow/dataset/partition_test.cc:
##########
@@ -959,6 +1025,16 @@ TEST_F(TestPartitioning, Range) {
less_equal(field_ref("z"), literal(3.0)))}));
}
+TEST_F(TestPartitioning, RangePartitoningEquals) {
Review Comment:
Not sure this check is actually useful? `RangePartitioning` is just a test
class...
##########
cpp/src/arrow/dataset/partition_test.cc:
##########
@@ -222,6 +237,18 @@ TEST_F(TestPartitioning, FilenamePartitioning) {
equal(field_ref("beta"),
literal("foo"))));
}
+TEST_F(TestPartitioning, FilenamePartitioningEquals) {
+ auto part = std::make_shared<FilenamePartitioning>(
+ schema({field("alpha", int32()), field("beta", utf8())}));
+ auto other_part = std::make_shared<FilenamePartitioning>(
+ schema({field("sigma", int32()), field("beta", utf8())}));
+ auto another_part = std::make_shared<FilenamePartitioning>(
+ schema({field("sigma", int64()), field("beta", utf8())}));
+ EXPECT_TRUE(part->Equals(*part));
+ EXPECT_FALSE(part->Equals(*other_part));
+ EXPECT_FALSE(other_part->Equals(*another_part));
Review Comment:
Like above, introduce a distinct object that's equal to one of the others?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
