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 <[email protected]> Authored: Wed Aug 27 11:00:09 2014 +0200 Committer: Sylvain Lebresne <[email protected]> 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) );
