Permalink
Browse files

IMPALA-4518: CopyStringVal() doesn't copy null string

Previously, CopyStringVal() mistakenly copies a null
StringVal as an empty string (i.e. a non-null string
with zero length). This change fixes the problem by
distinguishing between these two cases in CopyStringVal()
and handles them properly. Also added a test case for it.

This problem only started showing up recently due to
commit 51268c0 which
calls CopyStringVal() in OffsetFnInit(). All other
pre-existing callers of CopyStringVal() before that
commit checks if 'src' is null before calling it so
the problem never showed up. In that sense, this is
a latent bug exposed by the aforementioned commit.

Change-Id: I3a5b9349dd08556eba5cfedc8c0063cc59f5be03
Reviewed-on: http://gerrit.cloudera.org:8080/5198
Reviewed-by: Michael Ho <kwho@cloudera.com>
Tested-by: Internal Jenkins
  • Loading branch information...
michaelhkw authored and Internal Jenkins committed Nov 22, 2016
1 parent 707f71b commit b82eed5ee02eccc21cc69d4af107c0acf31e08fa
@@ -147,14 +147,18 @@ static void AllocBuffer(FunctionContext* ctx, StringVal* dst, size_t buf_len) {
// 'buf_len' bytes and copies the content of StringVal 'src' into it.
// If allocation fails, 'dst' will be set to a null string.
static void CopyStringVal(FunctionContext* ctx, const StringVal& src, StringVal* dst) {
uint8_t* copy = ctx->Allocate(src.len);
if (UNLIKELY(copy == NULL && src.len != 0)) {
DCHECK(!ctx->impl()->state()->GetQueryStatus().ok());
if (src.is_null) {
*dst = StringVal::null();
} else {
*dst = StringVal(copy, src.len);
// Avoid memcpy() to NULL ptr as it's undefined.
if (LIKELY(dst->ptr != NULL)) memcpy(dst->ptr, src.ptr, src.len);
uint8_t* copy = ctx->Allocate(src.len);
if (UNLIKELY(copy == NULL)) {
// Zero-length allocation always returns a hard-coded pointer.
DCHECK(src.len != 0 && !ctx->impl()->state()->GetQueryStatus().ok());
*dst = StringVal::null();
} else {
*dst = StringVal(copy, src.len);
memcpy(dst->ptr, src.ptr, src.len);
}
}
}
@@ -1907,3 +1907,13 @@ BIGINT
---- RESULTS
7299
====
---- QUERY
# Regression test for IMPALA-4518: Distinguishes between empty string and null string.
select f,lead(b,1,null) over (order by f)
from (select * from nulltable union all select * from nulltable) x;
---- TYPES
STRING, STRING
---- RESULTS
'a\x00b',''
'a\x00b','NULL'
====

0 comments on commit b82eed5

Please sign in to comment.