Skip to content

Commit bf73f86

Browse files
committed
fix(cpp): Apply review feedback and fix CI failures
1 parent 12d9321 commit bf73f86

File tree

2 files changed

+11
-31
lines changed

2 files changed

+11
-31
lines changed

cpp/src/arrow/array/array_struct_test.cc

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -753,88 +753,70 @@ TEST(TestStructBuilderUnsafe, UnsafeAppend) {
753753
auto str_type = utf8();
754754
auto struct_type = struct_({field("int", int_type), field("str", str_type)});
755755
auto pool = default_memory_pool();
756-
std::shared_ptr<Array> final_array;
757756
auto int_builder = std::make_shared<Int32Builder>(pool);
758757
auto str_builder = std::make_shared<StringBuilder>(pool);
759758
StructBuilder builder(struct_type, pool, {int_builder, str_builder});
760-
761759
ASSERT_OK(builder.Reserve(2));
762760

763761
builder.UnsafeAppend();
764762
ASSERT_OK(int_builder->Append(1));
765763
ASSERT_OK(str_builder->Append("hello"));
766-
767764
builder.UnsafeAppend();
768765
ASSERT_OK(int_builder->Append(2));
769766
ASSERT_OK(str_builder->Append("arrow"));
770767

771768
ASSERT_OK_AND_ASSIGN(auto final_array, builder.Finish());
772-
ASSERT_EQ(2, final_array->length());
773-
ASSERT_EQ(0, final_array->null_count());
774-
775-
auto expected_json = R"([{"a": 1, "b": "hello"}, {"a": 2, "b": "arrow"}])";
776-
auto expected_array = ArrayFromJSON(struct_type, expected_json);
777-
AssertArraysEqual(*expected_array, *final_array);
769+
auto expected_array = ArrayFromJSON(
770+
struct_type, R"([{"int": 1, "str": "hello"}, {"int": 2, "str": "arrow"}])");
771+
AssertArraysEqual(*final_array, *expected_array);
778772
}
779773

780774
TEST(TestStructBuilderUnsafe, UnsafeAppendNull) {
781775
auto int_type = int32();
782776
auto str_type = utf8();
783-
auto struct_type = struct_({field("a", int_type), field("b", str_type)});
777+
auto struct_type = struct_({field("int", int_type), field("str", str_type)});
784778
auto pool = default_memory_pool();
785-
std::shared_ptr<Array> final_array;
786779
auto int_builder = std::make_shared<Int32Builder>(pool);
787780
auto str_builder = std::make_shared<StringBuilder>(pool);
788781
StructBuilder builder(struct_type, pool, {int_builder, str_builder});
789-
790782
ASSERT_OK(builder.Reserve(3));
791783

792784
builder.UnsafeAppend();
793785
ASSERT_OK(int_builder->Append(1));
794786
ASSERT_OK(str_builder->Append("hello"));
795-
796787
ASSERT_OK(builder.UnsafeAppendNull());
797-
798788
builder.UnsafeAppend();
799789
ASSERT_OK(int_builder->Append(2));
800790
ASSERT_OK(str_builder->Append("arrow"));
801791

802-
ASSERT_OK(builder.Finish(&final_array));
803-
ASSERT_EQ(3, final_array->length());
804-
ASSERT_EQ(1, final_array->null_count());
805-
ASSERT_TRUE(final_array->IsNull(1));
806-
auto expected_json = R"([{"a": 1, "b": "hello"}, null, {"a": 2, "b": "arrow"}])";
807-
808-
auto expected_array = ArrayFromJSON(struct_type, expected_json);
792+
ASSERT_OK_AND_ASSIGN(auto final_array, builder.Finish());
793+
auto expected_array = ArrayFromJSON(
794+
struct_type, R"([{"int": 1, "str": "hello"}, null, {"int": 2, "str": "arrow"}])");
809795
AssertArraysEqual(*final_array, *expected_array);
810796
}
811797

812798
TEST(TestStructBuilderUnsafe, UnsafeAppendNulls) {
813799
auto int_type = int32();
814800
auto str_type = utf8();
815-
auto struct_type = struct_({field("a", int_type), field("b", str_type)});
801+
auto struct_type = struct_({field("int", int_type), field("str", str_type)});
816802
auto pool = default_memory_pool();
817-
std::shared_ptr<Array> final_array;
818803
auto int_builder = std::make_shared<Int32Builder>(pool);
819804
auto str_builder = std::make_shared<StringBuilder>(pool);
820805
StructBuilder builder(struct_type, pool, {int_builder, str_builder});
821-
822806
ASSERT_OK(builder.Reserve(4));
823807

824808
builder.UnsafeAppend();
825809
ASSERT_OK(int_builder->Append(1));
826810
ASSERT_OK(str_builder->Append("hello"));
827-
828811
ASSERT_OK(builder.UnsafeAppendNulls(2));
829-
830812
builder.UnsafeAppend();
831813
ASSERT_OK(int_builder->Append(2));
832814
ASSERT_OK(str_builder->Append("arrow"));
833815

834-
ASSERT_OK(builder.Finish(&final_array));
835-
816+
ASSERT_OK_AND_ASSIGN(auto final_array, builder.Finish());
836817
auto expected_array = ArrayFromJSON(
837-
struct_type, R"([{"a": 1, "b": "hello"}, null, null, {"a": 2, "b": "arrow"}])");
818+
struct_type,
819+
R"([{"int": 1, "str": "hello"}, null, null, {"int": 2, "str": "arrow"}])");
838820
AssertArraysEqual(*final_array, *expected_array);
839821
}
840822

cpp/src/arrow/array/builder_nested.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,6 @@ class ARROW_EXPORT StructBuilder : public ArrayBuilder {
771771
return Status::OK();
772772
}
773773

774-
/// This method is "unsafe" because it does not update the null bitmap.
775774
/// It doesn't check the capacity. Caller is responsible for calling Reserve()
776775
/// beforehand.
777776
void UnsafeAppend(bool is_valid = true) { UnsafeAppendToBitmap(is_valid); }
@@ -785,7 +784,6 @@ class ARROW_EXPORT StructBuilder : public ArrayBuilder {
785784
return Append(false);
786785
}
787786

788-
/// This method is "unsafe" because it does not check for capacity.
789787
/// The caller is responsible for calling Reserve() beforehand to ensure
790788
/// there is enough space. This method will append nulls to all child
791789
/// builders to maintain a consistent state.

0 commit comments

Comments
 (0)