avro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cutt...@apache.org
Subject svn commit: r835070 - in /hadoop/avro/trunk: ./ src/java/org/apache/avro/ src/java/org/apache/avro/generic/ src/java/org/apache/avro/reflect/ src/java/org/apache/avro/specific/ src/test/java/org/apache/avro/
Date Wed, 11 Nov 2009 21:11:52 GMT
Author: cutting
Date: Wed Nov 11 21:11:50 2009
New Revision: 835070

URL: http://svn.apache.org/viewvc?rev=835070&view=rev
Log:
AVRO-182.  Fix Java's generic and specfic implementations of equals() and hashCode() to be
consistent with compareTo().

Modified:
    hadoop/avro/trunk/CHANGES.txt
    hadoop/avro/trunk/src/java/org/apache/avro/Schema.java
    hadoop/avro/trunk/src/java/org/apache/avro/generic/GenericData.java
    hadoop/avro/trunk/src/java/org/apache/avro/reflect/ReflectData.java
    hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificData.java
    hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificExceptionBase.java
    hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificRecordBase.java
    hadoop/avro/trunk/src/test/java/org/apache/avro/TestCompare.java

Modified: hadoop/avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/CHANGES.txt?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/CHANGES.txt (original)
+++ hadoop/avro/trunk/CHANGES.txt Wed Nov 11 21:11:50 2009
@@ -75,6 +75,10 @@
 
     AVRO-189. test-c target fails (massie)
 
+    AVRO-182. Fix Java's generic and specific implementations of
+    equals() and hashCode() to be consistent with compareTo().
+    (cutting)
+
 Avro 1.2.0 (14 October 2009)
 
   INCOMPATIBLE CHANGES

Modified: hadoop/avro/trunk/src/java/org/apache/avro/Schema.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/java/org/apache/avro/Schema.java?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/src/java/org/apache/avro/Schema.java (original)
+++ hadoop/avro/trunk/src/java/org/apache/avro/Schema.java Wed Nov 11 21:11:50 2009
@@ -168,6 +168,12 @@
     throw new AvroRuntimeException("Not a named type: "+this);
   }
 
+  /** If this is a record, enum or fixed, returns its namespace-qualified name,
+   * if any. */
+  public String getFullName() {
+    throw new AvroRuntimeException("Not a named type: "+this);
+  }
+
   /** Returns true if this record is an error type. */
   public boolean isError() {
     throw new AvroRuntimeException("Not a record: "+this);
@@ -264,10 +270,14 @@
   }
 
   private static class Name {
-    private String name;
-    private String space;
+    private final String name;
+    private final String space;
+    private final String full;
     public Name(String name, String space) {
-      if (name == null) return;                   // anonymous
+      if (name == null) {                         // anonymous
+        this.name = this.space = this.full = null;
+        return;
+      }
       int lastDot = name.lastIndexOf('.');
       if (lastDot < 0) {                          // unqualified name
         this.space = space;                       // use default space
@@ -276,20 +286,18 @@
         this.space = name.substring(0, lastDot);  // get space from name
         this.name = name.substring(lastDot+1, name.length());
       }
+      this.full = (this.space == null) ? this.name : this.space+"."+this.name;
     }
     public boolean equals(Object o) {
       if (o == this) return true;
       if (!(o instanceof Name)) return false;
       Name that = (Name)o;
-      return that == null ? false
-        : (name==null ? that.name==null : name.equals(that.name))
-        && (space==null ? that.space==null : space.equals(that.space));
+      return full==null ? that.full==null : full.equals(that.full);
     }
     public int hashCode() {
-      return (name==null ? 0 : name.hashCode())
-        + (space==null ? 0 : space.hashCode());
+      return full==null ? 0 : full.hashCode();
     }
-    public String toString() { return "name="+name + " namespace="+space; }
+    public String toString() { return full; }
     public void writeName(Names names, JsonGenerator gen) throws IOException {
       if (name != null) gen.writeStringField("name", name);
       if (space != null) {
@@ -309,14 +317,14 @@
     }
     public String getName() { return name.name; }
     public String getNamespace() { return name.space; }
+    public String getFullName() { return name.full; }
     public boolean writeNameRef(Names names, JsonGenerator gen)
       throws IOException {
       if (this.equals(names.get(name))) {
         if (name.space == null || name.space.equals(names.space()))
-          gen.writeString(name.name);
-        else {
-          gen.writeString(name.space+"."+name.name);
-        }
+          gen.writeString(name.name);             // in default namespace
+        else
+          gen.writeString(name.full);             // use fully-qualified name
         return true;
       } else if (name.name != null) {
         names.put(name, this);

Modified: hadoop/avro/trunk/src/java/org/apache/avro/generic/GenericData.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/java/org/apache/avro/generic/GenericData.java?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/src/java/org/apache/avro/generic/GenericData.java (original)
+++ hadoop/avro/trunk/src/java/org/apache/avro/generic/GenericData.java Wed Nov 11 21:11:50
2009
@@ -53,8 +53,19 @@
       this.schema = schema;
     }
     public Schema getSchema() { return schema; }
+    public boolean equals(Object o) {
+      if (o == this) return true;                 // identical object
+      if (!(o instanceof Record)) return false;   // not a record
+      Record that = (Record)o;
+      if (!schema.getFullName().equals(that.schema.getFullName()))
+        return false;                             // not the same schema
+      return this.compareTo(that) == 0;
+    }
+    public int hashCode() {
+      return GenericData.get().hashCode(this, schema);
+    }
     public int compareTo(Record that) {
-      return GenericData.get().compare(this, that, this.getSchema());
+      return GenericData.get().compare(this, that, schema);
     }
   }
 
@@ -94,23 +105,15 @@
       return (size < elements.length) ? (T)elements[size] : null;
     }
     public int hashCode() {
-      int hashCode = 1;
-      for (T e : this)
-        hashCode = 31*hashCode + (e==null ? 0 : e.hashCode());
-      return hashCode;
+      return GenericData.get().hashCode(this, schema);
     }
     public boolean equals(Object o) {
-      if (o == this) return true;
-      if (!(o instanceof GenericArray)) return false;
-      Iterator e1 = iterator();
-      Iterator e2 = ((GenericArray)o).iterator();
-      while(e1.hasNext() && e2.hasNext()) {
-        Object o1 = e1.next();
-        Object o2 = e2.next();
-        if (!(o1==null ? o2==null : o1.equals(o2)))
-          return false;
-      }
-      return !(e1.hasNext() || e2.hasNext());
+      if (o == this) return true;                 // identical object
+      if (!(o instanceof Array)) return false;    // not an array
+      Array that = (Array)o;
+      if (!schema.equals(that.schema))
+        return false;                             // not the same schema
+      return this.compareTo((Array)that) == 0;
     }
     public int compareTo(Array<T> that) {
       return GenericData.get().compare(this, that, this.getSchema());
@@ -392,12 +395,49 @@
     return datum instanceof ByteBuffer;
   }
 
+  /** Compute a hash code according to a schema, consistent with {@link
+   * #compare(Object,Object,Schema)}. */
+  @SuppressWarnings(value="unchecked")
+  public int hashCode(Object o, Schema s) {
+    int hashCode = 1;
+    switch (s.getType()) {
+    case RECORD:
+      GenericRecord r = (GenericRecord)o;
+      for (Map.Entry<String, Field> e : s.getFields().entrySet()) {
+        Field f = e.getValue();
+        if (f.order() == Field.Order.IGNORE)
+          continue;
+        String name = e.getKey();
+        hashCode = hashCodeAdd(hashCode, r.get(name), f.schema());
+      }
+      return hashCode;
+    case ARRAY:
+      GenericArray a = (GenericArray)o;
+      Schema elementType = a.getSchema().getElementType();
+      for (Object e : a)
+        hashCode = hashCodeAdd(hashCode, e, elementType);
+      return hashCode;
+    case UNION:
+      return hashCode(o, s.getTypes().get(resolveUnion(s, o)));
+    case NULL:
+      return 0;
+    default:
+      return o.hashCode();
+    }
+  }
+
+  /** Add the hash code for an object into an accumulated hash code. */
+  protected int hashCodeAdd(int hashCode, Object o, Schema s) {
+    return 31*hashCode + hashCode(o, s);
+  }
+
   /** Compare objects according to their schema.  If equal, return zero.  If
    * greater-than, return 1, if less than return -1.  Order is consistent with
    * that of {@link BinaryData#compare(byte[], int, byte[], int, Schema)}.
    */
   @SuppressWarnings(value="unchecked")
   public int compare(Object o1, Object o2, Schema s) {
+    if (o1 == o2) return 0;
     switch (s.getType()) {
     case RECORD:
       GenericRecord r1 = (GenericRecord)o1;
@@ -433,10 +473,11 @@
       return (i1 == i2)
         ? compare(o1, o2, s.getTypes().get(i1))
         : i1 - i2;
+    case NULL:
+      return 0;
     default:
       return ((Comparable)o1).compareTo(o2);
     }
   }
 
 }
