cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tylerho...@apache.org
Subject [1/2] cassandra git commit: Fix case-sensitivity of index name on CREATE/DROP INDEX
Date Wed, 07 Jan 2015 19:36:49 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk ac27409e1 -> 34f9c97a5


Fix case-sensitivity of index name on CREATE/DROP INDEX

Patch by Benjamin Lerer; reviewed by Tyler Hobbs for CASSANDRA-8365


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

Branch: refs/heads/trunk
Commit: 68be72fdc6dcf9f26a99f0bf82046c7939f43ead
Parents: 905273e
Author: blerer <b_lerer@hotmail.com>
Authored: Wed Jan 7 13:35:20 2015 -0600
Committer: Tyler Hobbs <tyler@datastax.com>
Committed: Wed Jan 7 13:35:20 2015 -0600

----------------------------------------------------------------------
 CHANGES.txt                                     |   2 +
 pylib/cqlshlib/cql3handling.py                  |   7 +-
 src/java/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 +-
 .../cassandra/cql3/statements/UseStatement.java |   7 +-
 .../cql3/CreateIndexStatementTest.java          | 101 +++++++++++++++++++
 9 files changed, 221 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index e7a4ee5..58d94ed 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)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/pylib/cqlshlib/cql3handling.py
----------------------------------------------------------------------
diff --git a/pylib/cqlshlib/cql3handling.py b/pylib/cqlshlib/cql3handling.py
index dbd3709..b7b052a 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'''
-<createIndexStatement> ::= "CREATE" "CUSTOM"? "INDEX" ("IF" "NOT" "EXISTS")? indexname=<identifier>?
"ON"
+
+<idxName> ::= <identifier>
+            | <quotedName>
+            | <unreservedKeyword>;
+
+<createIndexStatement> ::= "CREATE" "CUSTOM"? "INDEX" ("IF" "NOT" "EXISTS")? indexname=<idxName>?
"ON"
                                cf=<columnFamilyName> "(" (
                                    col=<cident> |
                                    "keys(" col=<cident> ")" |

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/src/java/org/apache/cassandra/cql3/CFName.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/CFName.java b/src/java/org/apache/cassandra/cql3/CFName.java
index 8446a35..3f4a118 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;
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/src/java/org/apache/cassandra/cql3/Cql.g
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/Cql.g b/src/java/org/apache/cassandra/cql3/Cql.g
index 034813c..1fcdb71 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]

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/src/java/org/apache/cassandra/cql3/IndexName.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/IndexName.java b/src/java/org/apache/cassandra/cql3/IndexName.java
index ded86e4..d7ff8ff 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;
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/src/java/org/apache/cassandra/cql3/KeyspaceElementName.java
----------------------------------------------------------------------
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 0000000..0a68997
--- /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 <code>true</code> if the case must be kept, <code>false</code>
otherwise.
+     */
+    public final void setKeyspace(String ks, boolean keepCase)
+    {
+        ksName = toInternalName(ks, keepCase);
+    }
+
+    /**
+     * Checks if the keyspace is specified.
+     * @return <code>true</code> if the keyspace is specified, <code>false</code>
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 <code>true</code> if the case must be kept, <code>false</code>
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() + ".") : "";
+    }
+}

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateIndexStatement.java
index ac19f5c..778ec78 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;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/src/java/org/apache/cassandra/cql3/statements/UseStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/UseStatement.java b/src/java/org/apache/cassandra/cql3/statements/UseStatement.java
index efda72d..fe3d518 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);
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68be72fd/test/unit/org/apache/cassandra/cql3/CreateIndexStatementTest.java
----------------------------------------------------------------------
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 0000000..18e1be5
--- /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, '\"');
+    }
+}


Mime
View raw message