You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ha...@apache.org on 2015/08/19 19:25:23 UTC

hive git commit: HIVE-10697 : ObjectInspectorConvertors#UnionConvertor does a faulty conversion (Swarnim Kulkarni, reviewed by Hari Subramaniyan)

Repository: hive
Updated Branches:
  refs/heads/master c3c2bdacd -> 2688b6800


HIVE-10697 : ObjectInspectorConvertors#UnionConvertor does a faulty conversion (Swarnim Kulkarni, reviewed by Hari Subramaniyan)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/2688b680
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/2688b680
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/2688b680

Branch: refs/heads/master
Commit: 2688b6800f203ff5fd4b283e462594e9b4279975
Parents: c3c2bda
Author: Hari Subramaniyan <ha...@apache.org>
Authored: Wed Aug 19 10:25:14 2015 -0700
Committer: Hari Subramaniyan <ha...@apache.org>
Committed: Wed Aug 19 10:25:14 2015 -0700

----------------------------------------------------------------------
 .../ObjectInspectorConverters.java              | 31 +++----
 .../SettableUnionObjectInspector.java           |  4 +-
 .../StandardUnionObjectInspector.java           |  4 +-
 .../TestObjectInspectorConverters.java          | 89 +++++++++++++++++++-
 4 files changed, 108 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/2688b680/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java
