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/llap Commit: 2688b6800f203ff5fd4b283e462594e9b4279975 Parents: c3c2bda Author: Hari Subramaniyan <harisan...@apache.org> Authored: Wed Aug 19 10:25:14 2015 -0700 Committer: Hari Subramaniyan <harisan...@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