[
https://issues.apache.org/jira/browse/ARROW-16686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17544605#comment-17544605
]
Weston Pace commented on ARROW-16686:
-------------------------------------
This probably came from me. The caller can do this but I think it is
unintuitive for function options. Typically we receive options by value and
keep a copy of them for the duration of the ExecPlan.
There are some options that are more "infrastructure" (e.g. executor, memory
pool) where long term storage is inevitable. Taking a raw pointer is the
correct approach for these.
So that just leaves these middle ground types which are pointers only because
we need polymorphism. In this case I think it would be surprising for the user
to have to create and maintain the object for the duration of the exec plan.
As an example we can look at
https://github.com/apache/arrow/blob/50fbfa14aa612d638d54925ce9b5eaaaa7a9dd4b/r/src/compute-exec.cpp#L238
where we are currently doing the wrong thing.
The options object is created, and it is added to a "keep_alives" vector, but
the keep_alives vector itself is not actually kept alive. We only get away
with it at the moment because no aggregate function needs options. Rather than
keep this burden on the R developer it seems more intuitive to take in a
unique_ptr so we maintain the value semantics we use for other fields in
options types.
The "infrastructure" would still be an exception.
> [C++] Use unique_ptr with FunctionOptions
> -----------------------------------------
>
> Key: ARROW-16686
> URL: https://issues.apache.org/jira/browse/ARROW-16686
> Project: Apache Arrow
> Issue Type: Improvement
> Components: C++
> Reporter: Vibhatha Lakmal Abeykoon
> Assignee: Vibhatha Lakmal Abeykoon
> Priority: Major
>
>
> Update the usage of `FunctionOptions*` to `unique_ptr`
> {code:java}
> /// \brief Configure a grouped aggregation
> struct ARROW_EXPORT Aggregate {
> /// the name of the aggregation function
> std::string function;
> /// options for the aggregation function
> const FunctionOptions* options;
> }; {code}
>
>
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
