xudong963 opened a new pull request, #23330:
URL: https://github.com/apache/datafusion/pull/23330

   ## Which issue does this PR close?
   
   - Closes #23329.
   
   ## Rationale for this change
   
   `left`, `right`, and `substr` currently always return `Utf8View`, even when 
their first argument is `Utf8` or `LargeUtf8`. This makes them inconsistent 
with nearby string-producing functions such as `lower`, `upper`, `reverse`, 
`translate`, and `substr_index`, which preserve the first argument's string 
type.
   
   The unconditional `Utf8View` return type also means callers that 
intentionally avoid view types cannot keep `Utf8` flowing through these 
functions without explicit casts back to `Utf8`.
   
   This PR revisits the performance tradeoff from #21441 / #21442 and makes 
these functions follow the same input-driven return-type contract as the other 
string functions.
   
   ## What changes are included in this PR?
   
   - Change `left`, `right`, and `substr` return type inference to use the 
first argument's string type.
   - Return `StringArray` / `LargeStringArray` for `Utf8` / `LargeUtf8` inputs.
   - Keep the optimized `StringViewArray` path for `Utf8View` inputs.
   - Update unit tests and sqllogictests to cover `Utf8`, `LargeUtf8`, and 
`Utf8View` return types.
   
   ## Are these changes tested?
   
   Yes. I ran:
   
   ```shell
   cargo fmt --all
   cargo test -p datafusion-functions unicode::left::tests::test_functions
   cargo test -p datafusion-functions unicode::right::tests::test_functions
   cargo test -p datafusion-functions unicode::substr::tests::test_functions
   cargo test -p datafusion-functions 
unicode::substr::tests::test_sliced_string_array_array_args
   cargo test -p datafusion-sqllogictest --test sqllogictests -- 
string/string.slt --test-threads 1
   cargo test -p datafusion-sqllogictest --test sqllogictests -- 
string/string_view.slt string/large_string.slt string/dictionary_utf8.slt 
--test-threads 1
   cargo clippy -p datafusion-functions --lib --tests -- -D warnings
   ```
   
   ## Are there any user-facing changes?
   
   Yes. `left`, `right`, and `substr` now preserve the first argument's string 
type:
   
   - `Utf8 -> Utf8`
   - `LargeUtf8 -> LargeUtf8`
   - `Utf8View -> Utf8View`
   
   This may remove implicit downstream casts for users that expect `Utf8`, but 
it also means `Utf8` / `LargeUtf8` inputs no longer use the previous zero-copy 
`Utf8View` result representation.
   


-- 
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]

Reply via email to