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 large string val allocation failure, #3600 #3601

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion be/src/exprs/anyval_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ class AnyValUtil {
if (type.type == FunctionContext::TYPE_VARCHAR
|| type.type == FunctionContext::TYPE_CHAR) {
DCHECK(type.len >= 0);
val->len = std::min(val->len, type.len);
val->len = std::min(val->len, (uint64_t)type.len);
}
}

Expand Down
3 changes: 2 additions & 1 deletion be/src/exprs/bitmap_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not change it to int64 too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

return StringVal::null();
}

Expand Down
4 changes: 2 additions & 2 deletions be/src/runtime/free_pool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class FreePool {
virtual ~FreePool() {}

// Allocates a buffer of size.
uint8_t* allocate(int size) {
uint8_t* allocate(uint64_t size) {
// This is the typical malloc behavior. NULL is reserved for failures.
if (size == 0) {
return reinterpret_cast<uint8_t*>(0x1);
Expand Down Expand Up @@ -98,7 +98,7 @@ class FreePool {
// Returns an allocation that is at least 'size'. If the current allocation
// backing 'ptr' is big enough, 'ptr' is returned. Otherwise a new one is
// made and the contents of ptr are copied into it.
uint8_t* reallocate(uint8_t* ptr, int size) {
uint8_t* reallocate(uint8_t* ptr, uint64_t size) {
if (ptr == NULL || reinterpret_cast<int64_t>(ptr) == 0x1) {
return allocate(size);
}
Expand Down
10 changes: 5 additions & 5 deletions be/src/udf/udf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ void FunctionContextImpl::close() {
_closed = true;
}

uint8_t* FunctionContextImpl::allocate_local(int byte_size) {
uint8_t* FunctionContextImpl::allocate_local(uint64_t byte_size) {
uint8_t* buffer = _pool->allocate(byte_size);
_local_allocations.push_back(buffer);
return buffer;
Expand Down Expand Up @@ -365,12 +365,12 @@ bool FunctionContext::add_warning(const char* warning_msg) {
}
}

StringVal::StringVal(FunctionContext* context, int len) :
len(len),
StringVal::StringVal(FunctionContext* context, uint64_t len) :
len(len),
ptr(context->impl()->allocate_local(len)) {
}

bool StringVal::resize(FunctionContext* ctx, int new_len) {
bool StringVal::resize(FunctionContext* ctx, uint64_t new_len) {
if (new_len <= len) {
len = new_len;
return true;
Expand Down Expand Up @@ -398,7 +398,7 @@ StringVal StringVal::copy_from(FunctionContext* ctx, const uint8_t* buf, size_t
return result;
}

StringVal StringVal::create_temp_string_val(FunctionContext* ctx, int len) {
StringVal StringVal::create_temp_string_val(FunctionContext* ctx, uint64_t len) {
ctx->impl()->string_result().resize(len);
return StringVal((uint8_t*)ctx->impl()->string_result().c_str(), len);
}
Expand Down
10 changes: 5 additions & 5 deletions be/src/udf/udf.h
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ struct DateTimeVal : public AnyVal {
struct StringVal : public AnyVal {
static const int MAX_LENGTH = (1 << 30);

int len;
uint64_t len;
uint8_t* ptr;

// Construct a StringVal from ptr/len. Note: this does not make a copy of ptr
Expand All @@ -609,7 +609,7 @@ struct StringVal : public AnyVal {

// Construct a StringVal from ptr/len. Note: this does not make a copy of ptr
// so the buffer must exist as long as this StringVal does.
StringVal(uint8_t* ptr, int len) : len(len), ptr(ptr) {}
StringVal(uint8_t* ptr, uint64_t len) : len(len), ptr(ptr) {}

// Construct a StringVal from NULL-terminated c-string. Note: this does not make a
// copy of ptr so the underlying string must exist as long as this StringVal does.
Expand All @@ -624,12 +624,12 @@ struct StringVal : public AnyVal {
// Creates a StringVal, allocating a new buffer with 'len'. This should
// be used to return StringVal objects in UDF/UDAs that need to allocate new
// string memory.
StringVal(FunctionContext* context, int len);
StringVal(FunctionContext* context, uint64_t len);

// Creates a StringVal, which memory is avaliable when this funciont context is used next time
static StringVal create_temp_string_val(FunctionContext* ctx, int len);
static StringVal create_temp_string_val(FunctionContext* ctx, uint64_t len);

bool resize(FunctionContext* context, int len);
bool resize(FunctionContext* context, uint64_t len);

bool operator==(const StringVal& other) const {
if (is_null != other.is_null) {
Expand Down
2 changes: 1 addition & 1 deletion be/src/udf/udf_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class FunctionContextImpl {
// This is used where the lifetime of the allocation is clear.
// For UDFs, the allocations can be freed at the row level.
// TODO: free them at the batch level and save some copies?
uint8_t* allocate_local(int byte_size);
uint8_t* allocate_local(uint64_t byte_size);

// Frees all allocations returned by AllocateLocal().
void free_local_allocations();
Expand Down