Merge pull request twitter#242 from rangadi/null_values_in_protobuf_t…

…o_pig Null values in protobuf to pig
kevinweil · Sep 12, 2012 · 6a026cc · 6a026cc
2 parents 8f7a09d + 869c69e
commit 6a026cc
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 25 deletions.
diff --git a/core/src/main/java/com/twitter/elephantbird/mapreduce/io/ProtobufConverter.java b/core/src/main/java/com/twitter/elephantbird/mapreduce/io/ProtobufConverter.java
@@ -15,7 +15,7 @@
 public class ProtobufConverter<M extends Message> implements BinaryConverter<M> {
   private static final Logger LOG = LoggerFactory.getLogger(ProtobufConverter.class);
 
-  private Message.Builder protoBuilder;
+  private Message defaultInstance;
   private TypeRef<M> typeRef;
 
   // limit the number of warnings in case of serialization errors.
@@ -53,10 +53,13 @@ public M fromBytes(byte[] messageBuffer) {
   @SuppressWarnings("unchecked")
   public M fromBytes(byte[] messageBuffer, int offset, int len) {
     try {
-      if (protoBuilder == null) {
-        protoBuilder = Protobufs.getMessageBuilder(typeRef.getRawClass());
+      if (defaultInstance == null) {
+        defaultInstance = Protobufs.getMessageBuilder(typeRef.getRawClass())
+                                   .getDefaultInstanceForType();
       }
-      return  (M) protoBuilder.clone().mergeFrom(messageBuffer, offset, len).build();
+      return (M) defaultInstance.newBuilderForType()
+                                .mergeFrom(messageBuffer, offset, len)
+                                .build();
     } catch (InvalidProtocolBufferException e) {
       logWarning("Invalid Protobuf exception while building " + typeRef.getRawClass().getName(), e);
     } catch(UninitializedMessageException ume) {

diff --git a/pig/src/main/java/com/twitter/elephantbird/pig/util/ProtobufToPig.java b/pig/src/main/java/com/twitter/elephantbird/pig/util/ProtobufToPig.java
@@ -69,7 +69,7 @@ public Tuple toTuple(Message msg) {
       // Walk through all the possible fields in the message.
       for (FieldDescriptor fieldDescriptor : msgDescriptor.getFields()) {
         // Get the set value, or the default value, or null.
-        Object fieldValue = getFieldValue(msg, fieldDescriptor);
+        Object fieldValue = msg.getField(fieldDescriptor);
 
         if (fieldDescriptor.getType() == FieldDescriptor.Type.MESSAGE) {
           tuple.set(curField++, messageToTuple(fieldDescriptor, fieldValue));
@@ -90,6 +90,10 @@ public Tuple toTuple(Message msg) {
    * on whether the field is a Message or a simple field.
    */
   public Object fieldToPig(FieldDescriptor fieldDescriptor, Object fieldValue) {
+    if (fieldValue == null) {
+      // protobufs unofficially ensures values are not null. just in case:
+      return null;
+    }
     if (fieldDescriptor.getType() == FieldDescriptor.Type.MESSAGE) {
       return messageToTuple(fieldDescriptor, fieldValue);
     } else {
@@ -106,6 +110,10 @@ public Object fieldToPig(FieldDescriptor fieldDescriptor, Object fieldValue) {
    */
   @SuppressWarnings("unchecked")
   protected Object messageToTuple(FieldDescriptor fieldDescriptor, Object fieldValue) {
+    if (fieldValue == null) {
+      // protobufs unofficially ensures values are not null. just in case:
+      return null;
+    }
     assert fieldDescriptor.getType() == FieldDescriptor.Type.MESSAGE : "messageToTuple called with field of type " + fieldDescriptor.getType();
 
     if (fieldDescriptor.isRepeated()) {
@@ -191,26 +199,6 @@ private Object coerceToPigTypes(FieldDescriptor fieldDescriptor, Object fieldVal
     return fieldValue;
   }
 
-  /**
-   * A utility function for getting the value of a field in a protobuf message.  It first tries the
-   * literal set value in the protobuf's field list.  If the value isn't set, and the field has a default
-   * value, it uses that.  Otherwise, it returns null.
-   * @param msg the protobuf message
-   * @param fieldDescriptor the descriptor object for the given field.
-   * @return the value of the field, or null if none can be assigned.
-   */
-  protected Object getFieldValue(Message msg, FieldDescriptor fieldDescriptor) {
-    Object o = null;
-    Map<FieldDescriptor, Object> setFields = msg.getAllFields();
-    if (setFields.containsKey(fieldDescriptor)) {
-      o = setFields.get(fieldDescriptor);
-    } else if (fieldDescriptor.hasDefaultValue()) {
-      o = fieldDescriptor.getDefaultValue();
-    }
-
-    return o;
-  }
-
   /**
    * Turn a generic message descriptor into a Schema.  Individual fields that are enums
    * are converted into their string equivalents.