findepi commented on code in PR #17164: URL: https://github.com/apache/datafusion/pull/17164#discussion_r2273188493
########## docs/source/library-user-guide/upgrading.md: ########## @@ -24,13 +24,21 @@ **Note:** DataFusion `50.0.0` has not been released yet. The information provided in this section pertains to features and changes that have already been merged to the main branch and are awaiting release in this version. You can see the current [status of the `50.0.0 `release here](https://github.com/apache/datafusion/issues/16799) -### `AggregateUDFImpl` and `WindowUDFImpl` traits now require `PartialEq`, `Eq`, and `Hash` traits +### `ScalarUDFImpl`, `AggregateUDFImpl` and `WindowUDFImpl` traits now require `PartialEq`, `Eq`, and `Hash` traits -To address error-proneness of `AggregateUDFImpl::equals` and `WindowUDFImpl::equals` methods and -to make it easy to implement function equality correctly, the `equals` and `hash_value` methods have -been removed from `AggregateUDFImpl` and `WindowUDFImpl` traits. They are replaced the requirement to -implement the `PartialEq`, `Eq`, and `Hash` traits on any type implementing `AggregateUDFImpl` -or `WindowUDFImpl`. Please see [issue #16677] for more details. +To address error-proneness of `ScalarUDFImpl::equals`, `AggregateUDFImpl::equals`and +`WindowUDFImpl::equals` methods and to make it easy to implement function equality correctly, +the `equals` and `hash_value` methods have been removed from `ScalarUDFImpl`, `AggregateUDFImpl` +and `WindowUDFImpl` traits. They are replaced the requirement to implement the `PartialEq`, `Eq`, +and `Hash` traits on any type implementing `ScalarUDFImpl`, `AggregateUDFImpl` or `WindowUDFImpl`. +Please see [issue #16677] for more details. + +Most of the scalar functions are stateless and have a `signature` field. These can be migrated +using regular expressions + +- search for `\#\[derive\(Debug\)\](\n *(pub )?struct \w+ \{\n *signature\: Signature\,\n *\})`, +- replace with `#[derive(Debug, PartialEq, Eq, Hash)]$1`, +- review all the changes and make sure only function structs were changed. Review Comment: for reference, due to how DF functions are written, i used a different regex ``` \#\[derive\(Debug\)\](\n *(pub(\((super|crate)\))? )?struct \w+ \{\n( *name: String,\n)? *signature\: Signature\,\n( *aliases: Vec<String>,\n)? *\}) ``` - still not perfect, eg some functions derive Clone (just because they can) and the pattern doesn't cater for this - it's unlikely that downstream projects have fields such as `name` and `aliases` in their functions, so I didn't cater for these in the upgrade guide. -- 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
