4ktLuffy opened a new pull request, #24219:
URL: https://github.com/apache/datafusion/pull/24219

   ## Which issue does this PR close?
   
   - N/A — test coverage only, no linked issue.
   
   ## Rationale for this change
   
   `left` and `right` index by Unicode scalar value rather than byte offset, 
but currently have no sqllogictest coverage with multi-byte input. Searching 
the `.slt` corpus, `substr` and `strpos` have non-ASCII cases; `left` and 
`right` have none.
   
   `left_right_byte_length` takes an ASCII fast path introduced in 
apache/datafusion#23762 that derives the byte offset directly from the 
character count. That is sound only where byte length and character count 
coincide, and it is guarded by `string.is_ascii()`; non-ASCII input falls 
through to the `char_indices()` / `nth_back()` path.
   
   The two consumers of that offset behave differently if it is ever wrong. 
`general_left_right_array` slices via `&str` indexing, which validates char 
boundaries. `general_left_right_view` slices the underlying bytes without the 
`&str` boundary check, so an incorrect offset can produce an invalid UTF-8 view 
rather than the explicit boundary panic seen on `StringArray`.
   
   The `unicode` module has had several buffer-level performance changes 
recently — apache/datafusion#23762 (`left`/`right`), apache/datafusion#23586 
(`pad`), apache/datafusion#22171 (`translate`) — and none of them touched a 
sqllogictest file. These cases pin the character-indexing behaviour so that a 
future optimisation cannot alter it without a test failing.
   
   ## What changes are included in this PR?
   
   Eight cases in `functions.slt`, placed alongside the existing `left`/`right` 
tests:
   
   - 2-byte (`héllo`), 3-byte (`日本語`) and 4-byte (`hi🌏`) inputs
   - both positive and negative `n`, covering the `byte_offset_of_char` and 
`nth_back` branches
   - `Utf8View` variants so `general_left_right_view` is exercised in addition 
to `general_left_right_array`
   
   No source changes.
   
   ## Are these changes tested?
   
   They are tests, and I checked that they actually protect the invariant 
rather than just record current output: temporarily replacing the `is_ascii()` 
guard with `true`, so the ASCII fast path always applied, made the 
`StringArray` cases fail with
   
   ```
   end byte index 2 is not a char boundary; it is inside '日' (bytes 0..3 of 
string)
   ```
   
   and the `Utf8View` cases return an invalid UTF-8 result. With the guard 
restored, `cargo test -p datafusion-sqllogictest --test sqllogictests -- 
functions` passes.
   
   ## Are there any user-facing changes?
   
   No.
   


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