Update INITCAP scalar function to support Utf8View#11888
Conversation
| Ok(Arc::new(result) as ArrayRef) | ||
| } | ||
|
|
||
| fn initcap_utf8view<T: OffsetSizeTrait>(args: &[ArrayRef]) -> Result<ArrayRef> { |
There was a problem hiding this comment.
I am considering it might be a bit heavy to make this a macro, we wouldn't have a lot of initcap_* like this.
| let result = string_view_array | ||
| .iter() | ||
| .map(initcap_string) | ||
| .collect::<GenericStringArray<T>>(); |
There was a problem hiding this comment.
The return type is a StringArray instead of a StringViewArray, should I alter this behavior? In previous it was defined here
There was a problem hiding this comment.
The current utf8_to_str_type only return Utf8 or LargeUtf8. I think ideally we should support returning Utf8View. But since we are recreating the strings anyway, I'm not sure if StringView will help here.
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @xinlifoobar and @XiangpengHao
|
|
||
| fn initcap_string(string: Option<&str>) -> Option<String> { | ||
| string.map(|string: &str| { | ||
| let mut char_vector = Vec::<char>::new(); |
There was a problem hiding this comment.
I suspect you could make this faster by creating the vector once and then resetting on each loop -- like
let mut char_vector = Vec::<char>::new();
string.map(|string: &str| {
char_vector.clear();
...
}
|
Thanks again @xinlifoobar and @XiangpengHao |
3 participants
Which issue does this PR close?
Closes #11853 and part of #11790
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?