You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2014/08/11 18:23:54 UTC

[1/3] git commit: Fix validation when adding static columns

Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.1.0 4c387e46d -> ce96a2a12


Fix validation when adding static columns

patch by slebresne; reviewed by iamaleksey for CASSANDRA-7730


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/73b02d67
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/73b02d67
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/73b02d67

Branch: refs/heads/cassandra-2.1.0
Commit: 73b02d67c1fcea9a2f773251a0a525ec51b7477a
Parents: 2692c29
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Aug 11 17:44:14 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Mon Aug 11 17:45:21 2014 +0200

----------------------------------------------------------------------
 CHANGES.txt                                                 | 1 +
 .../cassandra/cql3/statements/AlterTableStatement.java      | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/73b02d67/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index d6976db..4a3e086 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.0.10
+ * Fix validation when adding static columns (CASSANDRA-7730)
  * (Thrift) fix range deletion of supercolumns (CASSANDRA-7733)
  * Fix potential AssertionError in RangeTombstoneList (CASSANDRA-7700)
  * Validate arguments of blobAs* functions (CASSANDRA-7707)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/73b02d67/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
index 85b3547..88f0de8 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
@@ -89,8 +89,13 @@ public class AlterTableStatement extends SchemaAlteringStatement
                 if (cfDef.isCompact)
                     throw new InvalidRequestException("Cannot add new column to a COMPACT STORAGE table");
 
