You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by db...@apache.org on 2023/03/06 14:18:43 UTC
[impala] branch master updated: IMPALA-11911: Fix NULL argument handling in Hive GenericUDFs
This is an automated email from the ASF dual-hosted git repository.
dbecker pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 67bb870aa IMPALA-11911: Fix NULL argument handling in Hive GenericUDFs
67bb870aa is described below
commit 67bb870aa302b3509fa4a0f8d846efedc04e1514
Author: Csaba Ringhofer <cs...@cloudera.com>
AuthorDate: Tue Feb 14 17:25:50 2023 +0100
IMPALA-11911: Fix NULL argument handling in Hive GenericUDFs
Before this patch if an argument of a GenericUDF was NULL, then Impala
passed it as null instead of a DeferredObject. This was incorrect, as
a DeferredObject is expected with a get() function that returns null.
See the Jira for more details and GenericUDF examples in Hive.
TestGenericUdf's NULL handling was further broken in IMPALA-11549,
leading to throwing null pointer exceptions when the UDF's result is
NULL. This test bug was not detected, because Hive udf tests were
running with default abort_java_udf_on_exception=false, which means
that exceptions from Hive UDFs only led to warnings and returning NULL,
which was the expected result in all affected test queries.
This patch fixes the behavior in HiveUdfExecutorGeneric and improves
FE/EE tests to catch null handling related issues. Most Hive UDF tests
are run with abort_java_udf_on_exception=true after this patch to treat
exceptions in UDFs as errors. The ones where the test checks that NULL
is returned if an exception is thrown while abort_java_udf_on_exception
is false are moved to new .test files.
TestGenericUdf is also fixed (and simplified) to handle NULL return
values correctly.
Change-Id: I53238612f4037572abb6d2cc913dd74ee830a9c9
Reviewed-on: http://gerrit.cloudera.org:8080/19499
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../hive/executor/HiveUdfExecutorGeneric.java | 13 +-
.../impala/hive/executor/TestGenericUdf.java | 94 +++-----
.../impala/hive/executor/UdfExecutorTest.java | 247 ++++++++++++---------
.../java/org/apache/impala/TestGenericUdf.java | 92 +++-----
.../generic-java-udf-no-abort-on-exception.test | 23 ++
.../queries/QueryTest/generic-java-udf.test | 20 +-
.../QueryTest/java-udf-no-abort-on-exception.test | 23 ++
.../queries/QueryTest/java-udf.test | 22 +-
tests/query_test/test_udfs.py | 10 +
9 files changed, 270 insertions(+), 274 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java b/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
index 45d9c8f55..5d292e17e 100644
--- a/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
+++ b/fe/src/main/java/org/apache/impala/hive/executor/HiveUdfExecutorGeneric.java
@@ -70,11 +70,13 @@ public class HiveUdfExecutorGeneric extends HiveUdfExecutor {
private GenericUDF genericUDF_;
// Hive Generic UDFs expect a DeferredObject for each parameter passed in. However,
- // if the ith parameter is NULL, then Hive expects the deferredObject[i] value to
- // contain null. The deferredParameters array is populated at initialization time. The
- // runtimeDeferredObjects is passed into the Hive Generic UDF code at runtime and
- // the runDeferredParameters[i] value will either contain deferredParameters[i] or NULL.
+ // if the ith parameter is NULL, then Hive expects the deferredObject[i].get() to
+ // return null. The deferredParameters array is populated at initialization time. The
+ // runtimeDeferredParameters_ is passed into the Hive Generic UDF code at runtime and
+ // the runtimeDeferredParameters_[i] value will either contain deferredParameters_[i]
+ // or a shared empty DeferredObject deferredNullParameter_.
private DeferredObject[] deferredParameters_;
+ private DeferredObject deferredNullParameter_;
private DeferredObject[] runtimeDeferredParameters_;
/**
@@ -87,6 +89,7 @@ public class HiveUdfExecutorGeneric extends HiveUdfExecutor {
JavaUdfDataType.getTypes(hiveJavaFn.getParameterTypes()));
genericUDF_ = hiveJavaFn.getGenericUDFInstance();
deferredParameters_ = createDeferredObjects();
+ deferredNullParameter_ = new DeferredJavaObject(null);
runtimeDeferredParameters_ = new DeferredObject[getNumParams()];
}
@@ -105,7 +108,7 @@ public class HiveUdfExecutorGeneric extends HiveUdfExecutor {
runtimeDeferredParameters_[i] =
(UnsafeUtil.UNSAFE.getByte(inputNullsPtr + i) == 0)
? deferredParameters_[i]
- : null;
+ : deferredNullParameter_;
}
return genericUDF_.evaluate(runtimeDeferredParameters_);
} catch (HiveException e) {
diff --git a/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java b/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
index 22ec3b09d..88158b1be 100644
--- a/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
+++ b/fe/src/test/java/org/apache/impala/hive/executor/TestGenericUdf.java
@@ -117,7 +117,7 @@ public class TestGenericUdf extends GenericUDF {
}
protected PrimitiveObjectInspector getReturnObjectInspector(
- PrimitiveObjectInspector oi) {
+ PrimitiveObjectInspector oi) {
// Simply returns the same object inspector. Subclasses can override this to return
// different types of object inspectors.
return oi;
@@ -131,6 +131,10 @@ public class TestGenericUdf extends GenericUDF {
"arguments expected. Expected: "
+ inputTypes_.size() + " actual: " + arguments.length);
}
+ // The null logic is the same for all types: if any arg is NULL, return NULL.
+ for (DeferredObject arg: arguments) {
+ if (arg.get() == null) return null;
+ }
switch (argAndRetType_) {
case BOOLEAN:
return evaluateBooleanWrapped(arguments);
@@ -180,11 +184,9 @@ public class TestGenericUdf extends GenericUDF {
protected Boolean evaluateBoolean(DeferredObject[] inputs) throws HiveException {
boolean finalBoolean = false;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof BooleanWritable)) {
- throw new HiveException("Expected BooleanWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected BooleanWritable but got " + input.get().getClass());
}
boolean currentBool = ((BooleanWritable) input.get()).get();
finalBoolean |= currentBool;
@@ -195,11 +197,9 @@ public class TestGenericUdf extends GenericUDF {
protected Byte evaluateByte(DeferredObject[] inputs) throws HiveException {
byte finalByte = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof ByteWritable)) {
- throw new HiveException("Expected ByteWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected ByteWritable but got " + input.get().getClass());
}
byte currentByte = ((ByteWritable) input.get()).get();
finalByte += currentByte;
@@ -210,11 +210,9 @@ public class TestGenericUdf extends GenericUDF {
protected Short evaluateShort(DeferredObject[] inputs) throws HiveException {
short finalShort = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof ShortWritable)) {
- throw new HiveException("Expected ShortWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected ShortWritable but got " + input.get().getClass());
}
short currentShort = ((ShortWritable) input.get()).get();
finalShort += currentShort;
@@ -225,11 +223,9 @@ public class TestGenericUdf extends GenericUDF {
protected Integer evaluateInt(DeferredObject[] inputs) throws HiveException {
int finalInt = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof IntWritable)) {
- throw new HiveException("Expected IntWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected IntWritable but got " + input.get().getClass());
}
int currentInt = ((IntWritable) input.get()).get();
finalInt += currentInt;
@@ -240,11 +236,9 @@ public class TestGenericUdf extends GenericUDF {
protected Long evaluateLong(DeferredObject[] inputs) throws HiveException {
long finalLong = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof LongWritable)) {
- throw new HiveException("Expected LongWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected LongWritable but got " + input.get().getClass());
}
long currentLong = ((LongWritable) input.get()).get();
finalLong += currentLong;
@@ -255,11 +249,9 @@ public class TestGenericUdf extends GenericUDF {
protected Float evaluateFloat(DeferredObject[] inputs) throws HiveException {
float finalFloat = 0.0F;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof FloatWritable)) {
- throw new HiveException("Expected FloatWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected FloatWritable but got " + input.get().getClass());
}
float currentFloat = ((FloatWritable) input.get()).get();
finalFloat += currentFloat;
@@ -269,12 +261,10 @@ public class TestGenericUdf extends GenericUDF {
protected Double evaluateDouble(DeferredObject[] inputs) throws HiveException {
double finalDouble = 0.0;
- for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
+ for (DeferredObject input : inputs) {;
if (!(input.get() instanceof DoubleWritable)) {
- throw new HiveException("Expected DoubleWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected DoubleWritable but got " + input.get().getClass());
}
double currentDouble = ((DoubleWritable) input.get()).get();
finalDouble += currentDouble;
@@ -285,9 +275,6 @@ public class TestGenericUdf extends GenericUDF {
protected String evaluateString(DeferredObject[] inputs) throws HiveException {
String finalString = "";
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof Text)) {
throw new HiveException("Expected Text but got " + input.get().getClass());
}
@@ -301,9 +288,6 @@ public class TestGenericUdf extends GenericUDF {
int resultLength = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof BytesWritable)) {
throw new HiveException(
"Expected BytesWritable but got " + input.get().getClass());
@@ -327,66 +311,46 @@ public class TestGenericUdf extends GenericUDF {
protected Object evaluateBooleanWrapped(DeferredObject[] inputs)
throws HiveException {
- BooleanWritable resultBool = new BooleanWritable();
- resultBool.set(evaluateBoolean(inputs));
- return resultBool;
+ return new BooleanWritable(evaluateBoolean(inputs));
}
protected Object evaluateByteWrapped(DeferredObject[] inputs)
throws HiveException {
- ByteWritable resultByte = new ByteWritable();
- resultByte.set(evaluateByte(inputs));
- return resultByte;
+ return new ByteWritable(evaluateByte(inputs));
}
protected Object evaluateShortWrapped(DeferredObject[] inputs)
throws HiveException {
- ShortWritable resultShort = new ShortWritable();
- resultShort.set(evaluateShort(inputs));
- return resultShort;
+ return new ShortWritable(evaluateShort(inputs));
}
protected Object evaluateIntWrapped(DeferredObject[] inputs)
throws HiveException {
- IntWritable resultInt = new IntWritable();
- resultInt.set(evaluateInt(inputs));
- return resultInt;
+ return new IntWritable(evaluateInt(inputs));
}
protected Object evaluateLongWrapped(DeferredObject[] inputs)
throws HiveException {
- LongWritable resultLong = new LongWritable();
- resultLong.set(evaluateLong(inputs));
- return resultLong;
+ return new LongWritable(evaluateLong(inputs));
}
protected Object evaluateFloatWrapped(DeferredObject[] inputs)
throws HiveException {
- FloatWritable resultFloat = new FloatWritable();
- resultFloat.set(evaluateFloat(inputs));
- return resultFloat;
+ return new FloatWritable(evaluateFloat(inputs));
}
protected Object evaluateDoubleWrapped(DeferredObject[] inputs)
throws HiveException {
- DoubleWritable resultDouble = new DoubleWritable();
- resultDouble.set(evaluateDouble(inputs));
- return resultDouble;
+ return new DoubleWritable(evaluateDouble(inputs));
}
protected Object evaluateStringWrapped(DeferredObject[] inputs) throws HiveException {
- Text resultString = new Text();
- resultString.set(evaluateString(inputs));
- return resultString;
+ return new Text(evaluateString(inputs));
}
protected Object evaluateBinaryWrapped(DeferredObject[] inputs)
throws HiveException {
- byte[] result = evaluateBinary(inputs);
- if (result == null) return null;
- BytesWritable resultBinary = new BytesWritable();
- resultBinary.set(result, 0, result.length);
- return resultBinary;
+ return new BytesWritable(evaluateBinary(inputs));
}
protected String getSignatureString(PrimitiveCategory argAndRetType_,
diff --git a/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java b/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
index a2e617a8e..0ea62bbe2 100644
--- a/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
+++ b/fe/src/test/java/org/apache/impala/hive/executor/UdfExecutorTest.java
@@ -203,7 +203,8 @@ public class UdfExecutorTest {
// Validate the argument's type. To mimic the behavior of the BE, only primitive types
// are allowed.
void validateArgType(Object w) {
- if (w instanceof String ||
+ if (w == null ||
+ w instanceof String ||
w instanceof Text ||
w instanceof ImpalaBytesWritable ||
w instanceof ImpalaIntWritable ||
@@ -225,16 +226,17 @@ public class UdfExecutorTest {
* jar file.
* @param udfClassPath: fully qualified class path for the UDF.
* @param retType: the return type of the UDF
- * @param args: the input parameters of the UDF
+ * @param originalArgs: input parameters used to deduce the type of the argument
+ * @param args: the input parameters of the UDF (can be null)
*/
UdfExecutor createUdfExecutor(String jarFile, String udfClassPath, Type retType,
- Object... args) throws ImpalaException, TException {
+ Object[] originalArgs, Object[] args) throws ImpalaException, TException {
int inputBufferSize = 0;
ArrayList<Integer> inputByteOffsets = Lists.newArrayList();
ArrayList<Type> argTypes = Lists.newArrayList();
- for (int i = 0; i < args.length; ++i) {
- Preconditions.checkNotNull(args[i]);
- Type argType = getType(args[i]);
+ for (Object originalArg: originalArgs) {
+ Preconditions.checkNotNull(originalArg);
+ Type argType = getType(originalArg);
inputByteOffsets.add(new Integer(inputBufferSize));
inputBufferSize += argType.getSlotSize();
argTypes.add(argType);
@@ -247,7 +249,8 @@ public class UdfExecutorTest {
long inputNullsPtr = allocate(argTypes.size());
for (int i = 0; i < argTypes.size(); ++i) {
- UnsafeUtil.UNSAFE.putByte(inputNullsPtr + i, (byte)0);
+ boolean isNull = args[i] == null;
+ UnsafeUtil.UNSAFE.putByte(inputNullsPtr + i, (byte)(isNull ? 1 : 0));
}
long inputBufferPtr = allocate(inputBufferSize);
long outputNullPtr = allocate(1);
@@ -260,16 +263,19 @@ public class UdfExecutorTest {
}
// Runs the hive udf contained in c. Validates that c.evaluate(args) == expectedValue,
- // if the "validate" argument is true. Arguments and return value cannot be NULL.
+ // if the "validate" argument is true.
+ // NULLs can be passed in 'args', but 'originalArgs' must contain non-NULL types
+ // to be able to deduce the signature of the UDF.
void TestUdfImpl(String jarFile, Class<?> c, Object expectedValue,
- Type expectedType, boolean validate, Object... args)
+ Type expectedType, boolean validate, Object[] originalArgs, Object[] args)
throws ImpalaException, MalformedURLException, TException {
- UdfExecutor e = createUdfExecutor(jarFile, c.getName(), expectedType, args);
+ UdfExecutor e = createUdfExecutor(
+ jarFile, c.getName(), expectedType, originalArgs, args);
Method method = e.getMethod();
Object[] inputArgs = new Object[args.length];
for (int i = 0; i < args.length; ++i) {
validateArgType(args[i]);
- if (args[i] instanceof String) {
+ if (args[i] != null && args[i] instanceof String) {
// For authoring the test, we'll just pass string and make the proper
// object here.
if (method != null && method.getParameterTypes()[i] == Text.class) {
@@ -287,96 +293,9 @@ public class UdfExecutorTest {
for (int i = 0; i < 10; ++i) {
long r = e.evaluateForTesting(inputArgs);
if (!validate) continue;
- List<String> errMsgs = Lists.newArrayList();
- switch (expectedType.getPrimitiveType()) {
- case BOOLEAN: {
- boolean expected = ((ImpalaBooleanWritable)expectedValue).get();
- boolean actual = UnsafeUtil.UNSAFE.getByte(r) != 0;
- if (expected != actual) {
- errMsgs.add("Expected bool: " + expected);
- errMsgs.add("Actual bool: " + actual);
- }
- break;
- }
- case TINYINT: {
- int expected = ((ImpalaTinyIntWritable)expectedValue).get();
- int actual = UnsafeUtil.UNSAFE.getByte(r);
- if (expected != actual) {
- errMsgs.add("Expected tinyint: " + expected);
- errMsgs.add("Actual tinyint: " + actual);
- }
- break;
- }
- case SMALLINT: {
- int expected = ((ImpalaSmallIntWritable)expectedValue).get();
- int actual = UnsafeUtil.UNSAFE.getShort(r);
- if (expected != actual) {
- errMsgs.add("Expected smallint: " + expected);
- errMsgs.add("Actual smallint: " + actual);
- }
- break;
- }
- case INT: {
- int expected = ((ImpalaIntWritable)expectedValue).get();
- int actual = UnsafeUtil.UNSAFE.getInt(r);
- if (expected != actual) {
- errMsgs.add("Expected int: " + expected);
- errMsgs.add("Actual int: " + actual);
- }
- break;
- }
- case BIGINT: {
- long expected = ((ImpalaBigIntWritable)expectedValue).get();
- long actual = UnsafeUtil.UNSAFE.getLong(r);
- if (expected != actual) {
- errMsgs.add("Expected bigint: " + expected);
- errMsgs.add("Actual bigint: " + actual);
- }
- break;
- }
- case FLOAT: {
- float expected = ((ImpalaFloatWritable)expectedValue).get();
- float actual = UnsafeUtil.UNSAFE.getFloat(r);
- if (expected != actual) {
- errMsgs.add("Expected float: " + expected);
- errMsgs.add("Actual float: " + actual);
- }
- break;
- }
- case DOUBLE:
- double expected = ((ImpalaDoubleWritable)expectedValue).get();
- double actual = UnsafeUtil.UNSAFE.getDouble(r);
- if (expected != actual) {
- errMsgs.add("Expected double: " + expected);
- errMsgs.add("Actual double: " + actual);
- }
- break;
- case STRING:
- case BINARY: {
- byte[] expectedBytes = null;
- if (expectedValue instanceof ImpalaBytesWritable) {
- expectedBytes = ((ImpalaBytesWritable)expectedValue).getBytes();
- } else if (expectedValue instanceof ImpalaTextWritable) {
- expectedBytes = ((ImpalaTextWritable)expectedValue).getBytes();
- } else if (expectedValue instanceof String) {
- expectedBytes = ((String)expectedValue).getBytes();
- } else {
- Preconditions.checkState(false);
- }
- ImpalaStringWritable sw = new ImpalaStringWritable(r);
- if (Arrays.equals(expectedBytes, sw.getBytes())) break;
-
- errMsgs.add("Expected string: " + Bytes.toString(expectedBytes));
- errMsgs.add("Actual string: " + Bytes.toString(sw.getBytes()));
- errMsgs.add("Expected bytes: " + Arrays.toString(expectedBytes));
- errMsgs.add("Actual bytes: " + Arrays.toString(sw.getBytes()));
- break;
- }
- default:
- Preconditions.checkArgument(false);
- }
-
// Check if there was a mismatch and print a detailed error log.
+ List<String> errMsgs = Lists.newArrayList();
+ ValidateReturnPtr(r, expectedValue, expectedType, errMsgs);
if (!errMsgs.isEmpty()) {
errMsgs.add("Eval iteration: " + i);
errMsgs.add("Return type: " + expectedType.toSql());
@@ -391,24 +310,139 @@ public class UdfExecutorTest {
}
}
+ // Interprets result pointer 'r' as expectedType and compares it to expectedValue.
+ // If there is a mismatch, errMsgs will contain info about the error. Emtpy errMsgs
+ // means that 'r' points to the expected result.
+ void ValidateReturnPtr(
+ long r, Object expectedValue, Type expectedType, List<String> errMsgs) {
+ if (expectedValue == null) {
+ if (r != 0) {
+ errMsgs.add("Expected NULL but got non-NULL result.");
+ }
+ return;
+ } else {
+ if (r == 0) {
+ errMsgs.add("Expected non-NULL but got NULL result.");
+ return;
+ }
+ }
+
+ switch (expectedType.getPrimitiveType()) {
+ case BOOLEAN: {
+ boolean expected = ((ImpalaBooleanWritable)expectedValue).get();
+ boolean actual = UnsafeUtil.UNSAFE.getByte(r) != 0;
+ if (expected != actual) {
+ errMsgs.add("Expected bool: " + expected);
+ errMsgs.add("Actual bool: " + actual);
+ }
+ break;
+ }
+ case TINYINT: {
+ int expected = ((ImpalaTinyIntWritable)expectedValue).get();
+ int actual = UnsafeUtil.UNSAFE.getByte(r);
+ if (expected != actual) {
+ errMsgs.add("Expected tinyint: " + expected);
+ errMsgs.add("Actual tinyint: " + actual);
+ }
+ break;
+ }
+ case SMALLINT: {
+ int expected = ((ImpalaSmallIntWritable)expectedValue).get();
+ int actual = UnsafeUtil.UNSAFE.getShort(r);
+ if (expected != actual) {
+ errMsgs.add("Expected smallint: " + expected);
+ errMsgs.add("Actual smallint: " + actual);
+ }
+ break;
+ }
+ case INT: {
+ int expected = ((ImpalaIntWritable)expectedValue).get();
+ int actual = UnsafeUtil.UNSAFE.getInt(r);
+ if (expected != actual) {
+ errMsgs.add("Expected int: " + expected);
+ errMsgs.add("Actual int: " + actual);
+ }
+ break;
+ }
+ case BIGINT: {
+ long expected = ((ImpalaBigIntWritable)expectedValue).get();
+ long actual = UnsafeUtil.UNSAFE.getLong(r);
+ if (expected != actual) {
+ errMsgs.add("Expected bigint: " + expected);
+ errMsgs.add("Actual bigint: " + actual);
+ }
+ break;
+ }
+ case FLOAT: {
+ float expected = ((ImpalaFloatWritable)expectedValue).get();
+ float actual = UnsafeUtil.UNSAFE.getFloat(r);
+ if (expected != actual) {
+ errMsgs.add("Expected float: " + expected);
+ errMsgs.add("Actual float: " + actual);
+ }
+ break;
+ }
+ case DOUBLE:
+ double expected = ((ImpalaDoubleWritable)expectedValue).get();
+ double actual = UnsafeUtil.UNSAFE.getDouble(r);
+ if (expected != actual) {
+ errMsgs.add("Expected double: " + expected);
+ errMsgs.add("Actual double: " + actual);
+ }
+ break;
+ case STRING:
+ case BINARY: {
+ byte[] expectedBytes = null;
+ if (expectedValue instanceof ImpalaBytesWritable) {
+ expectedBytes = ((ImpalaBytesWritable)expectedValue).getBytes();
+ } else if (expectedValue instanceof ImpalaTextWritable) {
+ expectedBytes = ((ImpalaTextWritable)expectedValue).getBytes();
+ } else if (expectedValue instanceof String) {
+ expectedBytes = ((String)expectedValue).getBytes();
+ } else {
+ Preconditions.checkState(false);
+ }
+ ImpalaStringWritable sw = new ImpalaStringWritable(r);
+ if (Arrays.equals(expectedBytes, sw.getBytes())) break;
+
+ errMsgs.add("Expected string: " + Bytes.toString(expectedBytes));
+ errMsgs.add("Actual string: " + Bytes.toString(sw.getBytes()));
+ errMsgs.add("Expected bytes: " + Arrays.toString(expectedBytes));
+ errMsgs.add("Actual bytes: " + Arrays.toString(sw.getBytes()));
+ break;
+ }
+ default:
+ Preconditions.checkArgument(false);
+ }
+ }
+
void TestUdf(String jar, Class<?> c, Writable expectedValue, Object... args)
throws MalformedURLException, ImpalaException, TException {
- TestUdfImpl(jar, c, expectedValue, getType(expectedValue), true, args);
+ TestUdfImpl(jar, c, expectedValue, getType(expectedValue), true, args, args);
}
void TestUdf(String jar, Class<?> c, String expectedValue, Object... args)
throws MalformedURLException, ImpalaException, TException {
- TestUdfImpl(jar, c, expectedValue, getType(expectedValue), true, args);
+ TestUdfImpl(jar, c, expectedValue, getType(expectedValue), true, args, args);
+ }
+
+ void TestUdfWithNulls(String jar, Class<?> c, Writable originalExpectedValue,
+ boolean expectNull, Object[] originalArgs, Object[] args)
+ throws MalformedURLException, ImpalaException, TException {
+ TestUdfImpl(jar, c, expectNull ? null : originalExpectedValue,
+ getType(originalExpectedValue), true, originalArgs, args);
}
void TestHiveUdf(Class<?> c, Writable expectedValue, Object... args)
throws MalformedURLException, ImpalaException, TException {
- TestUdfImpl(HIVE_BUILTIN_JAR, c, expectedValue, getType(expectedValue), true, args);
+ TestUdfImpl(
+ HIVE_BUILTIN_JAR, c, expectedValue, getType(expectedValue), true, args, args);
}
void TestHiveUdfNoValidate(Class<?> c, Writable expectedValue, Object... args)
throws MalformedURLException, ImpalaException, TException {
- TestUdfImpl(HIVE_BUILTIN_JAR, c, expectedValue, getType(expectedValue), false, args);
+ TestUdfImpl(
+ HIVE_BUILTIN_JAR, c, expectedValue, getType(expectedValue), false, args, args);
}
@Test
@@ -485,6 +519,11 @@ public class UdfExecutorTest {
// both Writable and primitive Java return types.
TestUdf(null, TestGenericUdf.class, expectedValue, args);
TestUdf(null, TestGenericUdfWithJavaReturnTypes.class, expectedValue, args);
+ // Test that calling with NULLs leads to returning NULL.
+ Object[] nullArgs = new Object[args.length];
+ TestUdfWithNulls(null, TestGenericUdf.class, expectedValue, true, args, nullArgs);
+ TestUdfWithNulls(null, TestGenericUdfWithJavaReturnTypes.class,
+ expectedValue, true, args, nullArgs);
}
@Test
diff --git a/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java b/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
index 2b1e05269..b47c1ab5b 100644
--- a/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
+++ b/java/test-hive-udfs/src/main/java/org/apache/impala/TestGenericUdf.java
@@ -134,6 +134,10 @@ public class TestGenericUdf extends GenericUDF {
"arguments expected. Expected: "
+ inputTypes_.size() + " actual: " + arguments.length);
}
+ // The null logic is the same for all types: if any arg is NULL, return NULL.
+ for (DeferredObject arg: arguments) {
+ if (arg.get() == null) return null;
+ }
switch (argAndRetType_) {
case BOOLEAN:
return evaluateBooleanWrapped(arguments);
@@ -183,11 +187,9 @@ public class TestGenericUdf extends GenericUDF {
protected Boolean evaluateBoolean(DeferredObject[] inputs) throws HiveException {
boolean finalBoolean = false;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof BooleanWritable)) {
- throw new HiveException("Expected BooleanWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected BooleanWritable but got " + input.get().getClass());
}
boolean currentBool = ((BooleanWritable) input.get()).get();
finalBoolean |= currentBool;
@@ -198,11 +200,9 @@ public class TestGenericUdf extends GenericUDF {
protected Byte evaluateByte(DeferredObject[] inputs) throws HiveException {
byte finalByte = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof ByteWritable)) {
- throw new HiveException("Expected ByteWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected ByteWritable but got " + input.get().getClass());
}
byte currentByte = ((ByteWritable) input.get()).get();
finalByte += currentByte;
@@ -213,11 +213,9 @@ public class TestGenericUdf extends GenericUDF {
protected Short evaluateShort(DeferredObject[] inputs) throws HiveException {
short finalShort = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof ShortWritable)) {
- throw new HiveException("Expected ShortWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected ShortWritable but got " + input.get().getClass());
}
short currentShort = ((ShortWritable) input.get()).get();
finalShort += currentShort;
@@ -228,11 +226,9 @@ public class TestGenericUdf extends GenericUDF {
protected Integer evaluateInt(DeferredObject[] inputs) throws HiveException {
int finalInt = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof IntWritable)) {
- throw new HiveException("Expected IntWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected IntWritable but got " + input.get().getClass());
}
int currentInt = ((IntWritable) input.get()).get();
finalInt += currentInt;
@@ -243,11 +239,9 @@ public class TestGenericUdf extends GenericUDF {
protected Long evaluateLong(DeferredObject[] inputs) throws HiveException {
long finalLong = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof LongWritable)) {
- throw new HiveException("Expected LongWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected LongWritable but got " + input.get().getClass());
}
long currentLong = ((LongWritable) input.get()).get();
finalLong += currentLong;
@@ -258,11 +252,9 @@ public class TestGenericUdf extends GenericUDF {
protected Float evaluateFloat(DeferredObject[] inputs) throws HiveException {
float finalFloat = 0.0F;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof FloatWritable)) {
- throw new HiveException("Expected FloatWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected FloatWritable but got " + input.get().getClass());
}
float currentFloat = ((FloatWritable) input.get()).get();
finalFloat += currentFloat;
@@ -272,12 +264,10 @@ public class TestGenericUdf extends GenericUDF {
protected Double evaluateDouble(DeferredObject[] inputs) throws HiveException {
double finalDouble = 0.0;
- for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
+ for (DeferredObject input : inputs) {;
if (!(input.get() instanceof DoubleWritable)) {
- throw new HiveException("Expected DoubleWritable but got " + input.getClass());
+ throw new HiveException(
+ "Expected DoubleWritable but got " + input.get().getClass());
}
double currentDouble = ((DoubleWritable) input.get()).get();
finalDouble += currentDouble;
@@ -288,9 +278,6 @@ public class TestGenericUdf extends GenericUDF {
protected String evaluateString(DeferredObject[] inputs) throws HiveException {
String finalString = "";
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof Text)) {
throw new HiveException("Expected Text but got " + input.get().getClass());
}
@@ -304,9 +291,6 @@ public class TestGenericUdf extends GenericUDF {
int resultLength = 0;
for (DeferredObject input : inputs) {
- if (input == null) {
- return null;
- }
if (!(input.get() instanceof BytesWritable)) {
throw new HiveException(
"Expected BytesWritable but got " + input.get().getClass());
@@ -330,66 +314,46 @@ public class TestGenericUdf extends GenericUDF {
protected Object evaluateBooleanWrapped(DeferredObject[] inputs)
throws HiveException {
- BooleanWritable resultBool = new BooleanWritable();
- resultBool.set(evaluateBoolean(inputs));
- return resultBool;
+ return new BooleanWritable(evaluateBoolean(inputs));
}
protected Object evaluateByteWrapped(DeferredObject[] inputs)
throws HiveException {
- ByteWritable resultByte = new ByteWritable();
- resultByte.set(evaluateByte(inputs));
- return resultByte;
+ return new ByteWritable(evaluateByte(inputs));
}
protected Object evaluateShortWrapped(DeferredObject[] inputs)
throws HiveException {
- ShortWritable resultShort = new ShortWritable();
- resultShort.set(evaluateShort(inputs));
- return resultShort;
+ return new ShortWritable(evaluateShort(inputs));
}
protected Object evaluateIntWrapped(DeferredObject[] inputs)
throws HiveException {
- IntWritable resultInt = new IntWritable();
- resultInt.set(evaluateInt(inputs));
- return resultInt;
+ return new IntWritable(evaluateInt(inputs));
}
protected Object evaluateLongWrapped(DeferredObject[] inputs)
throws HiveException {
- LongWritable resultLong = new LongWritable();
- resultLong.set(evaluateLong(inputs));
- return resultLong;
+ return new LongWritable(evaluateLong(inputs));
}
protected Object evaluateFloatWrapped(DeferredObject[] inputs)
throws HiveException {
- FloatWritable resultFloat = new FloatWritable();
- resultFloat.set(evaluateFloat(inputs));
- return resultFloat;
+ return new FloatWritable(evaluateFloat(inputs));
}
protected Object evaluateDoubleWrapped(DeferredObject[] inputs)
throws HiveException {
- DoubleWritable resultDouble = new DoubleWritable();
- resultDouble.set(evaluateDouble(inputs));
- return resultDouble;
+ return new DoubleWritable(evaluateDouble(inputs));
}
protected Object evaluateStringWrapped(DeferredObject[] inputs) throws HiveException {
- Text resultString = new Text();
- resultString.set(evaluateString(inputs));
- return resultString;
+ return new Text(evaluateString(inputs));
}
protected Object evaluateBinaryWrapped(DeferredObject[] inputs)
throws HiveException {
- byte[] result = evaluateBinary(inputs);
- if (result == null) return null;
- BytesWritable resultBinary = new BytesWritable();
- resultBinary.set(result, 0, result.length);
- return resultBinary;
+ return new BytesWritable(evaluateBinary(inputs));
}
protected String getSignatureString(PrimitiveCategory argAndRetType_,
diff --git a/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf-no-abort-on-exception.test b/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf-no-abort-on-exception.test
new file mode 100644
index 000000000..d05751d26
--- /dev/null
+++ b/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf-no-abort-on-exception.test
@@ -0,0 +1,23 @@
+====
+---- QUERY
+# IMPALA-1392: Hive UDFs that throw exceptions should return NULL
+select generic_throws_exception();
+---- TYPES
+boolean
+---- RESULTS
+NULL
+====
+---- QUERY
+select generic_throws_exception() from functional.alltypestiny;
+---- TYPES
+boolean
+---- RESULTS
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test b/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
index 3651cb23a..3c9fca948 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/generic-java-udf.test
@@ -124,26 +124,14 @@ int, string
3,'abcd'
====
---- QUERY
-# IMPALA-1392: Hive UDFs that throw exceptions should return NULL
select generic_throws_exception();
----- TYPES
-boolean
----- RESULTS
-NULL
+---- CATCH
+Test exception
====
---- QUERY
select generic_throws_exception() from functional.alltypestiny;
----- TYPES
-boolean
----- RESULTS
-NULL
-NULL
-NULL
-NULL
-NULL
-NULL
-NULL
-NULL
+---- CATCH
+Test exception
====
---- QUERY
select generic_add(cast(1 as int), cast(2 as int));
diff --git a/testdata/workloads/functional-query/queries/QueryTest/java-udf-no-abort-on-exception.test b/testdata/workloads/functional-query/queries/QueryTest/java-udf-no-abort-on-exception.test
new file mode 100644
index 000000000..051f91411
--- /dev/null
+++ b/testdata/workloads/functional-query/queries/QueryTest/java-udf-no-abort-on-exception.test
@@ -0,0 +1,23 @@
+====
+---- QUERY
+# IMPALA-1392: Hive UDFs that throw exceptions should return NULL
+select throws_exception();
+---- TYPES
+boolean
+---- RESULTS
+NULL
+====
+---- QUERY
+select throws_exception() from functional.alltypestiny;
+---- TYPES
+boolean
+---- RESULTS
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/java-udf.test b/testdata/workloads/functional-query/queries/QueryTest/java-udf.test
index 3a18780da..96769dc77 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/java-udf.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/java-udf.test
@@ -103,32 +103,14 @@ int, int, int
10,20,30
====
---- QUERY
-# IMPALA-1392: Hive UDFs that throw exceptions should return NULL
select throws_exception();
----- TYPES
-boolean
----- RESULTS
-NULL
-====
----- QUERY
-set abort_java_udf_on_exception=true;
-select throws_exception() from functional.alltypestiny;
---- CATCH
Test exception
====
---- QUERY
select throws_exception() from functional.alltypestiny;
----- TYPES
-boolean
----- RESULTS
-NULL
-NULL
-NULL
-NULL
-NULL
-NULL
-NULL
-NULL
+---- CATCH
+Test exception
====
---- QUERY
select hive_add(cast(1 as int), cast(2 as int));
diff --git a/tests/query_test/test_udfs.py b/tests/query_test/test_udfs.py
index d70a50663..5ac87c038 100644
--- a/tests/query_test/test_udfs.py
+++ b/tests/query_test/test_udfs.py
@@ -336,13 +336,23 @@ class TestUdfExecution(TestUdfBase):
self.run_test_case('QueryTest/udf-no-expr-rewrite', vector, use_db=unique_database)
def test_java_udfs(self, vector, unique_database):
+ vector = copy(vector)
+ vector.get_value('exec_option')['abort_java_udf_on_exception'] = True
self.run_test_case('QueryTest/load-java-udfs', vector, use_db=unique_database)
self.run_test_case('QueryTest/load-java-udfs-fail', vector, use_db=unique_database)
self.run_test_case('QueryTest/java-udf', vector, use_db=unique_database)
+ vector.get_value('exec_option')['abort_java_udf_on_exception'] = False
+ self.run_test_case('QueryTest/java-udf-no-abort-on-exception', vector,
+ use_db=unique_database)
def test_generic_java_udfs(self, vector, unique_database):
+ vector = copy(vector)
+ vector.get_value('exec_option')['abort_java_udf_on_exception'] = True
self.run_test_case('QueryTest/load-generic-java-udfs', vector, use_db=unique_database)
self.run_test_case('QueryTest/generic-java-udf', vector, use_db=unique_database)
+ vector.get_value('exec_option')['abort_java_udf_on_exception'] = False
+ self.run_test_case('QueryTest/generic-java-udf-no-abort-on-exception', vector,
+ use_db=unique_database)
def test_udf_errors(self, vector, unique_database):
# Only run with codegen disabled to force interpretation path to be taken.