You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by cu...@apache.org on 2009/11/11 22:11:52 UTC
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/
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")