-

Modified: hadoop/avro/trunk/src/java/org/apache/avro/reflect/ReflectData.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/java/org/apache/avro/reflect/ReflectData.java?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/src/java/org/apache/avro/reflect/ReflectData.java (original)
+++ hadoop/avro/trunk/src/java/org/apache/avro/reflect/ReflectData.java Wed Nov 11 21:11:50
2009
@@ -144,8 +144,7 @@
     }
   }
 
-  private Map<String,Map<String,Class>> classCache =
-    new ConcurrentHashMap<String,Map<String,Class>>();
+  private Map<String,Class> classCache = new ConcurrentHashMap<String,Class>();
 
   /** Return the class that implements this schema. */
   public Class getClass(Schema schema) {
@@ -153,18 +152,12 @@
     case FIXED:
     case RECORD:
     case ENUM:
-      String namespace = schema.getNamespace();
-      Map<String,Class> spaceCache = classCache.get(namespace);
-      if (spaceCache == null) {
-        spaceCache = new ConcurrentHashMap<String,Class>();
-        classCache.put(namespace, spaceCache);
-      }
-      String name = schema.getName();
-      Class c = spaceCache.get(name);
+      String full = schema.getFullName();
+      Class c = classCache.get(full);
       if (c == null) {
         try {
           c = Class.forName(getClassName(schema));
-          spaceCache.put(name, c);
+          classCache.put(full, c);
         } catch (ClassNotFoundException e) {
           throw new AvroRuntimeException(e);
         }

Modified: hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificData.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificData.java?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificData.java (original)
+++ hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificData.java Wed Nov 11 21:11:50
2009
@@ -69,6 +69,25 @@
   }
 
   @Override
+  public int hashCode(Object o, Schema s) {
+    switch (s.getType()) {
+    case RECORD:
+      int hashCode = 1;
+      SpecificRecord r = (SpecificRecord)o;
+      Iterator<Field> fields = s.getFields().values().iterator();
+      for (int i = 0; fields.hasNext(); i++) {
+        Field f = fields.next();
+        if (f.order() == Field.Order.IGNORE)
+          continue;
+        hashCode = hashCodeAdd(hashCode, r.get(i), f.schema());
+      }
+      return hashCode;
+    default:
+      return super.hashCode(o, s);
+    }
+  }
+
+  @Override
   public int compare(Object o1, Object o2, Schema s) {
     switch (s.getType()) {
     case RECORD:

Modified: hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificExceptionBase.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificExceptionBase.java?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificExceptionBase.java (original)
+++ hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificExceptionBase.java Wed Nov
11 21:11:50 2009
@@ -29,12 +29,15 @@
   public abstract Object get(int field);
   public abstract void set(int field, Object value);
 
-  public boolean equals(Object o) {
-    return SpecificRecordBase.equals(this, o);
+  public boolean equals(Object that) {
+    if (that == this) return true;                        // identical object
+    if (!(that instanceof SpecificExceptionBase)) return false; // not a record
+    if (this.getClass() != that.getClass()) return false; // not same schema
+    return SpecificData.get().compare(this, that, this.getSchema()) == 0;
   }
 
   public int hashCode() {
-    return SpecificRecordBase.hashCode(this);
+    return SpecificData.get().hashCode(this, this.getSchema());
   }
 
 }

Modified: hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificRecordBase.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificRecordBase.java?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificRecordBase.java (original)
+++ hadoop/avro/trunk/src/java/org/apache/avro/specific/SpecificRecordBase.java Wed Nov 11
21:11:50 2009
@@ -27,40 +27,15 @@
   public abstract Object get(int field);
   public abstract void set(int field, Object value);
 
-  public boolean equals(Object o) {
-    return SpecificRecordBase.equals(this, o);
+  public boolean equals(Object that) {
+    if (that == this) return true;                        // identical object
+    if (!(that instanceof SpecificRecord)) return false;  // not a record
+    if (this.getClass() != that.getClass()) return false; // not same schema
+    return this.compareTo((SpecificRecord)that) == 0;
   }
     
-  static boolean equals(SpecificRecord r1, Object o) {
-    if (r1 == o) return true;
-    if (!(o instanceof SpecificRecord)) return false;
-
-    SpecificRecord r2 = (SpecificRecord)o;
-    if (!r1.getSchema().equals(r2.getSchema())) return false;
-
-    int end = r1.getSchema().getFields().size();
-    for (int i = 0; i < end; i++) {
-      Object v1 = r1.get(i);
-      Object v2 = r2.get(i);
-      if (v1 == null) {
-        if (v2 != null) return false;
-      } else {
-        if (!v1.equals(v2)) return false;
-      }
-    }
-    return true;
-  }
-
   public int hashCode() {
-    return SpecificRecordBase.hashCode(this);
-  }
-
-  static int hashCode(SpecificRecord r) {
-    int result = 0;
-    int end = r.getSchema().getFields().size();
-    for (int i = 0; i < end; i++)
-      result += r.get(i).hashCode();
-    return result;
+    return SpecificData.get().hashCode(this, this.getSchema());
   }
 
   @SuppressWarnings(value="unchecked")

Modified: hadoop/avro/trunk/src/test/java/org/apache/avro/TestCompare.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/test/java/org/apache/avro/TestCompare.java?rev=835070&r1=835069&r2=835070&view=diff
==============================================================================
--- hadoop/avro/trunk/src/test/java/org/apache/avro/TestCompare.java (original)
+++ hadoop/avro/trunk/src/test/java/org/apache/avro/TestCompare.java Wed Nov 11 21:11:50 2009
@@ -109,10 +109,11 @@
 
   @Test
   public void testRecord() throws Exception {
-    String recordJson = "{\"type\":\"record\", \"name\":\"Test\", \"fields\":["
+    String fields = " \"fields\":["
       +"{\"name\":\"f\",\"type\":\"int\",\"order\":\"ignore\"},"
       +"{\"name\":\"g\",\"type\":\"int\",\"order\":\"descending\"},"
       +"{\"name\":\"h\",\"type\":\"int\"}]}";
+    String recordJson = "{\"type\":\"record\", \"name\":\"Test\","+fields;
     Schema schema = Schema.parse(recordJson);
     GenericData.Record r1 = new GenericData.Record(schema);
     r1.put("f", 1);
@@ -127,6 +128,14 @@
     r2.put("g", 13);
     r2.put("h", 42);
     check(recordJson, r1, r2);
+
+    String record2Json = "{\"type\":\"record\", \"name\":\"Test2\","+fields;
+    Schema schema2 = Schema.parse(record2Json);
+    GenericData.Record r3= new GenericData.Record(schema2);
+    r3.put("f", 1);
+    r3.put("g", 13);
+    r3.put("h", 41);
+    assert(!r1.equals(r3));                       // same fields, diff name
   }
 
   @Test
@@ -199,6 +208,17 @@
     assertEquals(1, compare(o2, o1, schema, comparable, comparator));
     assertEquals(0, compare(o1, o1, schema, comparable, comparator));
     assertEquals(0, compare(o2, o2, schema, comparable, comparator));
+
+    assert(o1.equals(o1));
+    assert(o2.equals(o2));
+    assert(!o1.equals(o2));
+    assert(!o2.equals(o1));
+    assert(!o1.equals(new Object()));
+    assert(!o2.equals(new Object()));
+    assert(!o1.equals(null));
+    assert(!o2.equals(null));
+
+    assert(o1.hashCode() != o2.hashCode());
   }
 
   @SuppressWarnings(value="unchecked")



Mime
View raw message