From 68be72fdc6dcf9f26a99f0bf82046c7939f43ead Mon Sep 17 00:00:00 2001 From: blerer Date: Wed, 7 Jan 2015 13:35:20 -0600 Subject: [PATCH] Fix case-sensitivity of index name on CREATE/DROP INDEX Patch by Benjamin Lerer; reviewed by Tyler Hobbs for CASSANDRA-8365 --- CHANGES.txt | 2 + pylib/cqlshlib/cql3handling.py | 7 +- .../org/apache/cassandra/cql3/CFName.java | 24 +---- src/java/org/apache/cassandra/cql3/Cql.g | 41 ++++--- .../org/apache/cassandra/cql3/IndexName.java | 26 +---- .../cassandra/cql3/KeyspaceElementName.java | 74 +++++++++++++ .../cql3/statements/CreateIndexStatement.java | 4 +- .../cql3/statements/UseStatement.java | 7 +- .../cql3/CreateIndexStatementTest.java | 101 ++++++++++++++++++ 9 files changed, 221 insertions(+), 65 deletions(-) create mode 100644 src/java/org/apache/cassandra/cql3/KeyspaceElementName.java create mode 100644 test/unit/org/apache/cassandra/cql3/CreateIndexStatementTest.java diff --git a/CHANGES.txt b/CHANGES.txt index e7a4ee5b77..58d94ed2e7 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,6 @@ 2.1.3 + * Fix case-sensitivity of index name on CREATE and DROP INDEX + statements (CASSANDRA-8365) * Better detection/logging for corruption in compressed sstables (CASSANDRA-8192) * Use the correct repairedAt value when closing writer (CASSANDRA-8570) * (cqlsh) Handle a schema mismatch being detected on startup (CASSANDRA-8512) diff --git a/pylib/cqlshlib/cql3handling.py b/pylib/cqlshlib/cql3handling.py index dbd3709157..b7b052afb4 100644 --- a/pylib/cqlshlib/cql3handling.py +++ b/pylib/cqlshlib/cql3handling.py @@ -992,7 +992,12 @@ def create_cf_composite_primary_key_comma_completer(ctxt, cass): return [','] syntax_rules += r''' - ::= "CREATE" "CUSTOM"? "INDEX" ("IF" "NOT" "EXISTS")? indexname=? "ON" + + ::= + | + | ; + + ::= "CREATE" "CUSTOM"? "INDEX" ("IF" "NOT" "EXISTS")? indexname=? "ON" cf= "(" ( col= | "keys(" col= ")" | diff --git a/src/java/org/apache/cassandra/cql3/CFName.java b/src/java/org/apache/cassandra/cql3/CFName.java index 8446a3535d..3f4a118b4b 100644 --- a/src/java/org/apache/cassandra/cql3/CFName.java +++ b/src/java/org/apache/cassandra/cql3/CFName.java @@ -17,31 +17,13 @@ */ package org.apache.cassandra.cql3; -import java.util.Locale; - -public class CFName +public class CFName extends KeyspaceElementName { - private String ksName; private String cfName; - public void setKeyspace(String ks, boolean keepCase) - { - ksName = keepCase ? ks : ks.toLowerCase(Locale.US); - } - public void setColumnFamily(String cf, boolean keepCase) { - cfName = keepCase ? cf : cf.toLowerCase(Locale.US); - } - - public boolean hasKeyspace() - { - return ksName != null; - } - - public String getKeyspace() - { - return ksName; + cfName = toInternalName(cf, keepCase); } public String getColumnFamily() @@ -52,6 +34,6 @@ public class CFName @Override public String toString() { - return (hasKeyspace() ? (ksName + ".") : "") + cfName; + return super.toString() + cfName; } } diff --git a/src/java/org/apache/cassandra/cql3/Cql.g b/src/java/org/apache/cassandra/cql3/Cql.g index 034813cba6..1fcdb717db 100644 --- a/src/java/org/apache/cassandra/cql3/Cql.g +++ b/src/java/org/apache/cassandra/cql3/Cql.g @@ -566,12 +566,13 @@ createIndexStatement returns [CreateIndexStatement expr] @init { IndexPropDefs props = new IndexPropDefs(); boolean ifNotExists = false; + IndexName name = new IndexName(); } : K_CREATE (K_CUSTOM { props.isCustom = true; })? K_INDEX (K_IF K_NOT K_EXISTS { ifNotExists = true; } )? - (idxName=IDENT)? K_ON cf=columnFamilyName '(' id=indexIdent ')' + (idxName[name])? K_ON cf=columnFamilyName '(' id=indexIdent ')' (K_USING cls=STRING_LITERAL { props.customClass = $cls.text; })? (K_WITH properties[props])? - { $expr = new CreateIndexStatement(cf, $idxName.text, id, props, ifNotExists); } + { $expr = new CreateIndexStatement(cf, name, id, props, ifNotExists); } ; indexIdent returns [IndexTarget.Raw id] @@ -832,34 +833,42 @@ ident returns [ColumnIdentifier id] // Keyspace & Column family names keyspaceName returns [String id] @init { CFName name = new CFName(); } - : cfOrKsName[name, true] { $id = name.getKeyspace(); } + : ksName[name] { $id = name.getKeyspace(); } ; indexName returns [IndexName name] @init { $name = new IndexName(); } - : (idxOrKsName[name, true] '.')? idxOrKsName[name, false] - ; - -idxOrKsName[IndexName name, boolean isKs] - : t=IDENT { if (isKs) $name.setKeyspace($t.text, false); else $name.setIndex($t.text, false); } - | t=QUOTED_NAME { if (isKs) $name.setKeyspace($t.text, true); else $name.setIndex($t.text, true); } - | k=unreserved_keyword { if (isKs) $name.setKeyspace(k, false); else $name.setIndex(k, false); } + : (ksName[name] '.')? idxName[name] ; columnFamilyName returns [CFName name] @init { $name = new CFName(); } - : (cfOrKsName[name, true] '.')? cfOrKsName[name, false] + : (ksName[name] '.')? cfName[name] ; userTypeName returns [UTName name] : (ks=ident '.')? ut=non_type_ident { return new UTName(ks, ut); } ; -cfOrKsName[CFName name, boolean isKs] - : t=IDENT { if (isKs) $name.setKeyspace($t.text, false); else $name.setColumnFamily($t.text, false); } - | t=QUOTED_NAME { if (isKs) $name.setKeyspace($t.text, true); else $name.setColumnFamily($t.text, true); } - | k=unreserved_keyword { if (isKs) $name.setKeyspace(k, false); else $name.setColumnFamily(k, false); } - | QMARK {addRecognitionError("Bind variables cannot be used for keyspace or table names");} +ksName[KeyspaceElementName name] + : t=IDENT { $name.setKeyspace($t.text, false);} + | t=QUOTED_NAME { $name.setKeyspace($t.text, true);} + | k=unreserved_keyword { $name.setKeyspace(k, false);} + | QMARK {addRecognitionError("Bind variables cannot be used for keyspace");} + ; + +cfName[CFName name] + : t=IDENT { $name.setColumnFamily($t.text, false); } + | t=QUOTED_NAME { $name.setColumnFamily($t.text, true); } + | k=unreserved_keyword { $name.setColumnFamily(k, false); } + | QMARK {addRecognitionError("Bind variables cannot be used for table names");} + ; + +idxName[IndexName name] + : t=IDENT { $name.setIndex($t.text, false); } + | t=QUOTED_NAME { $name.setIndex($t.text, true);} + | k=unreserved_keyword { $name.setIndex(k, false); } + | QMARK {addRecognitionError("Bind variables cannot be used for index names");} ; constant returns [Constants.Literal constant] diff --git a/src/java/org/apache/cassandra/cql3/IndexName.java b/src/java/org/apache/cassandra/cql3/IndexName.java index ded86e4c71..d7ff8ff6e6 100644 --- a/src/java/org/apache/cassandra/cql3/IndexName.java +++ b/src/java/org/apache/cassandra/cql3/IndexName.java @@ -17,31 +17,13 @@ */ package org.apache.cassandra.cql3; -import java.util.Locale; - -public class IndexName +public final class IndexName extends KeyspaceElementName { - private String ksName; private String idxName; - public void setKeyspace(String ks, boolean keepCase) - { - ksName = keepCase ? ks : ks.toLowerCase(Locale.US); - } - public void setIndex(String idx, boolean keepCase) { - idxName = keepCase ? idx : idx.toLowerCase(Locale.US); - } - - public boolean hasKeyspace() - { - return ksName != null; - } - - public String getKeyspace() - { - return ksName; + idxName = toInternalName(idx, keepCase); } public String getIdx() @@ -53,13 +35,13 @@ public class IndexName { CFName cfName = new CFName(); if (hasKeyspace()) - cfName.setKeyspace(ksName, true); + cfName.setKeyspace(getKeyspace(), true); return cfName; } @Override public String toString() { - return (hasKeyspace() ? (ksName + ".") : "") + idxName; + return super.toString() + idxName; } } diff --git a/src/java/org/apache/cassandra/cql3/KeyspaceElementName.java b/src/java/org/apache/cassandra/cql3/KeyspaceElementName.java new file mode 100644 index 0000000000..0a68997d01 --- /dev/null +++ b/src/java/org/apache/cassandra/cql3/KeyspaceElementName.java @@ -0,0 +1,74 @@ +/* + * 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 java.util.Locale; + +/** + * Base class for the names of the keyspace elements (e.g. table, index ...) + */ +abstract class KeyspaceElementName +{ + /** + * The keyspace name as stored internally. + */ + private String ksName; + + /** + * Sets the keyspace. + * + * @param ks the keyspace name + * @param keepCase true if the case must be kept, false otherwise. + */ + public final void setKeyspace(String ks, boolean keepCase) + { + ksName = toInternalName(ks, keepCase); + } + + /** + * Checks if the keyspace is specified. + * @return true if the keyspace is specified, false otherwise. + */ + public final boolean hasKeyspace() + { + return ksName != null; + } + + public final String getKeyspace() + { + return ksName; + } + + /** + * Converts the specified name into the name used internally. + * + * @param name the name + * @param keepCase true if the case must be kept, false otherwise. + * @return the name used internally. + */ + protected static String toInternalName(String name, boolean keepCase) + { + return keepCase ? name : name.toLowerCase(Locale.US); + } + + @Override + public String toString() + { + return hasKeyspace() ? (getKeyspace() + ".") : ""; + } +} diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java index ac19f5cfb0..778ec78cca 100644 --- a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java @@ -49,13 +49,13 @@ public class CreateIndexStatement extends SchemaAlteringStatement private final boolean ifNotExists; public CreateIndexStatement(CFName name, - String indexName, + IndexName indexName, IndexTarget.Raw target, IndexPropDefs properties, boolean ifNotExists) { super(name); - this.indexName = indexName; + this.indexName = indexName.getIdx(); this.rawTarget = target; this.properties = properties; this.ifNotExists = ifNotExists; diff --git a/src/java/org/apache/cassandra/cql3/statements/UseStatement.java b/src/java/org/apache/cassandra/cql3/statements/UseStatement.java index efda72dad4..fe3d51864a 100644 --- a/src/java/org/apache/cassandra/cql3/statements/UseStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/UseStatement.java @@ -59,9 +59,10 @@ public class UseStatement extends ParsedStatement implements CQLStatement return new ResultMessage.SetKeyspace(keyspace); } - public ResultMessage executeInternal(QueryState state, QueryOptions options) + public ResultMessage executeInternal(QueryState state, QueryOptions options) throws InvalidRequestException { - // Internal queries are exclusively on the system keyspace and 'use' is thus useless - throw new UnsupportedOperationException(); + // In production, internal queries are exclusively on the system keyspace and 'use' is thus useless + // but for some unit tests we need to set the keyspace (e.g. for tests with DROP INDEX) + return execute(state, options); } } diff --git a/test/unit/org/apache/cassandra/cql3/CreateIndexStatementTest.java b/test/unit/org/apache/cassandra/cql3/CreateIndexStatementTest.java new file mode 100644 index 0000000000..18e1be530d --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/CreateIndexStatementTest.java @@ -0,0 +1,101 @@ +/* + * 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 java.util.Locale; + +import org.apache.commons.lang.StringUtils; + +import org.junit.Test; + +public class CreateIndexStatementTest extends CQLTester +{ + @Test + public void testCreateAndDropIndex() throws Throwable + { + testCreateAndDropIndex("test", false); + testCreateAndDropIndex("test2", true); + } + + @Test + public void testCreateAndDropIndexWithQuotedIdentifier() throws Throwable + { + testCreateAndDropIndex("\"quoted_ident\"", false); + testCreateAndDropIndex("\"quoted_ident2\"", true); + } + + @Test + public void testCreateAndDropIndexWithCamelCaseIdentifier() throws Throwable + { + testCreateAndDropIndex("CamelCase", false); + testCreateAndDropIndex("CamelCase2", true); + } + + /** + * Test creating and dropping an index with the specified name. + * + * @param indexName the index name + * @param addKeyspaceOnDrop add the keyspace name in the drop statement + * @throws Throwable if an error occurs + */ + private void testCreateAndDropIndex(String indexName, boolean addKeyspaceOnDrop) throws Throwable + { + execute("USE system"); + assertInvalidMessage("Index '" + removeQuotes(indexName.toLowerCase(Locale.US)) + "' could not be found", "DROP INDEX " + indexName + ";"); + + createTable("CREATE TABLE %s (a int primary key, b int);"); + createIndex("CREATE INDEX " + indexName + " ON %s(b);"); + createIndex("CREATE INDEX IF NOT EXISTS " + indexName + " ON %s(b);"); + + assertInvalidMessage("Index already exists", "CREATE INDEX " + indexName + " ON %s(b)"); + + execute("INSERT INTO %s (a, b) values (?, ?);", 0, 0); + execute("INSERT INTO %s (a, b) values (?, ?);", 1, 1); + execute("INSERT INTO %s (a, b) values (?, ?);", 2, 2); + execute("INSERT INTO %s (a, b) values (?, ?);", 3, 1); + + assertRows(execute("SELECT * FROM %s where b = ?", 1), row(1, 1), row(3, 1)); + assertInvalidMessage("Index '" + removeQuotes(indexName.toLowerCase(Locale.US)) + "' could not be found in any of the tables of keyspace 'system'", "DROP INDEX " + indexName); + + if (addKeyspaceOnDrop) + { + dropIndex("DROP INDEX " + KEYSPACE + "." + indexName); + } + else + { + execute("USE " + KEYSPACE); + dropIndex("DROP INDEX " + indexName); + } + + assertInvalidMessage("No secondary indexes on the restricted columns support the provided operators", + "SELECT * FROM %s where b = ?", 1); + dropIndex("DROP INDEX IF EXISTS " + indexName); + assertInvalidMessage("Index '" + removeQuotes(indexName.toLowerCase(Locale.US)) + "' could not be found", "DROP INDEX " + indexName); + } + + /** + * Removes the quotes from the specified index name. + * + * @param indexName the index name from which the quotes must be removed. + * @return the unquoted index name. + */ + private static String removeQuotes(String indexName) + { + return StringUtils.remove(indexName, '\"'); + } +}