cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [1/2] git commit: Add syntax to remove multiple elements from a map
Date Wed, 27 Aug 2014 09:06:13 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk 44fa12ec4 -> 14dbba9d1


Add syntax to remove multiple elements from a map

patch by blerer & slebresne; reviewed by iamaleksey for CASSANDRA-6599


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

Branch: refs/heads/trunk
Commit: 1be0bbf71355b3b6cbd24c441e51873376bf3911
Parents: 3d5f3a6
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Wed Aug 27 11:00:09 2014 +0200
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Wed Aug 27 11:00:09 2014 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../org/apache/cassandra/cql3/Operation.java    |  19 +-
 .../org/apache/cassandra/cql3/CQLTester.java    |  53 ++++-
 .../apache/cassandra/cql3/CollectionsTest.java  | 227 +++++++++++++++++++
 .../cassandra/cql3/MultiColumnRelationTest.java |   4 +-
 .../apache/cassandra/cql3/TupleTypeTest.java    |   2 +-
 .../apache/cassandra/cql3/UserTypesTest.java    |   2 -
 7 files changed, 286 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/1be0bbf7/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index b5c3a32..0f1adf0 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.1.1
+ * Add syntax to remove multiple elements from a map (CASSANDRA-6599)
  * Support non-equals conditions in lightweight transactions (CASSANDRA-6839)
  * Add IF [NOT] EXISTS to create/drop triggers (CASSANDRA-7606)
  * (cqlsh) Display the current logged-in user (CASSANDRA-7785)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1be0bbf7/src/java/org/apache/cassandra/cql3/Operation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/Operation.java b/src/java/org/apache/cassandra/cql3/Operation.java
index 103606f..ace466c 100644
--- a/src/java/org/apache/cassandra/cql3/Operation.java
+++ b/src/java/org/apache/cassandra/cql3/Operation.java
@@ -22,9 +22,7 @@ import java.nio.ByteBuffer;
 import org.apache.cassandra.config.ColumnDefinition;
 import org.apache.cassandra.db.ColumnFamily;
 import org.apache.cassandra.db.composites.Composite;
