Skip to content
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.

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

Fix (u)int8 implicit char conversion in stringstream #5445

Merged
merged 4 commits into from
Feb 14, 2025

Conversation

kounelisagis
Copy link
Member

This PR fixes the implicit character conversion that occurs when writing a (u)int8 value to a stringstream in the helper function std::string to_str(const T& value). By casting to (u)int32, we avoid this conversion.

This issue was discovered after encountering a UnicodeDecodeError when using operator<< in the TileDB-Py API on a TileDB Array schema that contained a (u)int8 attribute. Both a minimal TileDB-Py reproduction and the original issue involving TileDB-SOMA now work as expected.

[sc-61915]


TYPE: NO_HISTORY | BUG
DESC: Fix implicit character conversion in to_str by casting (u)int8 to (u)int32.

@kounelisagis kounelisagis changed the title Fix (u)int8 implicit char conversion in stringstream Fix (u)int8 implicit char conversion in stringstream Feb 11, 2025
Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

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

Thanks! Can you add a test in unit_parse_argument.cc? It doesn't exist currently.

Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

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

Thanks!

@kounelisagis kounelisagis merged commit d0348dc into main Feb 14, 2025
58 of 59 checks passed
@kounelisagis kounelisagis deleted the agis/fix-to_str-int8-uint8 branch February 14, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants