Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 34 additions & 1 deletion cpp/src/arrow/compute/kernels/scalar_string_test.cc
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\"]";
Comment thread
pitrou marked this conversation as resolved.
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,39 @@ TYPED_TEST(TestStringKernels, Utf8Normalize) {
&options);
}

// 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);
}

// singleton: U+212B(ANGSTROM SIGN) decomposes to U+0041(LATIN CAPITAL LETTER A) +
// U+030A(COMBINING RING ABOVE), which composes to U+00C5(LATIN CAPITAL
// LETTER A WITH RING ABOVE), so the composed form differs from the input
const char* json_singleton = "[\"\xe2\x84\xab\"]";
json_composed = "[\"\xc3\x85\"]";
json_decomposed = "[\"A\xcc\x8a\"]";
for (const auto& options : compose_options) {
this->CheckUnary("utf8_normalize", json_singleton, this->type(), json_composed,
&options);
}
for (const auto& options : decompose_options) {
this->CheckUnary("utf8_normalize", json_singleton, 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
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().
Comment on lines +551 to +552

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.

Is it still useful that we call utf8proc_decompose first? Or should we just decode to UTF32 codepoints ourselves?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is still needed. utf8proc_normalize_utf32() composes pairs (through the composition
table, Hangul by arithmetic) and applies the newline and control-character options; it does no
decomposition and no reordering of combining marks, both of which happen in
utf8proc_decompose(). NFC and NFKC are the full decomposition followed by canonical
composition. Skipping utf8proc_decompose() would leave singletons alone (U+212B ANGSTROM SIGN
normalizes to U+00C5, there is nothing to compose), would not reorder marks (U+00E1 U+0323 must
become U+1EA1 U+0301), and would drop all of NFKC's compatibility mappings, which happen in the
decompose call. utf8proc's own utf8proc_map() is the same sequence, utf8proc_decompose() then
utf8proc_reencode(), which calls utf8proc_normalize_utf32() before encoding; the kernel does
that into its scratch buffer instead of the malloc in utf8proc_map(). I added the U+212B
singleton to the test, since it is the case that only passes with both steps.

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.

Oh, that's interesting indeed. Thanks for the example.

>>> s = "\u212B"
>>> unicodedata.normalize('NFC', s) == s
False
>>> unicodedata.normalize('NFKC', s) == s
False
>>> unicodedata.normalize('NFD', s) == s
False
>>> unicodedata.normalize('NFKD', s) == s
False

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