-import org.apache.cassandra.db.marshal.CollectionType;
-import org.apache.cassandra.db.marshal.CounterColumnType;
-import org.apache.cassandra.db.marshal.ListType;
+import org.apache.cassandra.db.marshal.*;
 import org.apache.cassandra.exceptions.InvalidRequestException;
 
 /**
@@ -291,23 +289,26 @@ public abstract class Operation
 
         public Operation prepare(String keyspace, ColumnDefinition receiver) throws InvalidRequestException
         {
-            Term v = value.prepare(keyspace, receiver);
-
             if (!(receiver.type instanceof CollectionType))
             {
                 if (!(receiver.type instanceof CounterColumnType))
                     throw new InvalidRequestException(String.format("Invalid operation (%s)
for non counter column %s", toString(receiver), receiver.name));
-                return new Constants.Substracter(receiver, v);
+                return new Constants.Substracter(receiver, value.prepare(keyspace, receiver));
             }
 
             switch (((CollectionType)receiver.type).kind)
             {
                 case LIST:
-                    return new Lists.Discarder(receiver, v);
+                    return new Lists.Discarder(receiver, value.prepare(keyspace, receiver));
                 case SET:
-                    return new Sets.Discarder(receiver, v);
+                    return new Sets.Discarder(receiver, value.prepare(keyspace, receiver));
                 case MAP:
-                    throw new InvalidRequestException(String.format("Invalid operation (%s)
for map column %s", toString(receiver), receiver));
+                    // The value for a map substraction is actually a set
+                    ColumnSpecification vr = new ColumnSpecification(receiver.ksName,
+                                                                     receiver.cfName,
+                                                                     receiver.name,
+                                                                     SetType.getInstance(((MapType)receiver.type).keys));
+                    return new Sets.Discarder(receiver, value.prepare(keyspace, vr));
             }
             throw new AssertionError();
         }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1be0bbf7/test/unit/org/apache/cassandra/cql3/CQLTester.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java
index cb32577..c8401d2 100644
--- a/test/unit/org/apache/cassandra/cql3/CQLTester.java
+++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java
@@ -43,6 +43,7 @@ import org.apache.cassandra.db.Keyspace;
 import org.apache.cassandra.db.marshal.*;
 import org.apache.cassandra.exceptions.*;
 import org.apache.cassandra.io.util.FileUtils;
+import org.apache.cassandra.serializers.TypeSerializer;
 import org.apache.cassandra.service.StorageService;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
@@ -142,6 +143,11 @@ public abstract class CQLTester
         }
     }
 
+    public boolean usePrepared()
+    {
+        return USE_PREPARED_VALUES;
+    }
+
     private static void removeAllSSTables(String ks, String table)
     {
         // clean up data directory which are stored as data directory/keyspace/data files
@@ -272,8 +278,8 @@ public abstract class CQLTester
                 ByteBuffer actualValue = actual.getBytes(column.name.toString());
 
                 if (!Objects.equal(expectedByteValue, actualValue))
-                    Assert.fail(String.format("Invalid value for row %d column %d (%s), expected
<%s> but got <%s>",
-                                              i, j, column.name, formatValue(expectedByteValue,
column.type), formatValue(actualValue, column.type)));
+                    Assert.fail(String.format("Invalid value for row %d column %d (%s of
type %s), expected <%s> but got <%s>",
+                                              i, j, column.name, column.type.asCQL3Type(),
formatValue(expectedByteValue, column.type), formatValue(actualValue, column.type)));
             }
         }
 
@@ -311,9 +317,28 @@ public abstract class CQLTester
         try
         {
             execute(query, values);
-            Assert.fail("Query should be invalid but no error was thrown. Query is: " + query);
+            String q = USE_PREPARED_VALUES
+                     ? query + " (values: " + formatAllValues(values) + ")"
+                     : replaceValues(query, values);
+            Assert.fail("Query should be invalid but no error was thrown. Query is: " + q);
         }
-        catch (SyntaxException | InvalidRequestException e)
+        catch (InvalidRequestException e)
+        {
+            // This is what we expect
+        }
+    }
+
+    protected void assertInvalidSyntax(String query, Object... values) throws Throwable
+    {
+        try
+        {
+            execute(query, values);
+            String q = USE_PREPARED_VALUES
+                     ? query + " (values: " + formatAllValues(values) + ")"
+                     : replaceValues(query, values);
+            Assert.fail("Query should have invalid syntax but no error was thrown. Query
is: " + q);
+        }
+        catch (SyntaxException e)
         {
             // This is what we expect
         }
@@ -447,7 +472,7 @@ public abstract class CQLTester
 
         // We need to reach inside collections for TupleValue. Besides, for some reason the
format
         // of collection that CollectionType.getString gives us is not at all 'CQL compatible'
-        if (value instanceof Collection)
+        if (value instanceof Collection || value instanceof Map)
         {
             StringBuilder sb = new StringBuilder();
             if (value instanceof List)
@@ -460,7 +485,7 @@ public abstract class CQLTester
                         sb.append(", ");
                     sb.append(formatForCQL(l.get(i)));
                 }
-                sb.append("[");
+                sb.append("]");
             }
             else if (value instanceof Set)
             {
@@ -520,7 +545,19 @@ public abstract class CQLTester
 
     private static String formatValue(ByteBuffer bb, AbstractType<?> type)
     {
-        return bb == null ? "null" : type.getString(bb);
+        if (bb == null)
+            return "null";
+
+        if (type instanceof CollectionType)
+        {
+            // CollectionType override getString() to use hexToBytes. We can't change that
+            // without breaking SSTable2json, but the serializer for collection have the
+            // right getString so using it directly instead.
+            TypeSerializer ser = type.getSerializer();
+            return ser.toString(ser.deserialize(bb));
+        }
+
+        return type.getString(bb);
     }
 
     protected Object tuple(Object...values)
@@ -544,7 +581,7 @@ public abstract class CQLTester
             throw new IllegalArgumentException();
 
         int size = values.length / 2;
-        Map m = new HashMap(size);
+        Map m = new LinkedHashMap(size);
         for (int i = 0; i < size; i++)
             m.put(values[2 * i], values[(2 * i) + 1]);
         return m;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1be0bbf7/test/unit/org/apache/cassandra/cql3/CollectionsTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/CollectionsTest.java b/test/unit/org/apache/cassandra/cql3/CollectionsTest.java
new file mode 100644
index 0000000..72a24f0
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/CollectionsTest.java
@@ -0,0 +1,227 @@
+/*
+ * 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;
+
+public class CollectionsTest extends CQLTester
+{
+    @Test
+    public void testMapBulkRemoval() throws Throwable
+    {
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, m map<text, text>)");
+
+        execute("INSERT INTO %s(k, m) VALUES (?, ?)", 0, map("k1", "v1", "k2", "v2", "k3",
"v3"));
+
+        assertRows(execute("SELECT * FROM %s"),
+            row(0, map("k1", "v1", "k2", "v2", "k3", "v3"))
+        );
+
+        execute("UPDATE %s SET m = m - ? WHERE k = ?", set("k2"), 0);
+
+        assertRows(execute("SELECT * FROM %s"),
+            row(0, map("k1", "v1", "k3", "v3"))
+        );
+
+        execute("UPDATE %s SET m = m + ?, m = m - ? WHERE k = ?", map("k4", "v4"), set("k3"),
0);
+
+        assertRows(execute("SELECT * FROM %s"),
+            row(0, map("k1", "v1", "k4", "v4"))
+        );
+    }
+
+    @Test
+    public void testInvalidCollectionsMix() throws Throwable
+    {
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, l list<text>, s set<text>,
m map<text, text>)");
+
+        // Note: we force the non-prepared form for some of those tests because a list and
a set
+        // have the same serialized format in practice and CQLTester don't validate that
the type
+        // of what's passed as a value in the prepared case, so the queries would work (which
is ok,
+        // CQLTester is just a "dumb" client).
+
+        assertInvalid("UPDATE %s SET l = l + { 'a', 'b' } WHERE k = 0");
+        assertInvalid("UPDATE %s SET l = l - { 'a', 'b' } WHERE k = 0");
+        // TODO: We should remove this 'if' once #7833 is resolved
+        if (!usePrepared())
+        {
+            assertInvalid("UPDATE %s SET l = l + ? WHERE k = 0", map("a", "b", "c", "d"));
+            assertInvalid("UPDATE %s SET l = l - ? WHERE k = 0", map("a", "b", "c", "d"));
+        }
+
+        assertInvalid("UPDATE %s SET s = s + [ 'a', 'b' ] WHERE k = 0");
+        assertInvalid("UPDATE %s SET s = s - [ 'a', 'b' ] WHERE k = 0");
+        // TODO: We should remove this 'if' once #7833 is resolved
+        if (!usePrepared())
+        {
+            assertInvalid("UPDATE %s SET s = s + ? WHERE k = 0", map("a", "b", "c", "d"));
+            assertInvalid("UPDATE %s SET s = s - ? WHERE k = 0", map("a", "b", "c", "d"));
+        }
+
+        assertInvalid("UPDATE %s SET m = m + ? WHERE k = 0", list("a", "b"));
+        assertInvalid("UPDATE %s SET m = m - [ 'a', 'b' ] WHERE k = 0");
+        assertInvalid("UPDATE %s SET m = m + ? WHERE k = 0", set("a", "b"));
+        // Note that we do allow substracting a set from a map, but not a map from a map
+        // TODO: We should remove this 'if' once #7833 is resolved
+        if (!usePrepared())
+            assertInvalid("UPDATE %s SET m = m - ? WHERE k = 0", map("a", "b", "c", "d"));
+    }
+
+    @Test
+    public void testSets() throws Throwable
+    {
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, s set<text>)");
+
+        execute("INSERT INTO %s(k, s) VALUES (0, ?)", set("v1", "v2", "v3", "v4"));
+
+        assertRows(execute("SELECT s FROM %s WHERE k = 0"),
+            row(set("v1", "v2", "v3", "v4"))
+        );
+
+        execute("DELETE s[?] FROM %s WHERE k = 0", "v1");
+
+        assertRows(execute("SELECT s FROM %s WHERE k = 0"),
+            row(set("v2", "v3", "v4"))
+        );
+
+        // Full overwrite
+        execute("UPDATE %s SET s = ? WHERE k = 0", set("v6", "v5"));
+
+        assertRows(execute("SELECT s FROM %s WHERE k = 0"),
+            row(set("v5", "v6"))
+        );
+
+        execute("UPDATE %s SET s = s + ? WHERE k = 0", set("v7"));
+
+        assertRows(execute("SELECT s FROM %s WHERE k = 0"),
+            row(set("v5", "v6", "v7"))
+        );
+
+        execute("UPDATE %s SET s = s - ? WHERE k = 0", set("v6", "v5"));
+
+        assertRows(execute("SELECT s FROM %s WHERE k = 0"),
+            row(set("v7"))
+        );
+
+        execute("DELETE s FROM %s WHERE k = 0");
+
+        assertRows(execute("SELECT s FROM %s WHERE k = 0"),
+            row((Object)null)
+        );
+    }
+
+    @Test
+    public void testMaps() throws Throwable
+    {
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, m map<text, int>)");
+
+        execute("INSERT INTO %s(k, m) VALUES (0, ?)", map("v1", 1, "v2", 2));
+
+        assertRows(execute("SELECT m FROM %s WHERE k = 0"),
+            row(map("v1", 1, "v2", 2))
+        );
+
+        execute("UPDATE %s SET m[?] = ?, m[?] = ? WHERE k = 0", "v3", 3, "v4", 4);
+
+        assertRows(execute("SELECT m FROM %s WHERE k = 0"),
+            row(map("v1", 1, "v2", 2, "v3", 3, "v4", 4))
+        );
+
+        execute("DELETE m[?] FROM %s WHERE k = 0", "v1");
+
+        assertRows(execute("SELECT m FROM %s WHERE k = 0"),
+            row(map("v2", 2, "v3", 3, "v4", 4))
+        );
+
+        // Full overwrite
+        execute("UPDATE %s SET m = ? WHERE k = 0", map("v6", 6, "v5", 5));
+
+        assertRows(execute("SELECT m FROM %s WHERE k = 0"),
+            row(map("v5", 5, "v6", 6))
+        );
+
+        execute("UPDATE %s SET m = m + ? WHERE k = 0", map("v7", 7));
+
+        assertRows(execute("SELECT m FROM %s WHERE k = 0"),
+            row(map("v5", 5, "v6", 6, "v7", 7))
+        );
+
+        // The empty map is parsed as an empty set (because we don't have enough info at
parsing
+        // time when we see a {}) and special cased later. This test checks this work properly
+        execute("UPDATE %s SET m = {} WHERE k = 0");
+
+        assertRows(execute("SELECT m FROM %s WHERE k = 0"),
+            row((Object)null)
+        );
+    }
+
+    @Test
+    public void testLists() throws Throwable
+    {
+        createTable("CREATE TABLE %s (k int PRIMARY KEY, l list<text>)");
+
+        execute("INSERT INTO %s(k, l) VALUES (0, ?)", list("v1", "v2", "v3"));
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row(list("v1", "v2", "v3"))
+        );
+
+        execute("DELETE l[?] FROM %s WHERE k = 0", 1);
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row(list("v1", "v3"))
+        );
+
+        execute("UPDATE %s SET l[?] = ? WHERE k = 0", 1, "v4");
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row(list("v1", "v4"))
+        );
+
+        // Full overwrite
+        execute("UPDATE %s SET l = ? WHERE k = 0", list("v6", "v5"));
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row(list("v6", "v5"))
+        );
+
+        execute("UPDATE %s SET l = l + ? WHERE k = 0", list("v7", "v8"));
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row(list("v6", "v5", "v7", "v8"))
+        );
+
+        execute("UPDATE %s SET l = ? + l WHERE k = 0", list("v9"));
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row(list("v9", "v6", "v5", "v7", "v8"))
+        );
+
+        execute("UPDATE %s SET l = l - ? WHERE k = 0", list("v5", "v8"));
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row(list("v9", "v6", "v7"))
+        );
+
+        execute("DELETE l FROM %s WHERE k = 0");
+
+        assertRows(execute("SELECT l FROM %s WHERE k = 0"),
+            row((Object)null)
+        );
+    }
+}

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1be0bbf7/test/unit/org/apache/cassandra/cql3/MultiColumnRelationTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/MultiColumnRelationTest.java b/test/unit/org/apache/cassandra/cql3/MultiColumnRelationTest.java
index daf0835..bfc6d2d 100644
--- a/test/unit/org/apache/cassandra/cql3/MultiColumnRelationTest.java
+++ b/test/unit/org/apache/cassandra/cql3/MultiColumnRelationTest.java
@@ -26,7 +26,7 @@ public class MultiColumnRelationTest extends CQLTester
     {
         createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))");
 
-        assertInvalid("SELECT * FROM %s WHERE () = (?, ?)", 1, 2);
+        assertInvalidSyntax("SELECT * FROM %s WHERE () = (?, ?)", 1, 2);
         assertInvalid("SELECT * FROM %s WHERE a = 0 AND (b) = (?) AND (b) > (?)", 0, 0);
         assertInvalid("SELECT * FROM %s WHERE a = 0 AND (b) > (?) AND (b) > (?)", 0,
1);
         assertInvalid("SELECT * FROM %s WHERE (a, b) = (?, ?)", 0, 0);
@@ -37,7 +37,7 @@ public class MultiColumnRelationTest extends CQLTester
     {
         createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c, d))");
 
-        assertInvalid("SELECT * FROM %s WHERE a = 0 AND (b, c) > ()");
+        assertInvalidSyntax("SELECT * FROM %s WHERE a = 0 AND (b, c) > ()");
         assertInvalid("SELECT * FROM %s WHERE a = 0 AND (b, c) > (?, ?, ?)", 1, 2, 3);
         assertInvalid("SELECT * FROM %s WHERE a = 0 AND (b, c) > (?, ?)", 1, null);
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1be0bbf7/test/unit/org/apache/cassandra/cql3/TupleTypeTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/TupleTypeTest.java b/test/unit/org/apache/cassandra/cql3/TupleTypeTest.java
index 84512a5..354a8f9 100644
--- a/test/unit/org/apache/cassandra/cql3/TupleTypeTest.java
+++ b/test/unit/org/apache/cassandra/cql3/TupleTypeTest.java
@@ -91,7 +91,7 @@ public class TupleTypeTest extends CQLTester
     {
         createTable("CREATE TABLE %s (k int PRIMARY KEY, t tuple<int, text, double>)");
 
-        assertInvalid("INSERT INTO %s (k, t) VALUES (0, ())");
+        assertInvalidSyntax("INSERT INTO %s (k, t) VALUES (0, ())");
         assertInvalid("INSERT INTO %s (k, t) VALUES (0, (2, 'foo', 3.1, 'bar'))");
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/1be0bbf7/test/unit/org/apache/cassandra/cql3/UserTypesTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/UserTypesTest.java b/test/unit/org/apache/cassandra/cql3/UserTypesTest.java
index c7f1851..0dfb6f7 100644
--- a/test/unit/org/apache/cassandra/cql3/UserTypesTest.java
+++ b/test/unit/org/apache/cassandra/cql3/UserTypesTest.java
@@ -41,14 +41,12 @@ public class UserTypesTest extends CQLTester
         execute("INSERT INTO %s(k, v) VALUES (?, {x:?})", 1, -104.99251);
         execute("UPDATE %s SET b = ? WHERE k = ?", true, 1);
 
-        System.out.println("-- First query");
         assertRows(execute("SELECT v.x FROM %s WHERE k = ? AND v = {x:?}", 1, -104.99251),
             row(-104.99251)
         );
 
         flush();
 
-        System.out.println("-- 2nd query");
         assertRows(execute("SELECT v.x FROM %s WHERE k = ? AND v = {x:?}", 1, -104.99251),
             row(-104.99251)
         );


Mime
View raw message