Santoshkumarpuppala commented on PR #51237: URL: https://github.com/apache/arrow/pull/51237#issuecomment-5605926422
Checked the diff. All three parts land, and the kernel change is in the right place — after the regrow retry and the `res < 0` check, so the buffer is already sized for `res` codepoints and composition only ever shrinks it. No spare-byte concern, and the existing `UTF8EncodedLength`/`UTF8Encode` loop is untouched. It also matches the reference path exactly: `utf8proc_map` is `decompose` then `reencode`, and `reencode` is `normalize_utf32` followed by the UTF-8 encode. Taking the first half and keeping Arrow's own encoder gives the same result. Gating on `UTF8PROC_COMPOSE` leaves NFD/NFKD alone. The composed fixture is now `"\xc3\xa1"`, so the compose loop asserts real composition instead of comparing a string to itself. That was the part that would have let this regress invisibly. One small suggestion. The pyarrow case includes Hangul, which is the more interesting input — Hangul composition in utf8proc is algorithmic rather than table-driven, so it exercises a different path. The C++ suite still only covers the Latin pair. A Hangul pair there too would be worth having, since the C++ kernel is what the other bindings sit on. Happy to look again once CI is green. -- 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]
