vibhatha commented on code in PR #13914:
URL: https://github.com/apache/arrow/pull/13914#discussion_r964710729
##########
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:
On second thoughts, it would be better to solve this one in another PR.
Because I am not quite sure if this would break R test cases.
--
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]
