jorgecarleitao commented on a change in pull request #7971:
URL: https://github.com/apache/arrow/pull/7971#discussion_r471894969
##########
File path: rust/datafusion/src/execution/context.rs
##########
@@ -509,33 +534,76 @@ impl SchemaProvider for ExecutionContextState {
}
fn get_function_meta(&self, name: &str) -> Option<Arc<FunctionMeta>> {
- self.scalar_functions
+ let scalar = self
+ .scalar_functions
.lock()
.expect("failed to lock mutex")
.get(name)
.map(|f| {
Arc::new(FunctionMeta::new(
name.to_owned(),
- f.args.clone(),
+ f.arg_types.clone(),
f.return_type.clone(),
FunctionType::Scalar,
))
+ });
+ // give priority to scalar functions
+ if scalar.is_some() {
+ return scalar;
+ }
+
+ self.aggregate_functions
+ .lock()
+ .expect("failed to lock mutex")
+ .get(name)
+ .map(|f| {
+ Arc::new(FunctionMeta::new(
+ name.to_owned(),
+ f.arg_types.clone(),
+ // this is wrong, but the actual type is overwritten by
the physical plan
+ // as aggregate functions have a variable type.
+ DataType::Float32,
Review comment:
@alamb this is one of the issues that the thread in the mailing list is
about: the return type in FunctionMeta is constant, but aggregates in our code
base have a variable type. As such, we can't represent our aggregates as
generic functions, which implies that we can't reconcile the logical and
physical plans.
[This
statement](https://github.com/apache/arrow/blob/2f98d1e65ef8821aecd9f716cfd6176f21315969/rust/datafusion/src/sql/planner.rs#L491)
is our current hack to this issue: we hard-code in the sql planner that the
return type is consistent with the physical expressions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
