icexelloss commented on code in PR #35364:
URL: https://github.com/apache/arrow/pull/35364#discussion_r1180823651
##########
cpp/src/arrow/engine/substrait/options.cc:
##########
@@ -205,12 +205,17 @@ class DefaultExtensionProvider : public
BaseExtensionProvider {
ARROW_ASSIGN_OR_RAISE(auto aggregate, internal::ParseAggregateMeasure(
agg_measure, ext_set,
conv_opts,
/*is_hash=*/!keys.empty(),
input_schema));
+ aggregate.name = aggregate.function;
Review Comment:
@rtpsw I am happy to merge the code that fixes the the segmented aggregation
bug (didn't pass the segmented key). However for the internal aggregation names
change we would need validate that this naming is consistent with other
aggregation codepath and/or refactor the naming logic to be shared (e.g., move
that logic into ParseAggregateMeasure). I would suggest revert that change for
now so we can move forward with the bug fix faster.
--
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]
