Return-Path: 
 <avro-commits-return-364-apmail-hadoop-avro-commits-archive=hadoop.apache.org@hadoop.apache.org>
Delivered-To: apmail-hadoop-avro-commits-archive@minotaur.apache.org
Received: (qmail 12395 invoked from network); 2 Feb 2010 06:52:53 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3)
  by minotaur.apache.org with SMTP; 2 Feb 2010 06:52:53 -0000
Received: (qmail 96109 invoked by uid 500); 2 Feb 2010 06:52:53 -0000
Delivered-To: apmail-hadoop-avro-commits-archive@hadoop.apache.org
Received: (qmail 96078 invoked by uid 500); 2 Feb 2010 06:52:52 -0000
Mailing-List: contact avro-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:avro-commits-help@hadoop.apache.org>
List-Unsubscribe: <mailto:avro-commits-unsubscribe@hadoop.apache.org>
List-Post: <mailto:avro-commits@hadoop.apache.org>
List-Id: <avro-commits.hadoop.apache.org>
Reply-To: avro-dev@hadoop.apache.org
Delivered-To: mailing list avro-commits@hadoop.apache.org
Received: (qmail 96068 invoked by uid 99); 2 Feb 2010 06:52:52 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 02 Feb 2010 06:52:52 +0000
X-ASF-Spam-Status: No, hits=-2000.0 required=10.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 02 Feb 2010 06:52:49 +0000
Received: by eris.apache.org (Postfix, from userid 65534)
	id EEC4523888EC; Tue,  2 Feb 2010 06:52:27 +0000 (UTC)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r905531 - in /hadoop/avro/trunk: CHANGES.txt
 lang/java/src/java/org/apache/avro/Schema.java
 lang/java/src/test/java/org/apache/avro/TestSchema.java
Date: Tue, 02 Feb 2010 06:52:17 -0000
To: avro-commits@hadoop.apache.org
From: philz@apache.org
X-Mailer: svnmailer-1.0.8
Message-Id: <20100202065227.EEC4523888EC@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

Author: philz
Date: Tue Feb  2 06:52:02 2010
New Revision: 905531

URL: http://svn.apache.org/viewvc?rev=905531&view=rev
Log:
AVRO-361. Specific Compiler fails to handle union with two fixed branches
(Scott Carey via philz)


Modified:
    hadoop/avro/trunk/CHANGES.txt
    hadoop/avro/trunk/lang/java/src/java/org/apache/avro/Schema.java
    hadoop/avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java

Modified: hadoop/avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/CHANGES.txt?rev=905531&r1=905530&r2=905531&view=diff
==============================================================================
--- hadoop/avro/trunk/CHANGES.txt (original)
+++ hadoop/avro/trunk/CHANGES.txt Tue Feb  2 06:52:02 2010
@@ -407,6 +407,8 @@
 
     AVRO-390. ResolvingDecoder does not handle default values for records well (thiru)
 
+    AVRO-361. Specific Compiler fails to handle union with two fixed branches (Scott Carey via philz)
+
 Avro 1.2.0 (14 October 2009)
 
   INCOMPATIBLE CHANGES

Modified: hadoop/avro/trunk/lang/java/src/java/org/apache/avro/Schema.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/lang/java/src/java/org/apache/avro/Schema.java?rev=905531&r1=905530&r2=905531&view=diff
==============================================================================
--- hadoop/avro/trunk/lang/java/src/java/org/apache/avro/Schema.java (original)
+++ hadoop/avro/trunk/lang/java/src/java/org/apache/avro/Schema.java Tue Feb  2 06:52:02 2010
@@ -610,13 +610,24 @@
       super(Type.UNION);
       this.types = types;
       int seen = 0;
