Skip to content

GH-51225: [C++] utf8_normalize: compose for NFC and NFKC #51237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
singhpratech wants to merge 2 commits into
base: main
Choose a base branch
from
+39 −1
Open

GH-51225: [C++] utf8_normalize: compose for NFC and NFKC #51237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4735374
GH-51225: [C++] utf8_normalize: compose for NFC and NFKC
singhpratech Sep 8, 2026
87c8bfb
GH-51225: [C++] utf8_normalize: add a Hangul pair to the compose test
singhpratech Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion cpp/src/arrow/compute/kernels/scalar_string_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,7 @@ TYPED_TEST(TestStringKernels, Utf8Normalize) {

// decomposed: U+0061(LATIN SMALL LETTER A) + U+0301(COMBINING ACUTE ACCENT)
// composed: U+00E1(LATIN SMALL LETTER A WITH ACUTE)
const char* json_composed = "[\"foo\", \"á\"]";
const char* json_composed = "[\"foo\", \"\xc3\xa1\"]";

@raulcd raulcd Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not seem correct and does not match the comment above

@singhpratech singhpratech Sep 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The literal on main renders as á, but its bytes are 61 cc 81,
a followed by U+0301, the decomposed form, so it was the same string as json_decomposed on the
next line and the compose assertions below compared a value with itself. That is why the test passed
with the bug (Santoshkumarpuppala spotted it on the issue). To check:

git show main:cpp/src/arrow/compute/kernels/scalar_string_test.cc | sed -n 1248p | xxd
...  5c22 61cc 815c 22 ...

\xc3\xa1 is U+00E1 in UTF-8, which is what the comment describes. I used the escape so the two
fixtures are visibly different in the source, the way json_decomposed already is; happy to write the
literal á (composed, c3 a1) instead, or add the byte values to the comment, whichever you prefer.

const char* json_decomposed = "[\"foo\", \"a\xcc\x81\"]";
for (const auto& options : compose_options) {
this->CheckUnary("utf8_normalize", json_decomposed, this->type(), json_composed,
Expand All @@ -1260,6 +1260,25 @@ TYPED_TEST(TestStringKernels, Utf8Normalize) {
&options);
}

// Hangul composes algorithmically in utf8proc, not through the composition table.
// decomposed: U+1112(HANGUL CHOSEONG HIEUH) + U+1161(HANGUL JUNGSEONG A) +
// U+11AB(HANGUL JONGSEONG NIEUN)
// composed: U+D55C(HANGUL SYLLABLE HAN)
json_composed = "[\"\xed\x95\x9c\"]";
json_decomposed = "[\"\xe1\x84\x92\xe1\x85\xa1\xe1\x86\xab\"]";
for (const auto& options : compose_options) {
this->CheckUnary("utf8_normalize", json_decomposed, this->type(), json_composed,
&options);
this->CheckUnary("utf8_normalize", json_composed, this->type(), json_composed,
&options);
}
for (const auto& options : decompose_options) {
this->CheckUnary("utf8_normalize", json_composed, this->type(), json_decomposed,
&options);
this->CheckUnary("utf8_normalize", json_decomposed, this->type(), json_decomposed,
&options);
}

// canonical: U+00B2(Superscript Two)
// compatibility: "2"
const char* json_canonical = "[\"01\xc2\xb2!\"]";
Expand Down
10 changes: 10 additions & 0 deletions cpp/src/arrow/compute/kernels/scalar_string_utf8.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,16 @@ struct Utf8NormalizeBase {
if (res < 0) {
return Status::Invalid("Cannot normalize utf8 string: ", utf8proc_errmsg(res));
}
if (decompose_options_ & UTF8PROC_COMPOSE) {
// utf8proc_decompose() only decomposes; the canonical composition step for
// NFC and NFKC is done in-place by utf8proc_normalize_utf32().
res = utf8proc_normalize_utf32(
reinterpret_cast<utf8proc_int32_t*>(codepoints_.data()), res,
decompose_options_);
if (res < 0) {
return Status::Invalid("Cannot normalize utf8 string: ", utf8proc_errmsg(res));
}
}
return res;
}

Expand Down
9 changes: 9 additions & 0 deletions python/pyarrow/tests/test_compute.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3825,6 +3825,15 @@ def test_utf8_normalize():
assert pc.utf8_normalize(arr, form="NFKC") == pa.array(["0123"])
assert pc.utf8_normalize(arr, "NFD") == arr
assert pc.utf8_normalize(arr, "NFKD") == pa.array(["0123"])
# GH-51225: composing forms must compose, not only decompose
composed = pa.array(["\u00e9", "\ud55c", None])
decomposed = pa.array(["e\u0301", "\u1112\u1161\u11ab", None])
for form in ("NFC", "NFKC"):
assert pc.utf8_normalize(decomposed, form=form) == composed
assert pc.utf8_normalize(composed, form=form) == composed
for form in ("NFD", "NFKD"):
assert pc.utf8_normalize(composed, form=form) == decomposed
assert pc.utf8_normalize(decomposed, form=form) == decomposed
with pytest.raises(
ValueError,
match='"NFZ" is not a valid Unicode normalization form'):
Expand Down