You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/12/07 05:27:48 UTC
[GitHub] [arrow] 9prady9 opened a new pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
9prady9 opened a new pull request #11877:
URL: https://github.com/apache/arrow/pull/11877
Added the following member functions to `StructType` in line with `Schema` class.
- AddField
- RemoveField
- SetField
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11877:
URL: https://github.com/apache/arrow/pull/11877#issuecomment-993532835
Benchmark runs are scheduled for baseline = e401246fc0d500fb43e0bad328854e6c8772509b and contender = 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78. 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/33ad50f5d2fa453f87dd0e58ad68f77e...8c9c0872c0734ff1a5e12a53dc960f3e/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/0dd93c7822de4a8da696b7930b0893e7...f737953a4274472abd79023bed4d08b3/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/34f449026bca4cd1bfa0ec07284ee908...28e2f077512b4924ac58a487201ff151/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm closed pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
lidavidm closed pull request #11877:
URL: https://github.com/apache/arrow/pull/11877
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11877:
URL: https://github.com/apache/arrow/pull/11877#issuecomment-993532835
Benchmark runs are scheduled for baseline = e401246fc0d500fb43e0bad328854e6c8772509b and contender = 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78. 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/33ad50f5d2fa453f87dd0e58ad68f77e...8c9c0872c0734ff1a5e12a53dc960f3e/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/0dd93c7822de4a8da696b7930b0893e7...f737953a4274472abd79023bed4d08b3/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/34f449026bca4cd1bfa0ec07284ee908...28e2f077512b4924ac58a487201ff151/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11877:
URL: https://github.com/apache/arrow/pull/11877#issuecomment-993532835
Benchmark runs are scheduled for baseline = e401246fc0d500fb43e0bad328854e6c8772509b and contender = 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78. 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/33ad50f5d2fa453f87dd0e58ad68f77e...8c9c0872c0734ff1a5e12a53dc960f3e/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/0dd93c7822de4a8da696b7930b0893e7...f737953a4274472abd79023bed4d08b3/)
[Finished :arrow_down:0.35% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/34f449026bca4cd1bfa0ec07284ee908...28e2f077512b4924ac58a487201ff151/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11877:
URL: https://github.com/apache/arrow/pull/11877#issuecomment-987582009
https://issues.apache.org/jira/browse/ARROW-11424
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a change in pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] mbrobbel commented on a change in pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
mbrobbel commented on a change in pull request #11877:
URL: https://github.com/apache/arrow/pull/11877#discussion_r763731186
##########
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()) {
+ return Status::Invalid("Invalid column index to add field.");
Review comment:
```suggestion
return Status::Invalid("Invalid column index to set field.");
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #11877: ARROW-11424: [C++] StructType::{AddField,RemoveField,SetField} member functions
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11877:
URL: https://github.com/apache/arrow/pull/11877#issuecomment-993532835
Benchmark runs are scheduled for baseline = e401246fc0d500fb43e0bad328854e6c8772509b and contender = 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78. 5d44cb8d3bbffcee7e0ba1e0b6e7645049638d78 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/33ad50f5d2fa453f87dd0e58ad68f77e...8c9c0872c0734ff1a5e12a53dc960f3e/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/0dd93c7822de4a8da696b7930b0893e7...f737953a4274472abd79023bed4d08b3/)
[Finished :arrow_down:0.35% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/34f449026bca4cd1bfa0ec07284ee908...28e2f077512b4924ac58a487201ff151/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org