pitrou commented on code in PR #46126:
URL: https://github.com/apache/arrow/pull/46126#discussion_r2120474962
##########
cpp/src/arrow/array/builder_nested.h:
##########
@@ -672,6 +672,7 @@ class ARROW_EXPORT FixedSizeListBuilder : public
ArrayBuilder {
/// using the child array builder.
Status Append();
+ void UnsafeAppend();
Review Comment:
1. Can you please add docstrings for these new methods?
2. Ideally, the definition of the methods should be inline here, so that
they can be inlined in the caller.
##########
cpp/src/arrow/testing/fixed_width_test_util.cc:
##########
@@ -137,7 +137,7 @@ Status NestedListGenerator::AppendNestedList(ArrayBuilder*
nested_builder,
if (type->id() == Type::FIXED_SIZE_LIST) {
auto* fsl_builder = checked_cast<FixedSizeListBuilder*>(builder);
assert(list_size == checked_cast<FixedSizeListType&>(*type).list_size());
- RETURN_NOT_OK(fsl_builder->Append());
+ fsl_builder->UnsafeAppend();
Review Comment:
I don't think we should change this one. This is a testing utility, so we
value robustness over performance.
##########
cpp/src/arrow/array/builder_nested.cc:
##########
@@ -216,12 +220,22 @@ Status FixedSizeListBuilder::AppendNull() {
return value_builder_->AppendNulls(list_size_);
}
+void FixedSizeListBuilder::UnsafeAppendNull() {
+ UnsafeAppendToBitmap(false);
+ (void) value_builder_->AppendNulls(list_size_);
Review Comment:
We don't want to ignore error returns, so instead we should call a
hypothetical `UnsafeAppendNulls(list_size_)`.
If such a method doesn't exist, it should be added.
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
