diff --git a/fe/src/main/java/com/cloudera/impala/hive/executor/UdfExecutor.java b/fe/src/main/java/com/cloudera/impala/hive/executor/UdfExecutor.java index 59365daff..b8ba01e23 100644 --- a/fe/src/main/java/com/cloudera/impala/hive/executor/UdfExecutor.java +++ b/fe/src/main/java/com/cloudera/impala/hive/executor/UdfExecutor.java @@ -51,7 +51,7 @@ import com.google.common.collect.Lists; // Wrapper object to run hive UDFs. This class works with UdfCallExpr in the // backend to marshall data back and forth between the execution engine and // the java UDF class. -// See the comments in be/src/exprs/udf-call-expr.h for more details. +// See the comments in be/src/exprs/hive-udf-call.h for more details. // TODO: should we cache loaded jars and classes? @SuppressWarnings("restriction") public class UdfExecutor { @@ -196,8 +196,9 @@ public class UdfExecutor { for (int i = 0; i < argTypes_.length; ++i) { if (UnsafeUtil.UNSAFE.getByte(inputNullsPtr_ + i) == 0) { if (isArgString_[i]) { - Preconditions.checkState(inputArgs_[i] instanceof ImpalaBytesWritable); - inputArgs_[i] = ((ImpalaBytesWritable)inputArgs_[i]).toString(); + Preconditions.checkState(inputObjects_[i] instanceof ImpalaBytesWritable); + inputArgs_[i] = + new String(((ImpalaBytesWritable)inputObjects_[i]).getBytes()); } else { inputArgs_[i] = inputObjects_[i]; } @@ -342,7 +343,7 @@ public class UdfExecutor { // These objects are allocated once and reused across calls to evaluate() private void allocateInputObjects() throws ImpalaRuntimeException { inputObjects_ = new Writable[argTypes_.length]; - inputArgs_ = new Writable[argTypes_.length]; + inputArgs_ = new Object[argTypes_.length]; isArgString_ = new boolean[argTypes_.length]; for (int i = 0; i < argTypes_.length; ++i) { @@ -378,8 +379,7 @@ public class UdfExecutor { inputObjects_[i] = w; } else if (method_.getParameterTypes()[i] == String.class) { isArgString_[i] = true; - // String can be mapped to any String-like Writable class. We need - // to call toString on it before calling the UDF. + // String can be mapped to any String-like Writable class. ImpalaBytesWritable w = new ImpalaBytesWritable(inputBufferPtr_ + offset); inputObjects_[i] = w; } else { diff --git a/testdata/workloads/functional-query/queries/QueryTest/hive-udf.test b/testdata/workloads/functional-query/queries/QueryTest/hive-udf.test index ff12c8040..83d959276 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/hive-udf.test +++ b/testdata/workloads/functional-query/queries/QueryTest/hive-udf.test @@ -71,11 +71,15 @@ double 10 ==== ---- QUERY -select udf_test.identity("why hello there"); +# IMPALA-1456. Each "identity" call below tests a different type (BytesWritable, Text, +# and String). +select udf_test.identity("why hello there"), + udf_test.identity("why", " hello there"), + udf_test.identity("why", " hello", " there"); ---- TYPES -string +string, string, string ---- RESULTS -'why hello there' +'why hello there','why hello there','why hello there' ==== ---- QUERY select udf_test.identity(NULL); @@ -85,12 +89,13 @@ boolean NULL ==== ---- QUERY -# IMPALA-1134. Each "identity" call below tests a different type (BytesWritable and Text). -# The different types are handled slightly differently. +# IMPALA-1134. Each "identity" call below tests a different type (BytesWritable, Text, +# and String). The different types are handled slightly differently. select length(udf_test.identity("0123456789")), - length(udf_test.identity("0123456789", "0123456789")); + length(udf_test.identity("0123456789", "0123456789")), + length(udf_test.identity("0123456789", "0123456789", "0123456789")); ---- TYPES -int, int +int, int, int ---- RESULTS -10,20 +10,20,30 ==== diff --git a/testdata/workloads/functional-query/queries/QueryTest/load-hive-udfs.test b/testdata/workloads/functional-query/queries/QueryTest/load-hive-udfs.test index ea9f341b6..fdc6269fe 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/load-hive-udfs.test +++ b/testdata/workloads/functional-query/queries/QueryTest/load-hive-udfs.test @@ -16,6 +16,7 @@ drop function if exists udf_test.identity(float); drop function if exists udf_test.identity(double); drop function if exists udf_test.identity(string); drop function if exists udf_test.identity(string, string); +drop function if exists udf_test.identity(string, string, string); drop function if exists udf_test.identity(timestamp); create database if not exists udf_test; @@ -79,4 +80,8 @@ symbol='com.cloudera.impala.TestUdf'; create function udf_test.identity(string, string) returns string location '/test-warehouse/impala-hive-udfs.jar' symbol='com.cloudera.impala.TestUdf'; + +create function udf_test.identity(string, string, string) returns string +location '/test-warehouse/impala-hive-udfs.jar' +symbol='com.cloudera.impala.TestUdf'; ==== diff --git a/tests/test-hive-udfs/src/main/java/com/cloudera/impala/TestUdf.java b/tests/test-hive-udfs/src/main/java/com/cloudera/impala/TestUdf.java index df0d7d850..fc5d3cf43 100644 --- a/tests/test-hive-udfs/src/main/java/com/cloudera/impala/TestUdf.java +++ b/tests/test-hive-udfs/src/main/java/com/cloudera/impala/TestUdf.java @@ -90,4 +90,8 @@ public class TestUdf extends UDF { if (a == null || b == null) return null; return new Text(a.toString() + b.toString()); } + public String evaluate(String a, String b, String c) { + if (a == null || b == null || c == null) return null; + return a + b + c; + } }