westonpace commented on PR #35513: URL: https://github.com/apache/arrow/pull/35513#issuecomment-1544485845
> To double check, is the ask to use "urn:arrow:substrait_simple_extension_function" instead of "https://github.com/substrait-io/substrait/blob/main/extensions/functions_aggregate_generic.yaml"; for the function URI? Yes. Furthermore, if you use `urn:arrow:substrait_simple_extension_function` then you don't need a mapping at all. However, since this is an aggregate function, I suspect we will need to fix `arrow::engine::ExtensionIdRegistry::SubstraitAggregateToArrow` so that it behaves similarly to `arrow::engine::ExtensionIdRegistry::SubstraitCallToArrow`: ``` Result<SubstraitCallToArrow> GetSubstraitCallToArrow( Id substrait_function_id) const override { // NEED TO ADD THIS CASE TO GetSubstraitAggregateToArrow if (substrait_function_id.uri == kArrowSimpleExtensionFunctionsUri) { return kSimpleSubstraitToArrow; } auto maybe_converter = substrait_to_arrow_.find(substrait_function_id); if (maybe_converter == substrait_to_arrow_.end()) { if (parent_) { return parent_->GetSubstraitCallToArrow(substrait_function_id); } return Status::NotImplemented( "No conversion function exists to convert the Substrait function ", substrait_function_id.uri, "#", substrait_function_id.name, " to an Arrow call expression"); } return maybe_converter->second; } ``` -- 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]