-                if (isStatic && !cfDef.isComposite)
-                    throw new InvalidRequestException("Static columns are not allowed in COMPACT STORAGE tables");
+                if (isStatic)
+                {
+                    if (!cfDef.isComposite)
+                        throw new InvalidRequestException("Static columns are not allowed in COMPACT STORAGE tables");
+                    if (cfDef.clusteringColumns().isEmpty())
+                        throw new InvalidRequestException("Static columns are only useful (and thus allowed) if the table has at least one clustering column");
+                }
 
                 if (name != null)
                 {


[3/3] git commit: Merge branch 'cassandra-2.0' into cassandra-2.1.0

Posted by sl...@apache.org.
Merge branch 'cassandra-2.0' into cassandra-2.1.0

Conflicts:
	CHANGES.txt
	src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ce96a2a1
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ce96a2a1
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ce96a2a1

Branch: refs/heads/cassandra-2.1.0
Commit: ce96a2a120508ba0a31cc35e44905eae808f4be3
Parents: 4c387e4 bd0bb6d
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Aug 11 18:23:43 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Mon Aug 11 18:23:43 2014 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |  3 +
 .../cql3/statements/AlterTableStatement.java    | 23 +++++-
 .../apache/cassandra/cql3/AlterTableTest.java   | 86 ++++++++++++++++++++
 3 files changed, 110 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce96a2a1/CHANGES.txt
----------------------------------------------------------------------
diff --cc CHANGES.txt
index 910feb4,cd51e04..64b4d65
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@@ -1,11 -1,7 +1,14 @@@
 -2.0.10
 +2.1.0-rc6
 + * Include stress yaml example in release and deb (CASSANDRA-7717)
 + * workaround for netty issue causing corrupted data off the wire (CASSANDRA-7695)
 + * cqlsh DESC CLUSTER fails retrieving ring information (CASSANDRA-7687)
 + * Fix binding null values inside UDT (CASSANDRA-7685)
 + * Fix UDT field selection with empty fields (CASSANDRA-7670)
 + * Bogus deserialization of static cells from sstable (CASSANDRA-7684)
 +Merged from 2.0:
+  * Better error message when adding a collection with the same name
+    than a previously dropped one (CASSANDRA-6276)
+  * Fix validation when adding static columns (CASSANDRA-7730)
   * (Thrift) fix range deletion of supercolumns (CASSANDRA-7733)
   * Fix potential AssertionError in RangeTombstoneList (CASSANDRA-7700)
   * Validate arguments of blobAs* functions (CASSANDRA-7707)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce96a2a1/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
----------------------------------------------------------------------
diff --cc src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
index 273ee11,136c430..be28943
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
@@@ -85,16 -86,23 +85,23 @@@ public class AlterTableStatement extend
          switch (oType)
          {
              case ADD:
 -                if (cfDef.isCompact)
 +                if (cfm.comparator.isDense())
                      throw new InvalidRequestException("Cannot add new column to a COMPACT STORAGE table");
-                 if (isStatic && !cfm.comparator.isCompound())
-                     throw new InvalidRequestException("Static columns are not allowed in COMPACT STORAGE tables");
+ 
+                 if (isStatic)
+                 {
 -                    if (!cfDef.isComposite)
++                    if (!cfm.comparator.isCompound())
+                         throw new InvalidRequestException("Static columns are not allowed in COMPACT STORAGE tables");
 -                    if (cfDef.clusteringColumns().isEmpty())
++                    if (cfm.clusteringColumns().isEmpty())
+                         throw new InvalidRequestException("Static columns are only useful (and thus allowed) if the table has at least one clustering column");
+                 }
+ 
 -                if (name != null)
 +                if (def != null)
                  {
 -                    switch (name.kind)
 +                    switch (def.kind)
                      {
 -                        case KEY_ALIAS:
 -                        case COLUMN_ALIAS:
 +                        case PARTITION_KEY:
 +                        case CLUSTERING_COLUMN:
                              throw new InvalidRequestException(String.format("Invalid column name %s because it conflicts with a PRIMARY KEY part", columnName));
                          default:
                              throw new InvalidRequestException(String.format("Invalid column name %s because it conflicts with an existing column", columnName));
@@@ -104,18 -112,39 +111,30 @@@
                  AbstractType<?> type = validator.getType();
                  if (type instanceof CollectionType)
                  {
 -                    if (!cfDef.isComposite)
 +                    if (!cfm.comparator.supportCollections())
                          throw new InvalidRequestException("Cannot use collection types with non-composite PRIMARY KEY");
 -                    if (cfDef.cfm.isSuper())
 +                    if (cfm.isSuper())
                          throw new InvalidRequestException("Cannot use collection types with Super column family");
  
 -                    Map<ByteBuffer, CollectionType> collections = cfDef.hasCollections
 -                                                                ? new HashMap<ByteBuffer, CollectionType>(cfDef.getCollectionType().defined)
 -                                                                : new HashMap<ByteBuffer, CollectionType>();
+ 
+                     // If there used to be a collection column with the same name (that has been dropped), it will
+                     // still be appear in the ColumnToCollectionType because or reasons explained on #6276. The same
+                     // reason mean that we can't allow adding a new collection with that name (see the ticket for details).
 -                    CollectionType previous = collections.get(columnName.key);
 -                    if (previous != null && !type.isCompatibleWith(previous))
 -                        throw new InvalidRequestException(String.format("Cannot add a collection with the name %s " +
 -                                    "because a collection with the same name and a different type has already been used in the past", columnName));
++                    if (cfm.comparator.hasCollections())
++                    {
++                        CollectionType previous = cfm.comparator.collectionType() == null ? null : cfm.comparator.collectionType().defined.get(columnName.bytes);
++                        if (previous != null && !type.isCompatibleWith(previous))
++                            throw new InvalidRequestException(String.format("Cannot add a collection with the name %s " +
++                                        "because a collection with the same name and a different type has already been used in the past", columnName));
++                    }
+ 
 -                    collections.put(columnName.key, (CollectionType)type);
 -                    ColumnToCollectionType newColType = ColumnToCollectionType.getInstance(collections);
 -                    List<AbstractType<?>> ctypes = new ArrayList<AbstractType<?>>(((CompositeType)cfm.comparator).types);
 -                    if (cfDef.hasCollections)
 -                        ctypes.set(ctypes.size() - 1, newColType);
 -                    else
 -                        ctypes.add(newColType);
 -                    cfm.comparator = CompositeType.getInstance(ctypes);
 +                    cfm.comparator = cfm.comparator.addOrUpdateCollection(columnName, (CollectionType)type);
                  }
  
 -                Integer componentIndex = cfDef.isComposite
 -                                       ? ((CompositeType)meta.comparator).types.size() - (cfDef.hasCollections ? 2 : 1)
 -                                       : null;
 +                Integer componentIndex = cfm.comparator.isCompound() ? cfm.comparator.clusteringPrefixSize() : null;
                  cfm.addColumnDefinition(isStatic
 -                                        ? ColumnDefinition.staticDef(columnName.key, type, componentIndex)
 -                                        : ColumnDefinition.regularDef(columnName.key, type, componentIndex));
 +                                        ? ColumnDefinition.staticDef(cfm, columnName.bytes, type, componentIndex)
 +                                        : ColumnDefinition.regularDef(cfm, columnName.bytes, type, componentIndex));
                  break;
  
              case ALTER:

http://git-wip-us.apache.org/repos/asf/cassandra/blob/ce96a2a1/test/unit/org/apache/cassandra/cql3/AlterTableTest.java
----------------------------------------------------------------------
diff --cc test/unit/org/apache/cassandra/cql3/AlterTableTest.java
index 0000000,0000000..f5747ed
new file mode 100644
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/AlterTableTest.java
@@@ -1,0 -1,0 +1,86 @@@
++/*
++ * Licensed to the Apache Software Foundation (ASF) under one
++ * or more contributor license agreements.  See the NOTICE file
++ * distributed with this work for additional information
++ * regarding copyright ownership.  The ASF licenses this file
++ * to you under the Apache License, Version 2.0 (the
++ * "License"); you may not use this file except in compliance
++ * with the License.  You may obtain a copy of the License at
++ *
++ *     http://www.apache.org/licenses/LICENSE-2.0
++ *
++ * Unless required by applicable law or agreed to in writing, software
++ * distributed under the License is distributed on an "AS IS" BASIS,
++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
++ * See the License for the specific language governing permissions and
++ * limitations under the License.
++ */
++package org.apache.cassandra.cql3;
++
++import org.junit.Test;
++
++import org.apache.cassandra.exceptions.InvalidRequestException;
++
++public class AlterTableTest extends CQLTester
++{
++    @Test
++    public void testAddList() throws Throwable
++    {
++        createTable("CREATE TABLE %s (id text PRIMARY KEY, content text);");
++        execute("ALTER TABLE %s ADD myCollection list<text>;");
++        execute("INSERT INTO %s (id, content , myCollection) VALUES ('test', 'first test', ['first element']);");
++
++        assertRows(execute("SELECT * FROM %s;"), row("test", "first test", list("first element")));
++    }
++
++    @Test
++    public void testDropList() throws Throwable
++    {
++        createTable("CREATE TABLE %s (id text PRIMARY KEY, content text, myCollection list<text>);");
++        execute("INSERT INTO %s (id, content , myCollection) VALUES ('test', 'first test', ['first element']);");
++        execute("ALTER TABLE %s DROP myCollection;");
++
++        assertRows(execute("SELECT * FROM %s;"), row("test", "first test"));
++    }
++    @Test
++    public void testAddMap() throws Throwable
++    {
++        createTable("CREATE TABLE %s (id text PRIMARY KEY, content text);");
++        execute("ALTER TABLE %s ADD myCollection map<text, text>;");
++        execute("INSERT INTO %s (id, content , myCollection) VALUES ('test', 'first test', { '1' : 'first element'});");
++
++        assertRows(execute("SELECT * FROM %s;"), row("test", "first test", map("1", "first element")));
++    }
++
++    @Test
++    public void testDropMap() throws Throwable
++    {
++        createTable("CREATE TABLE %s (id text PRIMARY KEY, content text, myCollection map<text, text>);");
++        execute("INSERT INTO %s (id, content , myCollection) VALUES ('test', 'first test', { '1' : 'first element'});");
++        execute("ALTER TABLE %s DROP myCollection;");
++
++        assertRows(execute("SELECT * FROM %s;"), row("test", "first test"));
++    }
++
++    @Test
++    public void testDropListAndAddListWithSameName() throws Throwable
++    {
++        createTable("CREATE TABLE %s (id text PRIMARY KEY, content text, myCollection list<text>);");
++        execute("INSERT INTO %s (id, content , myCollection) VALUES ('test', 'first test', ['first element']);");
++        execute("ALTER TABLE %s DROP myCollection;");
++        execute("ALTER TABLE %s ADD myCollection list<text>;");
++
++        assertRows(execute("SELECT * FROM %s;"), row("test", "first test", null));
++        execute("UPDATE %s set myCollection = ['second element'] WHERE id = 'test';");
++        assertRows(execute("SELECT * FROM %s;"), row("test", "first test", list("second element")));
++    }
++    @Test
++    public void testDropListAndAddMapWithSameName() throws Throwable
++    {
++        createTable("CREATE TABLE %s (id text PRIMARY KEY, content text, myCollection list<text>);");
++        execute("INSERT INTO %s (id, content , myCollection) VALUES ('test', 'first test', ['first element']);");
++        execute("ALTER TABLE %s DROP myCollection;");
++
++        assertInvalid("ALTER TABLE %s ADD myCollection map<int, int>;");
++    }
++}


[2/3] git commit: Better error message when trying to add a collection with the same name than a previously dropped one

Posted by sl...@apache.org.
Better error message when trying to add a collection with the same name than a previously dropped one


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/bd0bb6df
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/bd0bb6df
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/bd0bb6df

Branch: refs/heads/cassandra-2.1.0
Commit: bd0bb6df4613588967ab0c67c268a231c112b321
Parents: 73b02d6
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon Aug 11 18:07:35 2014 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Mon Aug 11 18:07:35 2014 +0200

----------------------------------------------------------------------
 CHANGES.txt                                                  | 2 ++
 .../cassandra/cql3/statements/AlterTableStatement.java       | 8 ++++++++
 2 files changed, 10 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/bd0bb6df/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 4a3e086..cd51e04 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,6 @@
 2.0.10
+ * Better error message when adding a collection with the same name
+   than a previously dropped one (CASSANDRA-6276)
  * Fix validation when adding static columns (CASSANDRA-7730)
  * (Thrift) fix range deletion of supercolumns (CASSANDRA-7733)
  * Fix potential AssertionError in RangeTombstoneList (CASSANDRA-7700)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/bd0bb6df/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
index 88f0de8..136c430 100644
--- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java
@@ -121,6 +121,14 @@ public class AlterTableStatement extends SchemaAlteringStatement
                                                                 ? new HashMap<ByteBuffer, CollectionType>(cfDef.getCollectionType().defined)
                                                                 : new HashMap<ByteBuffer, CollectionType>();
 
+                    // If there used to be a collection column with the same name (that has been dropped), it will
+                    // still be appear in the ColumnToCollectionType because or reasons explained on #6276. The same
+                    // reason mean that we can't allow adding a new collection with that name (see the ticket for details).
+                    CollectionType previous = collections.get(columnName.key);
+                    if (previous != null && !type.isCompatibleWith(previous))
+                        throw new InvalidRequestException(String.format("Cannot add a collection with the name %s " +
+                                    "because a collection with the same name and a different type has already been used in the past", columnName));
+
                     collections.put(columnName.key, (CollectionType)type);
                     ColumnToCollectionType newColType = ColumnToCollectionType.getInstance(collections);
                     List<AbstractType<?>> ctypes = new ArrayList<AbstractType<?>>(((CompositeType)cfm.comparator).types);