Upper and Lower do not handle invalid UTF-8

[Riolku]

If we call Upper() or Lower() with binary data (anything non-UTF-8), our application simply hangs.

This is because we don't check the return value of utf8proc_codepoint, which is -1 when UTF-8 is invalid. Our size goes unmodified, and we loop forever. Worse, the assert we have simply does nothing, since (as far as I can tell), it doesn't check that the codepoint is non-negative.

Reproduce with:

LOAD FROM "test.csv" RETURN upper(column0);

Where you can create test.csv by running:

python -c "open('test.csv', 'wb').write(b'\xff')"

[Riolku]

#2156 adds a hack to make sure we don't hang in this case, but we should fix this properly by ensuring STRING types are always valid UTF-8, which is a much bigger task. This is compliant with what postgres does.

We should still keep this check just in case, it's always better to throw an unexpected error than crash.

[acquamarin]

Solved in #2208

[Riolku]

After a bunch of investigation, here's the state of things:

• Copy from CSV is broken and could be fixed by Throw copy exception on invalid utf8 string #2208.
• Copy from parquet doesn't support strings.
• Copy from NPY also doesn't support strings.
• ANTLR4 throws on invalid UTF-8, so we can't insert bad strings via any of the APIs (right now).

This implies that #2208 would fix the issue. However, I would instead propose that we treat all data in CSV and other files as BLOB, and then call an appropriate cast function. We already have DECODE for the latter task. It'd be good to make UTF-8 checks centralized, instead of duplicating and inevitably forgetting them here and there in the codebase.

Finally, our current LIST_SLICE function doesn't respect UTF-8, and thus means that this issue is not fixed by #2208.

kuzu> MATCH (u:user) set u.name = "成績評価の甘い授業が高く評価";
-
(0 tuples)
Time: 0.76ms (compiling), 3.70ms (executing)
kuzu> MATCH (u:user) RETURN upper(u.name[:4]);
-------------------------------------------
| UPPER(LIST_SLICE(u.name,TO_INT64(0),4)) |
-------------------------------------------
| 成侀                                    |
-------------------------------------------
(1 tuple)

[acquamarin]

However, I would instead propose that we treat all data in CSV and other files as BLOB, and then call an appropriate cast function.
Does this mean we should implement casting functions from blob to all other types that we support?

[Riolku]

I don't know, because that seems silly. It definitely makes sense to implement BLOB -> String. I suppose BLOB -> INT64 only makes sense internally. We don't need a UTF-8 check for those cases, anything non-ascii isn't a valid integer anyway.

On second thought I'm no longer sure what to do. Maybe having per-filetype checks is reasonable since they are so different (and we don't want to pay the price of a copy).

[semihsalihoglu-uw]

My 2 cents is to have per filetype checks but I'll leave the solution to the implementer. I suggest we prioritize this. Calling upper and lower functions on UTF-8 could be a common scenario.

[andyfengHKU]

Test for all string functions on utf-8 is done in #3287