+      Set<String> seenNames = new HashSet<String>();
       for (Schema type : types) {                 // check legality of union
         switch (type.getType()) {
         case UNION: 
           throw new AvroRuntimeException("Nested union: "+this);
         case RECORD:
-          if (type.getName() != null)
-            continue;
+        case FIXED:
+        case ENUM:
+          String fullname = type.getFullName();
+          if (fullname != null) {
+            if (seenNames.add(fullname)) {
+              continue;
+            } else {
+              throw new AvroRuntimeException("Duplicate name in union:" + fullname);
+            }
+          } else {
+            throw new AvroRuntimeException("Nameless Record, Fixed, or Enum in union:"+this);
+          }
         default:
           int mask = 1 << type.getType().ordinal();
           if ((seen & mask) != 0)

Modified: hadoop/avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java?rev=905531&r1=905530&r2=905531&view=diff
==============================================================================
--- hadoop/avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java (original)
+++ hadoop/avro/trunk/lang/java/src/test/java/org/apache/avro/TestSchema.java Tue Feb  2 06:52:02 2010
@@ -27,7 +27,10 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 
 import org.apache.avro.Schema.Type;
 import org.apache.avro.generic.GenericArray;
@@ -233,6 +236,61 @@
   }
 
   @Test
+  public void testComplexUnions() throws Exception {
+    // one of each unnamed type and two of named types
+    String partial = "[\"int\", \"long\", \"float\", \"double\", \"boolean\", \"bytes\"," +
+    " \"string\", {\"type\":\"array\", \"items\": \"long\"}," +
+    " {\"type\":\"map\", \"values\":\"long\"}";
+    String namedTypes = ", {\"type\":\"record\",\"name\":\"Foo\",\"fields\":[]}," +
+    " {\"type\":\"fixed\",\"name\":\"Bar\",\"size\": 1}," +
+    " {\"type\":\"enum\",\"name\":\"Baz\",\"symbols\": [\"X\"]}";
+    
+    String namedTypes2 = ", {\"type\":\"record\",\"name\":\"Foo2\",\"fields\":[]}," +
+    " {\"type\":\"fixed\",\"name\":\"Bar2\",\"size\": 1}," +
+    " {\"type\":\"enum\",\"name\":\"Baz2\",\"symbols\": [\"X\"]}";
+    
+    check(partial + namedTypes + "]", false);
+    check(partial + namedTypes + namedTypes2 + "]", false); 
+    checkParseError(partial + namedTypes + namedTypes + "]");
+    
+    // fail with two branches of the same unnamed type
+    checkUnionError(new Schema[] {Schema.create(Type.INT), Schema.create(Type.INT)});
+    checkUnionError(new Schema[] {Schema.create(Type.LONG), Schema.create(Type.LONG)});
+    checkUnionError(new Schema[] {Schema.create(Type.FLOAT), Schema.create(Type.FLOAT)});
+    checkUnionError(new Schema[] {Schema.create(Type.DOUBLE), Schema.create(Type.DOUBLE)});
+    checkUnionError(new Schema[] {Schema.create(Type.BOOLEAN), Schema.create(Type.BOOLEAN)});
+    checkUnionError(new Schema[] {Schema.create(Type.BYTES), Schema.create(Type.BYTES)});
+    checkUnionError(new Schema[] {Schema.create(Type.STRING), Schema.create(Type.STRING)});
+    checkUnionError(new Schema[] {Schema.createArray(Schema.create(Type.INT)), 
+        Schema.createArray(Schema.create(Type.INT))});
+    checkUnionError(new Schema[] {Schema.createMap(Schema.create(Type.INT)), 
+        Schema.createMap(Schema.create(Type.INT))});
+    
+    List<String> symbols = new ArrayList<String>();
+    symbols.add("NOTHING");
+    
+    // succeed with two branches of the same named type, if different names
+    buildUnion(new Schema[] {Schema.createRecord("Foo", null, "org.test", false),
+        Schema.createRecord("Foo2", null, "org.test", false)});
+    buildUnion(new Schema[] {Schema.createEnum("Bar", null, "org.test", symbols),
+        Schema.createEnum("Bar2", null, "org.test", symbols)});
+    buildUnion(new Schema[] {Schema.createFixed("Baz", null, "org.test", 2),
+        Schema.createFixed("Baz2", null, "org.test", 1)});
+
+    // fail with two branches of the same named type, but same names
+    checkUnionError(new Schema[] {Schema.createRecord("Foo", null, "org.test", false),
+        Schema.createRecord("Foo", null, "org.test", false)});
+    checkUnionError(new Schema[] {Schema.createEnum("Bar", null, "org.test", symbols),
+        Schema.createEnum("Bar", null, "org.test", symbols)});
+    checkUnionError(new Schema[] {Schema.createFixed("Baz", null, "org.test", 2),
+        Schema.createFixed("Baz", null, "org.test", 1)});
+    
+    Schema union = buildUnion(new Schema[] {Schema.create(Type.INT)});
+    // fail if creating a union of a union
+    checkUnionError(new Schema[] {union});
+  }
+  
+  @Test
   public void testComplexProp() throws Exception {
     String json = "{\"type\":\"null\", \"foo\": [0]}";
     Schema s = Schema.parse(json);
@@ -270,6 +328,21 @@
     fail("Should not have parsed: "+json);
   }
 
+  private static void checkUnionError(Schema[] branches) {
+    List<Schema> branchList = Arrays.asList(branches);
+    try {
+      Schema.createUnion(branchList);
+      fail("Union should not have constructed from: " + branchList);
+    } catch (AvroRuntimeException are) {
+      return;
+    }
+  }
+
+  private static Schema buildUnion(Schema[] branches) {
+    List<Schema> branchList = Arrays.asList(branches);
+    return Schema.createUnion(branchList);
+  }
+
   /**
    * Makes sure that "doc" tags are transcribed in the schemas.
    * Note that there are docs both for fields and for the records