diff --git a/be/src/exprs/aggregate-functions-ir.cc b/be/src/exprs/aggregate-functions-ir.cc index 79d7ac791..853661c4f 100644 --- a/be/src/exprs/aggregate-functions-ir.cc +++ b/be/src/exprs/aggregate-functions-ir.cc @@ -1388,6 +1388,7 @@ void AggregateFunctions::LastValUpdate(FunctionContext* ctx, const StringVal& sr } else { new_ptr = ctx->Reallocate(dst->ptr, src.len); } + // Note that a zero-length string is not the same as StringVal::null(). RETURN_IF_NULL(ctx, new_ptr); dst->ptr = new_ptr; memcpy(dst->ptr, src.ptr, src.len); diff --git a/be/src/runtime/free-pool.h b/be/src/runtime/free-pool.h index 90df74992..8762f1df2 100644 --- a/be/src/runtime/free-pool.h +++ b/be/src/runtime/free-pool.h @@ -62,12 +62,10 @@ class FreePool { return NULL; } #endif - ++net_allocations_; - if (FLAGS_disable_mem_pools) return reinterpret_cast(malloc(size)); - /// Return a non-NULL dummy pointer. NULL is reserved for failures. if (UNLIKELY(size == 0)) return mem_pool_->EmptyAllocPtr(); - + ++net_allocations_; + if (FLAGS_disable_mem_pools) return reinterpret_cast(malloc(size)); int free_list_idx = Bits::Log2Ceiling64(size); DCHECK_LT(free_list_idx, NUM_LISTS); FreeListNode* allocation = lists_[free_list_idx].next; @@ -92,12 +90,12 @@ class FreePool { } void Free(uint8_t* ptr) { + if (UNLIKELY(ptr == NULL || ptr == mem_pool_->EmptyAllocPtr())) return; --net_allocations_; if (FLAGS_disable_mem_pools) { free(ptr); return; } - if (UNLIKELY(ptr == NULL || ptr == mem_pool_->EmptyAllocPtr())) return; FreeListNode* node = reinterpret_cast(ptr - sizeof(FreeListNode)); FreeListNode* list = node->list; #ifndef NDEBUG diff --git a/be/src/udf/udf-test.cc b/be/src/udf/udf-test.cc index 4857a0630..86f02c2cb 100644 --- a/be/src/udf/udf-test.cc +++ b/be/src/udf/udf-test.cc @@ -123,8 +123,9 @@ IntVal ValidateFail(FunctionContext* context) { } IntVal ValidateMem(FunctionContext* context) { - EXPECT_TRUE(context->Allocate(0) == NULL); - uint8_t* buffer = context->Allocate(10); + uint8_t* buffer = context->Allocate(0); + EXPECT_TRUE(buffer != NULL); + buffer = context->Reallocate(buffer, 10); EXPECT_TRUE(buffer != NULL); memset(buffer, 0, 10); context->Free(buffer); diff --git a/be/src/udf/udf.cc b/be/src/udf/udf.cc index ff81c9c04..c1fb5be42 100644 --- a/be/src/udf/udf.cc +++ b/be/src/udf/udf.cc @@ -286,7 +286,6 @@ inline bool FunctionContextImpl::CheckAllocResult(const char* fn_name, uint8_t* FunctionContext::Allocate(int byte_size) noexcept { assert(!impl_->closed_); - if (byte_size == 0) return NULL; uint8_t* buffer = impl_->pool_->Allocate(byte_size); if (UNLIKELY(!impl_->CheckAllocResult("FunctionContext::Allocate", buffer, byte_size))) { @@ -418,7 +417,6 @@ void FunctionContext::SetFunctionState(FunctionStateScope scope, void* ptr) { uint8_t* FunctionContextImpl::AllocateLocal(int64_t byte_size) noexcept { assert(!closed_); - if (byte_size == 0) return NULL; uint8_t* buffer = pool_->Allocate(byte_size); if (UNLIKELY(!CheckAllocResult("FunctionContextImpl::AllocateLocal", buffer, byte_size))) { diff --git a/testdata/workloads/functional-query/queries/QueryTest/aggregation.test b/testdata/workloads/functional-query/queries/QueryTest/aggregation.test index 5389b04f3..740f777a6 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/aggregation.test +++ b/testdata/workloads/functional-query/queries/QueryTest/aggregation.test @@ -154,13 +154,31 @@ from alltypesagg where day is not null bigint, string, string, string, string ==== ---- QUERY -# Test for IMPALA-3018. Verify update() function of min() handles +# Test for IMPALA-3018. Verify update() functions of min() and max() handle # zero-length string correctly. -select min(str) from (values ('aaa' as str), ('')) as tmp +select max(str), min(str) from (values ('aaa' as str), (''), ('123')) as tmp +---- RESULTS +'aaa','' +---- TYPES +string,string +==== +---- QUERY +# Test for IMPALA-3018. Verify update() function of last_value() handles +# zero-length string correctly. +select last_value(b) over (partition by a order by d) from functional.nulltable; ---- RESULTS '' ---- TYPES -String +string +==== +---- QUERY +# Test for IMPALA-3018. Verify update() function of first_value() handles +# zero-length string correctly. +select first_value(b) over (partition by a order by d) from functional.nulltable; +---- RESULTS +'' +---- TYPES +string ==== ---- QUERY # grouping by different data types, with NULLs