edit: datum struct string type added utf8 check #1488
Conversation
dust1
changed the title
dust1
changed the title
|
I forgot, I'll try adding a few more unit tests later |
| @@ -765,6 +776,11 @@ impl Datum { | |||
| } | |||
| } | |||
|
|
|||
| 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.
| @@ -765,6 +776,11 @@ impl Datum { | |||
| } | |||
| } | |||
|
|
|||
| 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 |
Rationale
Close #1300
Detailed Changes
Check whether it is a utf8 string when inserting data
Test Plan
pass