From 1a30c074a61a45dc7984f5255ba33bcce8542124 Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Tue, 18 Oct 2022 17:51:58 +0200 Subject: [PATCH 1/3] ARROW-18087: [C++] RecordBatch::Equals should not ignore field names --- cpp/src/arrow/record_batch.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/record_batch.cc b/cpp/src/arrow/record_batch.cc index 9b93949d39f..28245c8f5de 100644 --- a/cpp/src/arrow/record_batch.cc +++ b/cpp/src/arrow/record_batch.cc @@ -231,10 +231,8 @@ bool RecordBatch::Equals(const RecordBatch& other, bool check_metadata) const { return false; } - if (check_metadata) { - if (!schema_->Equals(*other.schema(), /*check_metadata=*/true)) { - return false; - } + if (!schema_->Equals(*other.schema(), check_metadata)) { + return false; } for (int i = 0; i < num_columns(); ++i) { From 32f97a0c042fd3bb1d6f73395c28f5aa0aa4bd6f Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Wed, 19 Oct 2022 09:17:59 +0200 Subject: [PATCH 2/3] add test --- cpp/src/arrow/record_batch_test.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cpp/src/arrow/record_batch_test.cc b/cpp/src/arrow/record_batch_test.cc index 83371c94e49..ed2f3e552e7 100644 --- a/cpp/src/arrow/record_batch_test.cc +++ b/cpp/src/arrow/record_batch_test.cc @@ -47,6 +47,7 @@ TEST_F(TestRecordBatch, Equals) { auto f0 = field("f0", int32()); auto f1 = field("f1", uint8()); auto f2 = field("f2", int16()); + auto f2b = field("f2b", int16()); auto metadata = key_value_metadata({"foo"}, {"bar"}); @@ -54,6 +55,7 @@ TEST_F(TestRecordBatch, Equals) { auto schema = ::arrow::schema({f0, f1, f2}); auto schema2 = ::arrow::schema({f0, f1}); auto schema3 = ::arrow::schema({f0, f1, f2}, metadata); + auto schema4 = ::arrow::schema({f0, f1, f2b}); random::RandomArrayGenerator gen(42); @@ -65,11 +67,15 @@ TEST_F(TestRecordBatch, Equals) { auto b2 = RecordBatch::Make(schema3, length, {a0, a1, a2}); auto b3 = RecordBatch::Make(schema2, length, {a0, a1}); auto b4 = RecordBatch::Make(schema, length, {a0, a1, a1}); + auto b5 = RecordBatch::Make(schema4, length, {a0, a1, a2}); ASSERT_TRUE(b1->Equals(*b1)); ASSERT_FALSE(b1->Equals(*b3)); ASSERT_FALSE(b1->Equals(*b4)); + // Same values and types, but different field names + ASSERT_FALSE(b1->Equals(*b5)); + // Different metadata ASSERT_TRUE(b1->Equals(*b2)); ASSERT_FALSE(b1->Equals(*b2, /*check_metadata=*/true)); From 3067b871b4b6098ddf5e2b65cb91c70aba5df118 Mon Sep 17 00:00:00 2001 From: Joris Van den Bossche Date: Thu, 20 Oct 2022 09:37:24 +0200 Subject: [PATCH 3/3] remove unnecessary true --- cpp/src/arrow/dataset/test_util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/dataset/test_util.h b/cpp/src/arrow/dataset/test_util.h index 02464f0c38d..17065bfd7d2 100644 --- a/cpp/src/arrow/dataset/test_util.h +++ b/cpp/src/arrow/dataset/test_util.h @@ -157,7 +157,7 @@ class DatasetFixtureMixin : public ::testing::Test { std::shared_ptr lhs; ASSERT_OK(expected->ReadNext(&lhs)); EXPECT_NE(lhs, nullptr); - AssertBatchesEqual(*lhs, batch, true); + AssertBatchesEqual(*lhs, batch); } /// \brief Ensure that record batches found in reader are equals to the