From 5fb5d9c2ba8aeff2de829c7759c11e5a582bb265 Mon Sep 17 00:00:00 2001 From: Hyukjin Kwon Date: Fri, 2 Jan 2026 12:13:02 +0900 Subject: [PATCH 1/3] [C++] Add tests for DayTimeIntervalBuilder --- cpp/src/arrow/array/array_test.cc | 65 ++++++++++++++++++++++++++++++ cpp/src/arrow/array/builder_time.h | 2 - 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 3fa4f16e85b..b74132ef369 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -4237,4 +4237,69 @@ TEST_F(TestHalfFloatBuilder, TestBulkAppend) { } } +class TestDayTimeIntervalBuilder : public ::testing::Test { + public: + void VerifyValue(const DayTimeIntervalBuilder& builder, int64_t index, + DayTimeIntervalType::DayMilliseconds expected) { + ASSERT_EQ(builder.GetValue(index), expected); + ASSERT_EQ(builder[index], expected); + } +}; + +TEST_F(TestDayTimeIntervalBuilder, TestAppend) { + DayTimeIntervalBuilder builder; + DayTimeIntervalType::DayMilliseconds value1{1, 100}; + DayTimeIntervalType::DayMilliseconds value2{3, 200}; + DayTimeIntervalType::DayMilliseconds value3{5, 300}; + + ASSERT_OK(builder.Append(value1)); + ASSERT_OK(builder.Append(value2)); + ASSERT_OK(builder.AppendNull()); + ASSERT_OK(builder.Reserve(3)); + builder.UnsafeAppend(value3); + + VerifyValue(builder, 0, value1); + VerifyValue(builder, 1, value2); + VerifyValue(builder, 3, value3); + + ASSERT_OK_AND_ASSIGN(auto array, builder.Finish()); + ASSERT_TRUE(array->IsNull(2)); +} + +TEST_F(TestDayTimeIntervalBuilder, TestBulkAppend) { + DayTimeIntervalBuilder builder; + std::vector values{{1, 100}, {3, 200}, {5, 300}}; + std::vector is_valid{true, false, true}; + std::vector is_valid_bytes{1, 0, 1}; + + ASSERT_OK(builder.AppendValues(values)); + ASSERT_OK(builder.AppendValues(values, is_valid)); + ASSERT_OK(builder.AppendValues(values.data(), values.size(), is_valid_bytes.data())); + + ASSERT_OK_AND_ASSIGN(auto array, builder.Finish()); + ASSERT_OK(array->ValidateFull()); + ASSERT_EQ(array->null_count(), 2); + ASSERT_EQ(array->length(), 9); + + const auto& day_time_array = checked_cast(*array); + ASSERT_EQ(day_time_array.GetValue(0), values[0]); + ASSERT_TRUE(day_time_array.IsNull(4)); + ASSERT_TRUE(day_time_array.IsNull(7)); + ASSERT_EQ(day_time_array.GetValue(2), values[2]); +} + +TEST_F(TestDayTimeIntervalBuilder, TestConstructors) { + DayTimeIntervalBuilder builder1; + ASSERT_EQ(builder1.type()->id(), Type::INTERVAL_DAY_TIME); + + MemoryPool* pool = default_memory_pool(); + DayTimeIntervalBuilder builder2(pool); + ASSERT_EQ(builder2.type()->id(), Type::INTERVAL_DAY_TIME); + + auto type = day_time_interval(); + DayTimeIntervalBuilder builder3(type, pool); + ASSERT_EQ(builder3.type()->id(), Type::INTERVAL_DAY_TIME); + ASSERT_TRUE(builder3.type()->Equals(type)); +} + } // namespace arrow diff --git a/cpp/src/arrow/array/builder_time.h b/cpp/src/arrow/array/builder_time.h index da29ae3124b..b471e9621cd 100644 --- a/cpp/src/arrow/array/builder_time.h +++ b/cpp/src/arrow/array/builder_time.h @@ -30,8 +30,6 @@ namespace arrow { /// /// @{ -// TODO(ARROW-7938): this class is untested - class ARROW_EXPORT DayTimeIntervalBuilder : public NumericBuilder { public: using DayMilliseconds = DayTimeIntervalType::DayMilliseconds; From fb809d00fdf026cd1c56623ba83466df60ea9890 Mon Sep 17 00:00:00 2001 From: Hyukjin Kwon Date: Wed, 7 Jan 2026 10:35:37 +0900 Subject: [PATCH 2/3] Review comments --- cpp/src/arrow/array/array_test.cc | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index b74132ef369..be58fabe569 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -4255,6 +4255,7 @@ TEST_F(TestDayTimeIntervalBuilder, TestAppend) { ASSERT_OK(builder.Append(value1)); ASSERT_OK(builder.Append(value2)); ASSERT_OK(builder.AppendNull()); + ASSERT_EQ(1, builder.null_count()); // Verify null count in builder ASSERT_OK(builder.Reserve(3)); builder.UnsafeAppend(value3); @@ -4263,7 +4264,19 @@ TEST_F(TestDayTimeIntervalBuilder, TestAppend) { VerifyValue(builder, 3, value3); ASSERT_OK_AND_ASSIGN(auto array, builder.Finish()); - ASSERT_TRUE(array->IsNull(2)); + const auto& day_time_array = checked_cast(*array); + + // Verify null value + ASSERT_TRUE(day_time_array.IsNull(2)); + ASSERT_EQ(1, day_time_array.null_count()); + + // Verify non-null values in the array + ASSERT_FALSE(day_time_array.IsNull(0)); + ASSERT_EQ(day_time_array.GetValue(0), value1); + ASSERT_FALSE(day_time_array.IsNull(1)); + ASSERT_EQ(day_time_array.GetValue(1), value2); + ASSERT_FALSE(day_time_array.IsNull(3)); + ASSERT_EQ(day_time_array.GetValue(3), value3); } TEST_F(TestDayTimeIntervalBuilder, TestBulkAppend) { From a3cb3d3b73cc43d88a72984cd87f07f070fa23a8 Mon Sep 17 00:00:00 2001 From: Hyukjin Kwon Date: Fri, 9 Jan 2026 10:19:09 +0900 Subject: [PATCH 3/3] Update cpp/src/arrow/array/array_test.cc Co-authored-by: Sutou Kouhei --- cpp/src/arrow/array/array_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index be58fabe569..dc82488f6a3 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -4305,7 +4305,7 @@ TEST_F(TestDayTimeIntervalBuilder, TestConstructors) { DayTimeIntervalBuilder builder1; ASSERT_EQ(builder1.type()->id(), Type::INTERVAL_DAY_TIME); - MemoryPool* pool = default_memory_pool(); + auto pool = default_memory_pool(); DayTimeIntervalBuilder builder2(pool); ASSERT_EQ(builder2.type()->id(), Type::INTERVAL_DAY_TIME);