wesm commented on a change in pull request #7598:
URL: https://github.com/apache/arrow/pull/7598#discussion_r449156988
##########
File path: cpp/src/arrow/array/array_struct_test.cc
##########
@@ -256,16 +256,6 @@ TEST_F(TestStructBuilder, TestAppendNull) {
ASSERT_OK(builder_->AppendNull());
ASSERT_EQ(2, static_cast<int>(builder_->num_fields()));
- ListBuilder* list_vb =
checked_cast<ListBuilder*>(builder_->field_builder(0));
Review comment:
I mentioned this in the PR description. IMHO it makes sense for both the
union and struct builders to take responsibility for invoking
AppendNull/AppendNulls on their children rather than UnionBuilder do it and
StructBuilder not do it. There were several places where the children were
being manually appended to and the new way is definitely cleaner. There's some
chance that there is third party code that uses StructBuilder but I'm not sure
what to do about this -- I presume such users would be responsible enough to
write unit tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
