[GitHub] [arrow] milesgranger commented on a diff in pull request #14495: ARROW-17989: [C++][Python] Enable struct_field kernel to accept string field names

[GitBox <gi...@apache.org>]

milesgranger commented on code in PR #14495:
URL: https://github.com/apache/arrow/pull/14495#discussion_r1010063191


##########
cpp/src/arrow/compute/api_scalar.h:
##########
@@ -277,12 +277,16 @@ class ARROW_EXPORT SetLookupOptions : public FunctionOptions {
 class ARROW_EXPORT StructFieldOptions : public FunctionOptions {
  public:
   explicit StructFieldOptions(std::vector<int> indices);
+  explicit StructFieldOptions(std::initializer_list<int>);
+  explicit StructFieldOptions(FieldRef field_ref,
+                              std::vector<int> indices = std::vector<int>());
   StructFieldOptions();
   static constexpr char const kTypeName[] = "StructFieldOptions";
 
   /// The child indices to extract. For instance, to get the 2nd child
   /// of the 1st child of a struct or union, this would be {0, 1}.
   std::vector<int> indices;
+  FieldRef field_ref;

Review Comment:
   In the JIRA issue, [seems it should be kept](https://issues.apache.org/jira/browse/ARROW-17989?focusedCommentId=17617009&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17617009). I tried `ARROW_DEPRECATED` above the public `indices` member which it didn't like since that's one of the places we declare it in the first place. 
   
   
   > /home/milesg/Projects/arrow/cpp/src/arrow/compute/api_scalar.h:277:20: error: 'arrow::compute::StructFieldOptions::indices' is deprecated: Deprecated since 11.0.0. Use `field_ref` instead. [-Werror=deprecated-declarations]
     277 | class ARROW_EXPORT StructFieldOptions : public FunctionOptions {
         |                    ^~~~~~~~~~~~~~~~~~
   /home/milesg/Projects/arrow/cpp/src/arrow/compute/api_scalar.h:289:20: note: declared here
     289 |   std::vector<int> indices;
         |                    ^~~~~~~
   
   
   Is it better to add the deprecation on the constructors which use `indices`? I initially didn't think so as users can still subsequently be using `StructFieldOptions::indices` without any deprecation notice, and seems anyway constructing from indices ought to be alright.



-- 
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