Details
-
Improvement
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
None
Description
Currently AggregateNodeOptions is:
class ARROW_EXPORT AggregateNodeOptions : public ExecNodeOptions {
public:
// aggregations which will be applied to the targetted fields
std::vector<internal::Aggregate> aggregates;
// fields to which aggregations will be applied
std::vector<FieldRef> targets;
// output field names for aggregations
std::vector<std::string> names;
// keys by which aggregations will be grouped
std::vector<FieldRef> keys;
};
It is not very obvious how aggregates and targets are related. My initial read of the comments led me to think that each aggregate would be applied to each target and you would end up with len(aggregates) * len(targets) output fields. In reality the aggregate at index i only applies to the target at index i. It would be simpler to add a FieldRef target to internal::Aggregate (and Aggregate should not be internal).
Alternatively, the entire internal::Aggregate could be replaced by a "call" arrow::compute::Expression
Attachments
Issue Links
- links to
