Garamda commented on code in PR #13511:
URL: https://github.com/apache/datafusion/pull/13511#discussion_r1938230292
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -146,6 +146,10 @@ pub struct SessionState {
scalar_functions: HashMap<String, Arc<ScalarUDF>>,
/// Aggregate functions registered in the context
aggregate_functions: HashMap<String, Arc<AggregateUDF>>,
+ /// Ordered-set aggregate functions registered in the context
+ ///
+ /// Note : ordered_set_aggregate_functions are a subset of
aggregate_functions.
+ ordered_set_aggregate_functions: HashMap<String, Arc<AggregateUDF>>,
Review Comment:
There are certain cases when ordered set aggregate functions are needed to
be recognized.
1. [`pub(super) fn
sql_function_to_expr`](https://github.com/apache/datafusion/pull/13511/files#diff-e4760f867708d4ecd16dd4f38161a39d330c8d55f170e059aaba07317a07e42aR347-R354)
2. [`pub async fn
from_substrait_agg_func`](https://github.com/apache/datafusion/pull/13511/files#diff-474e53672159d74dae38992a914a74eba81b8350ebe161f11d755f06414ed7b4R1955-R1959)
There is certainly a way to avoid using sessions in those cases. However, I
am concerned that doing so might fragment the code.
**_If adding a new `ordered_set_aggregate_functions` field in the
`SessionState` doesn't seem like a good idea, please let me know._**
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
