You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hivemall.apache.org by ta...@apache.org on 2017/11/02 04:48:12 UTC

incubator-hivemall git commit: [HIVEMALL-157] Avoid Null Pointer Exception caused by uninitialized queue handler

Repository: incubator-hivemall
Updated Branches:
  refs/heads/master 7b9e6bae6 -> 3960cf2cd


[HIVEMALL-157] Avoid Null Pointer Exception caused by uninitialized queue handler

## What changes were proposed in this pull request?

Even though `to_ordered_list` allows (and ignores) NULL inputs, following query fails due to NPE:

```sql
select to_ordered_list(null, null)
```
> Null Pointer Exception

This PR fixes the problem; now, the function returns empty list in case that queue handler is uninitialized because of NULL inputs:

> []

## What type of PR is it?

Bug Fix

## What is the Jira issue?

https://issues.apache.org/jira/browse/HIVEMALL-157

## How was this patch tested?

Manually tested on local and EMR Hive

## Checklist

- [x] Did you apply source code formatter, i.e., `mvn formatter:format`, for your commit?
- [x] Did you run system tests on Hive (or Spark)?

Author: Takuya Kitazawa <k....@gmail.com>

Closes #124 from takuti/fix-to_ordered_list-npe.


Project: http://git-wip-us.apache.org/repos/asf/incubator-hivemall/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hivemall/commit/3960cf2c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hivemall/tree/3960cf2c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hivemall/diff/3960cf2c

Branch: refs/heads/master
Commit: 3960cf2cd591f1a371648a0576c2a80bbd50aba8
Parents: 7b9e6ba
Author: Takuya Kitazawa <k....@gmail.com>
Authored: Thu Nov 2 13:48:00 2017 +0900
Committer: Takuya Kitazawa <ta...@apache.org>
Committed: Thu Nov 2 13:48:00 2017 +0900

----------------------------------------------------------------------
 .../hivemall/tools/list/UDAFToOrderedList.java  | 15 ++++++--
 .../tools/list/UDAFToOrderedListTest.java       | 39 ++++++++++++++++++++
 2 files changed, 50 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-hivemall/blob/3960cf2c/core/src/main/java/hivemall/tools/list/UDAFToOrderedList.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/hivemall/tools/list/UDAFToOrderedList.java b/core/src/main/java/hivemall/tools/list/UDAFToOrderedList.java
index 52c521c..f17d1f4 100644
--- a/core/src/main/java/hivemall/tools/list/UDAFToOrderedList.java
+++ b/core/src/main/java/hivemall/tools/list/UDAFToOrderedList.java
@@ -311,11 +311,11 @@ public final class UDAFToOrderedList extends AbstractGenericUDAFResolver {
             QueueAggregationBuffer myagg = (QueueAggregationBuffer) agg;
 
             Pair<List<Object>, List<Object>> tuples = myagg.drainQueue();
-            List<Object> keyList = tuples.getKey();
-            List<Object> valueList = tuples.getValue();
-            if (valueList.isEmpty()) {
+            if (tuples == null) {
                 return null;
             }
+            List<Object> keyList = tuples.getKey();
+            List<Object> valueList = tuples.getValue();
 
             Object[] partialResult = new Object[4];
             partialResult[0] = valueList;
@@ -363,6 +363,9 @@ public final class UDAFToOrderedList extends AbstractGenericUDAFResolver {
                 throws HiveException {
             QueueAggregationBuffer myagg = (QueueAggregationBuffer) agg;
             Pair<List<Object>, List<Object>> tuples = myagg.drainQueue();
+            if (tuples == null) {
+                return null;
+            }
             return tuples.getValue();
         }
 
@@ -404,8 +407,12 @@ public final class UDAFToOrderedList extends AbstractGenericUDAFResolver {
                 }
             }
 
-            @Nonnull
+            @Nullable
             Pair<List<Object>, List<Object>> drainQueue() {
+                if (queueHandler == null) {
+                    return null;
+                }
+
                 int n = queueHandler.size();
                 final Object[] keys = new Object[n];
                 final Object[] values = new Object[n];

http://git-wip-us.apache.org/repos/asf/incubator-hivemall/blob/3960cf2c/core/src/test/java/hivemall/tools/list/UDAFToOrderedListTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/hivemall/tools/list/UDAFToOrderedListTest.java b/core/src/test/java/hivemall/tools/list/UDAFToOrderedListTest.java
index c3039d1..c7e9e70 100644
--- a/core/src/test/java/hivemall/tools/list/UDAFToOrderedListTest.java
+++ b/core/src/test/java/hivemall/tools/list/UDAFToOrderedListTest.java
@@ -339,4 +339,43 @@ public class UDAFToOrderedListTest {
         Assert.assertEquals("banana", res.get(1));
     }
 
+    @Test
+    public void testNullOnly() throws Exception {
+        ObjectInspector[] inputOIs = new ObjectInspector[] {PrimitiveObjectInspectorFactory.javaDoubleObjectInspector};
+
+        final String[] values = new String[] {null, null, null};
+
+        evaluator.init(GenericUDAFEvaluator.Mode.PARTIAL1, inputOIs);
+        evaluator.reset(agg);
+
+        for (int i = 0; i < values.length; i++) {
+            evaluator.iterate(agg, new Object[] {values[i]});
+        }
+
+        List<Object> res = evaluator.terminate(agg);
+
+        Assert.assertNull(res);
+    }
+
+    @Test
+    public void testNullMixed() throws Exception {
+        ObjectInspector[] inputOIs = new ObjectInspector[] {PrimitiveObjectInspectorFactory.javaDoubleObjectInspector};
+
+        final String[] values = new String[] {"banana", "apple", null, "candy"};
+
+        evaluator.init(GenericUDAFEvaluator.Mode.PARTIAL1, inputOIs);
+        evaluator.reset(agg);
+
+        for (int i = 0; i < values.length; i++) {
+            evaluator.iterate(agg, new Object[] {values[i]});
+        }
+
+        List<Object> res = evaluator.terminate(agg);
+
+        Assert.assertEquals(3, res.size());
+        Assert.assertEquals("apple", res.get(0));
+        Assert.assertEquals("banana", res.get(1));
+        Assert.assertEquals("candy", res.get(2));
+    }
+
 }