fix large string val allocation failure, #3600 #3601
Conversation
Large bitmap will need use StringVal to allocate large memory, which is large than MAX_INT. The overflow will cause serialization failure of bitmap.
|
Please add a unit test about large string val |
chaoyli
added
the
area/storage/in-memory
chaoyli
removed
the
area/storage/in-memory
| @@ -458,7 +458,8 @@ StringVal BitmapFunctions::bitmap_from_string(FunctionContext* ctx, const String | |||
| } | |||
|
|
|||
| std::vector<uint64_t> bits; | |||
| if (!SplitStringAndParse({(const char*)input.ptr, input.len}, ",", &safe_strtou64, &bits)) { | |||
| // TODO: I think StringPiece's len should also be uint64_t | |||
| if (!SplitStringAndParse({(const char*)input.ptr, (int)input.len}, ",", &safe_strtou64, &bits)) { | |||
There was a problem hiding this comment.
Why not change it to int64 too?
There was a problem hiding this comment.
because it relate to another struct StringPiece, which is defined in gutil, and to much code reference it. I think whether to change StringPiece need to be discussed and if true, can be done in another pr.
morningman
added
area/udf
@kangpinghuang Hi. you mean the bitmap size is larger than 2G? are you sure which is reasonable？ |
yes, There is a user who use the bitmap to do the count distinct. In his situation, the dimension is about 30billion, the bitmap I see is about 3G. |
I See, Thanks. Have you tested this PR in prod env a long time? I think simply change the StringVal size from int to uint64_t maybe have a lot of issues: Such as, How do we handle network transfer？Mempool could handle 3G huge memory allocate well? the StringVal and Slice exchange could work well? |
|
This pr has some error. The correct pr is here #3724 |
Large bitmap will need use StringVal to allocate large memory, which is large than MAX_INT.
The overflow will cause serialization failure of bitmap.