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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org