-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-37782: [C++] Add CanReferenceFieldsByNames
method to arrow::StructArray
#37823
Conversation
2. Add CanReferenceFieldsByNames to StructArray
StructArray::CanReferenceFieldsByName
I don't think the CI failures are related to my changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 5ca26e8. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them. |
…w::StructArray` (apache#37823) ### Rationale for this change `arrow::Schema` has a method called `CanReferenceFieldsByNames` which callers can use prior to calling `GetFieldByName`. It would be nice if `arrow::StructArray` also had `CanReferenceFieldsByNames` as a method. I also think it would be nice to add a `CanReferenceFieldByName` method that accepts a `std::string` instead of a `std::vector<std::string>` to `StructArray` and `Schema`. That way, users wouldn't have to create a `std::vector` containing one `std::string` when they just have one field name. ### What changes are included in this PR? 1. Added `CanReferenceFieldsByNames` method to `StructArray` 2. Added `CanReferenceFieldByName` method to `StructArray` 3. Added `CanReferenceFieldsByName` method to `Schema` ### Are these changes tested? Yes. I added unit tests for `CanReferenceFieldsByNames` and `CanReferenceFieldByName` to `array_struct_test.cc` and `type_test.cc`. ### Are there any user-facing changes? Yes. `CanReferenceFieldsByNames` and `CanReferenceFieldByName` can be called on a `StructArray`. Users can also call `CanReferenceFieldByName` on a `Schema`. * Closes: apache#37782 Authored-by: Sarah Gilmore <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…w::StructArray` (apache#37823) ### Rationale for this change `arrow::Schema` has a method called `CanReferenceFieldsByNames` which callers can use prior to calling `GetFieldByName`. It would be nice if `arrow::StructArray` also had `CanReferenceFieldsByNames` as a method. I also think it would be nice to add a `CanReferenceFieldByName` method that accepts a `std::string` instead of a `std::vector<std::string>` to `StructArray` and `Schema`. That way, users wouldn't have to create a `std::vector` containing one `std::string` when they just have one field name. ### What changes are included in this PR? 1. Added `CanReferenceFieldsByNames` method to `StructArray` 2. Added `CanReferenceFieldByName` method to `StructArray` 3. Added `CanReferenceFieldsByName` method to `Schema` ### Are these changes tested? Yes. I added unit tests for `CanReferenceFieldsByNames` and `CanReferenceFieldByName` to `array_struct_test.cc` and `type_test.cc`. ### Are there any user-facing changes? Yes. `CanReferenceFieldsByNames` and `CanReferenceFieldByName` can be called on a `StructArray`. Users can also call `CanReferenceFieldByName` on a `Schema`. * Closes: apache#37782 Authored-by: Sarah Gilmore <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…w::StructArray` (apache#37823) ### Rationale for this change `arrow::Schema` has a method called `CanReferenceFieldsByNames` which callers can use prior to calling `GetFieldByName`. It would be nice if `arrow::StructArray` also had `CanReferenceFieldsByNames` as a method. I also think it would be nice to add a `CanReferenceFieldByName` method that accepts a `std::string` instead of a `std::vector<std::string>` to `StructArray` and `Schema`. That way, users wouldn't have to create a `std::vector` containing one `std::string` when they just have one field name. ### What changes are included in this PR? 1. Added `CanReferenceFieldsByNames` method to `StructArray` 2. Added `CanReferenceFieldByName` method to `StructArray` 3. Added `CanReferenceFieldsByName` method to `Schema` ### Are these changes tested? Yes. I added unit tests for `CanReferenceFieldsByNames` and `CanReferenceFieldByName` to `array_struct_test.cc` and `type_test.cc`. ### Are there any user-facing changes? Yes. `CanReferenceFieldsByNames` and `CanReferenceFieldByName` can be called on a `StructArray`. Users can also call `CanReferenceFieldByName` on a `Schema`. * Closes: apache#37782 Authored-by: Sarah Gilmore <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…w::StructArray` (apache#37823) ### Rationale for this change `arrow::Schema` has a method called `CanReferenceFieldsByNames` which callers can use prior to calling `GetFieldByName`. It would be nice if `arrow::StructArray` also had `CanReferenceFieldsByNames` as a method. I also think it would be nice to add a `CanReferenceFieldByName` method that accepts a `std::string` instead of a `std::vector<std::string>` to `StructArray` and `Schema`. That way, users wouldn't have to create a `std::vector` containing one `std::string` when they just have one field name. ### What changes are included in this PR? 1. Added `CanReferenceFieldsByNames` method to `StructArray` 2. Added `CanReferenceFieldByName` method to `StructArray` 3. Added `CanReferenceFieldsByName` method to `Schema` ### Are these changes tested? Yes. I added unit tests for `CanReferenceFieldsByNames` and `CanReferenceFieldByName` to `array_struct_test.cc` and `type_test.cc`. ### Are there any user-facing changes? Yes. `CanReferenceFieldsByNames` and `CanReferenceFieldByName` can be called on a `StructArray`. Users can also call `CanReferenceFieldByName` on a `Schema`. * Closes: apache#37782 Authored-by: Sarah Gilmore <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
Rationale for this change
arrow::Schema
has a method calledCanReferenceFieldsByNames
which callers can use prior to callingGetFieldByName
. It would be nice ifarrow::StructArray
also hadCanReferenceFieldsByNames
as a method.I also think it would be nice to add a
CanReferenceFieldByName
method that accepts astd::string
instead of astd::vector<std::string>
toStructArray
andSchema
. That way, users wouldn't have to create astd::vector
containing onestd::string
when they just have one field name.What changes are included in this PR?
CanReferenceFieldsByNames
method toStructArray
CanReferenceFieldByName
method toStructArray
CanReferenceFieldsByName
method toSchema
Are these changes tested?
Yes. I added unit tests for
CanReferenceFieldsByNames
andCanReferenceFieldByName
toarray_struct_test.cc
andtype_test.cc
.Are there any user-facing changes?
Yes.
CanReferenceFieldsByNames
andCanReferenceFieldByName
can be called on aStructArray
. Users can also callCanReferenceFieldByName
on aSchema
.CanReferenceFieldsByNames
method toarrow::StructArray
#37782