vibhatha commented on code in PR #13914:
URL: https://github.com/apache/arrow/pull/13914#discussion_r962444320
##########
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:
@jvanstraten I also noticed this, but I forget to leave a comment about it.
This is probably a separate JIRA because of the order used in the
`aggregate_node.cc`[1]. Please refer to the comment in this line and the two
loops after that. The aggregate fields appened first and then the key fields.
One thing we can do is swap the response here.
cc @westonpace
[1].
https://github.com/apache/arrow/blob/50a7d15dfb4cbc4dd449ff2bb3ba2b1cde62a3ab/cpp/src/arrow/compute/exec/aggregate_node.cc#L345
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
