icexelloss commented on code in PR #35514:
URL: https://github.com/apache/arrow/pull/35514#discussion_r1213356646


##########
cpp/src/arrow/engine/substrait/extension_set.cc:
##########
@@ -979,23 +981,22 @@ ExtensionIdRegistry::SubstraitAggregateToArrow 
DecodeBasicAggregate(
             options = std::make_shared<compute::VarianceOptions>(ddof);
           }
         }
-        fixed_arrow_func += arrow_function_name;
 
-        ARROW_ASSIGN_OR_RAISE(compute::Expression arg, call.GetValueArg(0));
-        const FieldRef* arg_ref = arg.field_ref();
-        if (!arg_ref) {
-          return Status::Invalid("Expected an aggregate call ", call.id().uri, 
"#",
-                                 call.id().name, " to have a direct 
reference");
+        fixed_arrow_func += arrow_function_name;
+        std::vector<FieldRef> target;
+        for (int i = 0; i < call.size(); i++) {
+          ARROW_ASSIGN_OR_RAISE(compute::Expression arg, call.GetValueArg(i));
+          const FieldRef* arg_ref = arg.field_ref();

Review Comment:
   @felipecrv My understanding is with `const FieldRef*` then the move below 
has no effect / need to copy the data (because it is pointer to const FieldRef) 
and with `FieldRef*` the move operator can then actually move the object and 
not copying it?



-- 
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]

Reply via email to