vibhatha commented on code in PR #13914:
URL: https://github.com/apache/arrow/pull/13914#discussion_r962926872
##########
cpp/src/arrow/engine/substrait/relation_internal.cc:
##########
@@ -399,17 +506,45 @@ Result<DeclarationInfo> FromProto(const substrait::Rel&
rel, const ExtensionSet&
ExtensionIdRegistry::SubstraitAggregateToArrow converter,
ext_set.registry()->GetSubstraitAggregateToArrow(aggregate_call.id()));
ARROW_ASSIGN_OR_RAISE(compute::Aggregate arrow_agg,
converter(aggregate_call));
+
+ // find aggregate field ids from schema
+ const auto field_ref = arrow_agg.target;
+ ARROW_ASSIGN_OR_RAISE(auto match, field_ref.FindOne(*input_schema));
+ agg_src_field_ids[measure_id] = match[0];
+
aggregates.push_back(std::move(arrow_agg));
} else {
return Status::Invalid("substrait::AggregateFunction not provided");
}
}
+ FieldVector output_fields;
+ output_fields.reserve(key_field_ids.size() + agg_src_field_ids.size());
+ // extract aggregate fields to output schema
+ for (int id = 0; id < static_cast<int>(agg_src_field_ids.size()); id++) {
+ output_fields.emplace_back(input_schema->field(agg_src_field_ids[id]));
+ }
+ // extract key fields to output schema
+ for (int id = 0; id < static_cast<int>(key_field_ids.size()); id++) {
+ output_fields.emplace_back(input_schema->field(key_field_ids[id]));
+ }
Review Comment:
Yes, the requirement in Acero may be static here.
We can use the project to swap things around and document it properly.
Probably we can do it in this PR as well.
cc @westonpace
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
