edit: datum struct string type added utf8 check#1488
edit: datum struct string type added utf8 check#1488dust1 wants to merge 2 commits intoapache:mainfrom
Conversation
dust1
changed the title
dust1
changed the title
|
I forgot, I'll try adding a few more unit tests later |
| } | ||
| } | ||
|
|
||
| fn valid_is_utf8(s: &str) -> Result<()> { |
There was a problem hiding this comment.
This check may be expensive for long string, better to add an option to decide whether to do this check.
| } | ||
|
|
||
| fn valid_is_utf8(s: &str) -> Result<()> { | ||
| from_utf8(s.as_bytes()).context(InvalidStringEncoding { msg: s })?; |
There was a problem hiding this comment.
IMO this function should return bool, not a result.
|
I checked the rust official documentation, and for the way to build Datum objects from String in datum.rs, rust guarantees that String is a utf8 string, which I might need to modify elsewhere. 😢 |
Yes, I grep the code and find several place contains As for debugging this issue, you can construct a GBK string using SDK, and trace why there is no error for it. |
Ok, I'll try |
|
The |
2 participants
Rationale
Close #1300
Detailed Changes
Check whether it is a utf8 string when inserting data
Test Plan
pass