----------------------------------------------------------------------
diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java b/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java
index 8ef8ce1..24b3d4e 100644
--- a/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java
+++ b/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorConverters.java
@@ -424,8 +424,9 @@ public final class ObjectInspectorConverters {
     UnionObjectInspector inputOI;
     SettableUnionObjectInspector outputOI;
 
-    List<? extends ObjectInspector> inputFields;
-    List<? extends ObjectInspector> outputFields;
+    // Object inspectors for the tags for the input and output unionss
+    List<? extends ObjectInspector> inputTagsOIs;
+    List<? extends ObjectInspector> outputTagsOIs;
 
     ArrayList<Converter> fieldConverters;
 
@@ -436,14 +437,14 @@ public final class ObjectInspectorConverters {
       if (inputOI instanceof UnionObjectInspector) {
         this.inputOI = (UnionObjectInspector)inputOI;
         this.outputOI = outputOI;
-        inputFields = this.inputOI.getObjectInspectors();
-        outputFields = outputOI.getObjectInspectors();
+        inputTagsOIs = this.inputOI.getObjectInspectors();
+        outputTagsOIs = outputOI.getObjectInspectors();
 
         // If the output has some extra fields, set them to NULL in convert().
-        int minFields = Math.min(inputFields.size(), outputFields.size());
+        int minFields = Math.min(inputTagsOIs.size(), outputTagsOIs.size());
         fieldConverters = new ArrayList<Converter>(minFields);
         for (int f = 0; f < minFields; f++) {
-          fieldConverters.add(getConverter(inputFields.get(f), outputFields.get(f)));
+          fieldConverters.add(getConverter(inputTagsOIs.get(f), outputTagsOIs.get(f)));
         }
 
         // Create an empty output object which will be populated when convert() is invoked.
@@ -461,18 +462,18 @@ public final class ObjectInspectorConverters {
         return null;
       }
 
-      int minFields = Math.min(inputFields.size(), outputFields.size());
-      // Convert the fields
-      for (int f = 0; f < minFields; f++) {
-        Object outputFieldValue = fieldConverters.get(f).convert(inputOI);
-        outputOI.addField(output, (ObjectInspector)outputFieldValue);
-      }
+      Object inputFieldValue = inputOI.getField(input);
+      Object inputFieldTag = inputOI.getTag(input);
+      Object outputFieldValue = null;
 
-      // set the extra fields to null
-      for (int f = minFields; f < outputFields.size(); f++) {
-        outputOI.addField(output, null);
+      int inputFieldTagIndex = ((Byte)inputFieldTag).intValue();
+
+      if (inputFieldTagIndex >= 0 && inputFieldTagIndex < fieldConverters.size()) {
+         outputFieldValue = fieldConverters.get(inputFieldTagIndex).convert(inputFieldValue);
       }
 
+      outputOI.addField(output, outputFieldValue);
+
       return output;
     }
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/2688b680/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/SettableUnionObjectInspector.java
----------------------------------------------------------------------
diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/SettableUnionObjectInspector.java b/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/SettableUnionObjectInspector.java
index a64aee0..564d8d6 100644
--- a/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/SettableUnionObjectInspector.java
+++ b/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/SettableUnionObjectInspector.java
@@ -29,6 +29,6 @@ public abstract class SettableUnionObjectInspector implements
   /* Create an empty object */
   public abstract Object create();
 
-  /* Add fields to the object */
-  public abstract Object addField(Object union, ObjectInspector oi);
+  /* Add field to the object */
+  public abstract Object addField(Object union, Object field);
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/2688b680/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardUnionObjectInspector.java
----------------------------------------------------------------------
diff --git a/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardUnionObjectInspector.java b/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardUnionObjectInspector.java
index d1b11e8..f26c9ec 100644
--- a/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardUnionObjectInspector.java
+++ b/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardUnionObjectInspector.java
@@ -124,9 +124,9 @@ public class StandardUnionObjectInspector extends SettableUnionObjectInspector {
   }
 
   @Override
-  public Object addField(Object union, ObjectInspector oi) {
+  public Object addField(Object union, Object field) {
     ArrayList<Object> a = (ArrayList<Object>) union;
-    a.add(oi);
+    a.add(field);
     return a;
   }
 

http://git-wip-us.apache.org/repos/asf/hive/blob/2688b680/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorConverters.java
----------------------------------------------------------------------
diff --git a/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorConverters.java b/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorConverters.java
index 1185283..dd18517 100644
--- a/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorConverters.java
+++ b/serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorConverters.java
@@ -17,6 +17,9 @@
  */
 package org.apache.hadoop.hive.serde2.objectinspector;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import junit.framework.TestCase;
 
 import org.apache.hadoop.hive.common.type.HiveDecimal;
@@ -168,6 +171,90 @@ public class TestObjectInspectorConverters extends TestCase {
           {(byte)'h', (byte)'i',(byte)'v',(byte)'e'}),
           baConverter.convert(new Text("hive")));
       assertEquals("BAConverter", null, baConverter.convert(null));
+
+      // Union
+      ArrayList<String> fieldNames = new ArrayList<String>();
+      fieldNames.add("firstInteger");
+      fieldNames.add("secondString");
+      fieldNames.add("thirdBoolean");
+      ArrayList<ObjectInspector> fieldObjectInspectors = new ArrayList<ObjectInspector>();
+      fieldObjectInspectors
+          .add(PrimitiveObjectInspectorFactory.javaIntObjectInspector);
+      fieldObjectInspectors
+          .add(PrimitiveObjectInspectorFactory.javaStringObjectInspector);
+      fieldObjectInspectors
+          .add(PrimitiveObjectInspectorFactory.javaBooleanObjectInspector);
+
+      ArrayList<String> fieldNames2 = new ArrayList<String>();
+      fieldNames2.add("firstString");
+      fieldNames2.add("secondInteger");
+      fieldNames2.add("thirdBoolean");
+      ArrayList<ObjectInspector> fieldObjectInspectors2 = new ArrayList<ObjectInspector>();
+      fieldObjectInspectors2
+          .add(PrimitiveObjectInspectorFactory.javaStringObjectInspector);
+      fieldObjectInspectors2
+          .add(PrimitiveObjectInspectorFactory.javaIntObjectInspector);
+      fieldObjectInspectors2
+          .add(PrimitiveObjectInspectorFactory.javaBooleanObjectInspector);
+
+      Converter unionConverter0 = ObjectInspectorConverters.getConverter(ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectors),
+          ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectors2));
+
+      Object convertedObject0 = unionConverter0.convert(new StandardUnionObjectInspector.StandardUnion((byte)0, 1));
+      List<String> expectedObject0 = new ArrayList<String>();
+      expectedObject0.add("1");
+
+      assertEquals(expectedObject0, convertedObject0);
+
+      Converter unionConverter1 = ObjectInspectorConverters.getConverter(ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectors),
+		  ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectors2));
+
+      Object convertedObject1 = unionConverter1.convert(new StandardUnionObjectInspector.StandardUnion((byte)1, "1"));
+      List<Integer> expectedObject1 = new ArrayList<Integer>();
+      expectedObject1.add(1);
+
+      assertEquals(expectedObject1, convertedObject1);
+
+      Converter unionConverter2 = ObjectInspectorConverters.getConverter(ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectors),
+          ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectors2));
+
+      Object convertedObject2 = unionConverter2.convert(new StandardUnionObjectInspector.StandardUnion((byte)2, true));
+      List<Boolean> expectedObject2 = new ArrayList<Boolean>();
+      expectedObject2.add(true);
+
+      assertEquals(expectedObject2, convertedObject2);
+
+      // Union (extra fields)
+      ArrayList<String> fieldNamesExtra = new ArrayList<String>();
+      fieldNamesExtra.add("firstInteger");
+      fieldNamesExtra.add("secondString");
+      fieldNamesExtra.add("thirdBoolean");
+      ArrayList<ObjectInspector> fieldObjectInspectorsExtra = new ArrayList<ObjectInspector>();
+      fieldObjectInspectorsExtra
+          .add(PrimitiveObjectInspectorFactory.javaIntObjectInspector);
+      fieldObjectInspectorsExtra
+          .add(PrimitiveObjectInspectorFactory.javaStringObjectInspector);
+      fieldObjectInspectorsExtra
+          .add(PrimitiveObjectInspectorFactory.javaBooleanObjectInspector);
+
+      ArrayList<String> fieldNamesExtra2 = new ArrayList<String>();
+      fieldNamesExtra2.add("firstString");
+      fieldNamesExtra2.add("secondInteger");
+      ArrayList<ObjectInspector> fieldObjectInspectorsExtra2 = new ArrayList<ObjectInspector>();
+      fieldObjectInspectorsExtra2
+          .add(PrimitiveObjectInspectorFactory.javaStringObjectInspector);
+      fieldObjectInspectorsExtra2
+          .add(PrimitiveObjectInspectorFactory.javaIntObjectInspector);
+
+      Converter unionConverterExtra = ObjectInspectorConverters.getConverter(ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectorsExtra),
+          ObjectInspectorFactory.getStandardUnionObjectInspector(fieldObjectInspectorsExtra2));
+
+      Object convertedObjectExtra = unionConverterExtra.convert(new StandardUnionObjectInspector.StandardUnion((byte)2, true));
+      List<Object> expectedObjectExtra = new ArrayList<Object>();
+      expectedObjectExtra.add(null);
+
+      assertEquals(expectedObjectExtra, convertedObjectExtra); // we should get back null
+
     } catch (Throwable e) {
       e.printStackTrace();
       throw e;
@@ -192,4 +279,4 @@ public class TestObjectInspectorConverters extends TestCase {
     VarcharTypeInfo vcParams = (VarcharTypeInfo) poi.getTypeInfo();
     assertEquals("varchar length doesn't match", 5, vcParams.getLength());
   }
-}
+}
\ No newline at end of file