lyne7-sc commented on code in PR #23930:
URL: https://github.com/apache/datafusion/pull/23930#discussion_r3672115767
##########
datafusion/functions/src/unicode/character_length.rs:
##########
@@ -59,11 +62,16 @@ impl Default for CharacterLengthFunc {
impl CharacterLengthFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::uniform(
- 1,
- vec![Utf8, LargeUtf8, Utf8View],
Review Comment:
yeah, this narrows the set of accepted inputs, but some of the implicit
coercions allowed by the previous signatures don’t seem very intuitive to me.
For example, `reverse([1, 2, 3])` returns "]3 ,2 ,1[", and `reverse(true)`
returns "eurt".
I checked postgresql and duckdb, and both require an explicit string cast
here. This behavior seems more reasonable to me.
```
select reverse(true);
ERROR: function reverse(boolean) does not exist
HINT: You might need to add explicit type casts.
select reverse(ARRAY[1,2,3]);
ERROR: function reverse(integer[]) does not exist
HINT: You might need to add explicit type casts.
```
so I’m not sure whether we should keep the existing implicit conversions or
require an explicit cast?
--
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]
