davidm-db commented on code in PR #46206:
URL: https://github.com/apache/spark/pull/46206#discussion_r1579027113
##########
common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationSupport.java:
##########
Review Comment:
Per my understanding, logic here is the same as in `Contains`, `StartsWith`,
etc.
As I explained yesterday, `StringTrim*` functions work in specific way:
- If `trimString` param is not provided, then only spaces are trimmed.
- If `trimString` param is provided, then there are two options:
- `trimString = null` - in this case `null` is returned always.
- otherwise, characters from `trimString` are trimmed.
For this reason, in the original implementation, the methods were separated
to the methods that have `trimString` param and to the methods that don't.
I only kept this reasoning for the sake of not changing default/expected
behavior.
If you still think that we need to change to having only one method of each
(`exec`, `genCode`, `execBinary` and `execICU`), let's sync offline to figure
out how best to do this and have original logic perserved.
--
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]
