lidavidm commented on a change in pull request #11877:
URL: https://github.com/apache/arrow/pull/11877#discussion_r768100125
##########
File path: cpp/src/arrow/type.h
##########
@@ -996,6 +996,12 @@ class ARROW_EXPORT StructType : public NestedType {
/// \brief Return the indices of all fields having this name in sorted order
std::vector<int> GetAllFieldIndices(const std::string& name) const;
+ Result<std::shared_ptr<StructType>> AddField(int i,
+ const std::shared_ptr<Field>&
field) const;
+ Result<std::shared_ptr<StructType>> RemoveField(int i) const;
+ Result<std::shared_ptr<StructType>> SetField(int i,
+ const std::shared_ptr<Field>&
field) const;
Review comment:
nit: can we add (brief) docstrings for each of these?
##########
File path: cpp/src/arrow/type.cc
##########
@@ -787,6 +787,30 @@ std::vector<std::shared_ptr<Field>>
StructType::GetAllFieldsByName(
return result;
}
+Result<std::shared_ptr<StructType>> StructType::AddField(
+ int i, const std::shared_ptr<Field>& field) const {
+ if (i < 0 || i > this->num_fields()) {
+ return Status::Invalid("Invalid column index to add field.");
+ }
+ return std::make_shared<StructType>(internal::AddVectorElement(children_, i,
field));
+}
+
+Result<std::shared_ptr<StructType>> StructType::RemoveField(int i) const {
+ if (i < 0 || i >= this->num_fields()) {
+ return Status::Invalid("Invalid column index to remove field.");
+ }
+ return std::make_shared<StructType>(internal::DeleteVectorElement(children_,
i));
+}
+
+Result<std::shared_ptr<StructType>> StructType::SetField(
+ int i, const std::shared_ptr<Field>& field) const {
+ if (i < 0 || i > this->num_fields()) {
Review comment:
```suggestion
if (i < 0 || i >= this->num_fields()) {
```
##########
File path: cpp/src/arrow/type_test.cc
##########
@@ -1599,6 +1599,28 @@ TEST(TestStructType, TestFieldsDifferOnlyInMetadata) {
ASSERT_NE(s0.metadata_fingerprint(), s1.metadata_fingerprint());
}
+TEST(TestStructType, FieldModifierMethods) {
+ auto f0 = field("f0", int32());
+ auto f1 = field("f1", utf8());
+
+ std::vector<std::shared_ptr<Field>> fields = {f0, f1};
+
+ StructType struct_type(fields);
+
+ ASSERT_OK_AND_ASSIGN(auto new_struct, struct_type.AddField(1, field("f2",
int8())));
+ ASSERT_EQ(3, new_struct->num_fields());
+ ASSERT_EQ(1, new_struct->GetFieldIndex("f2"));
+
+ ASSERT_OK_AND_ASSIGN(new_struct, new_struct->RemoveField(1));
+ ASSERT_EQ(2, new_struct->num_fields());
+ ASSERT_EQ(-1, new_struct->GetFieldIndex("f2")); // No f2 after removal
+
+ ASSERT_OK_AND_ASSIGN(new_struct, new_struct->SetField(1, field("f2",
int8())));
+ ASSERT_EQ(2, new_struct->num_fields());
+ ASSERT_EQ(1, new_struct->GetFieldIndex("f2"));
+ ASSERT_EQ(int8(), new_struct->GetFieldByName("f2")->type());
Review comment:
Can we also test that we get an error with out-of-bounds indices?
(EXPECT_RAISES_WITH_MESSAGE_THAT or ASSERT_RAISES)
--
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
