---

skrulcik · vkedia · commit 1f598290d577 · 2017-08-22T11:31:14.000-07:00
yaml --- r: 8273 b: refs/heads/master c: be26690 h: refs/heads/master i: 8271: 16910a9
diff --git a/[refs] b/[refs]
@@ -1,5 +1,5 @@
 ---
-refs/heads/master: cec93a50abb6ebe241c9d61aee8d8835cbcf7d24
+refs/heads/master: be26690e2461ec271e16376a86afef64f3448b42
 refs/heads/travis: 47e4fee4fd5af9b2a8ce46f23c72ec95f9b195b2
 refs/heads/gh-pages: 983ad13eceefeee06f3300486563b47902cedb77
 refs/tags/0.0.9: 22f1839238f66c39e67ed4dfdcd273b1ae2e8444
diff --git a/trunk/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Mutation.java b/trunk/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Mutation.java
@@ -24,10 +24,12 @@
 
 import java.io.Serializable;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -188,9 +190,17 @@ public ValueBinder<WriteBuilder> set(String columnName) {
       return binder;
     }
 
+    /**
+     * Returns a newly created {@code Mutation} based on the contents of the {@code Builder}.
+     *
+     * @throws IllegalStateException if any duplicate columns are present. Duplicate detection is
+     * case-insensitive.
+     */
     public Mutation build() {
       checkBindingInProgress(false);
-      return new Mutation(table, operation, columns.build(), values.build(), null);
+      ImmutableList<String> columnNames = columns.build();
+      checkDuplicateColumns(columnNames);
+      return new Mutation(table, operation, columnNames, values.build(), null);
     }
 
     private void checkBindingInProgress(boolean expectInProgress) {
@@ -200,6 +210,17 @@ private void checkBindingInProgress(boolean expectInProgress) {
         throw new IllegalStateException("Incomplete binding for column " + currentColumn);
       }
     }
+
+    private void checkDuplicateColumns(ImmutableList<String> columnNames) {
+      Set<String> columnNameSet = new HashSet<>();
+      for (String columnName : columnNames) {
+        columnName = columnName.toLowerCase();
+        if (columnNameSet.contains(columnName)) {
+          throw new IllegalStateException("Duplicate column: " + columnName);
+        }
+        columnNameSet.add(columnName);
+      }
+    }
   }
 
   /** Returns the name of the table that this mutation will affect. */
@@ -247,9 +268,6 @@ public Map<String, Value> asMap() {
     LinkedHashMap<String, Value> map = new LinkedHashMap<>();
     for (int i = 0; i < columns.size(); ++i) {
       Value existing = map.put(columns.get(i), values.get(i));
-      if (existing != null) {
-        throw new IllegalStateException("Duplicate column: " + columns.get(i));
-      }
     }
     return Collections.unmodifiableMap(map);
   }
diff --git a/trunk/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MutationTest.java b/trunk/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MutationTest.java
@@ -124,13 +124,16 @@ public void replace() {
 
   @Test
   public void duplicateColumn() {
-    // The current implementation does not repeat validation performed by the server; duplicates
-    // are permitted but will later cause an operation failure.
-    Mutation m = Mutation.newInsertBuilder("T1").set("C1").to(true).set("C1").to(false).build();
-    assertThat(m.getOperation()).isEqualTo(Mutation.Op.INSERT);
-    assertThat(m.getColumns()).containsExactly("C1", "C1").inOrder();
-    assertThat(m.getValues()).containsExactly(Value.bool(true), Value.bool(false)).inOrder();
-    assertThat(m.toString()).isEqualTo("insert(T1{C1=true,C1=false})");
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectMessage("Duplicate column");
+    Mutation.newInsertBuilder("T1").set("C1").to(true).set("C1").to(false).build();
+  }
+
+  @Test
+  public void duplicateColumnCaseInsensitive() {
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectMessage("Duplicate column");
+    Mutation.newInsertBuilder("T1").set("C1").to(true).set("c1").to(false).build();
   }
 
   @Test
@@ -141,11 +144,6 @@ public void asMap() {
     m = Mutation.newInsertBuilder("T").set("C1").to(true).set("C2").to(1234).build();
     assertThat(m.asMap())
         .isEqualTo(ImmutableMap.of("C1", Value.bool(true), "C2", Value.int64(1234)));
-
-    m = Mutation.newInsertBuilder("T").set("C1").to(true).set("C1").to(false).build();
-    expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage("Duplicate column");
-    m.asMap();
   }
 
   @Test
