apache · olim7t · Mar 3, 2016 · Feb 2, 2016 · Mar 1, 2016 · Mar 1, 2016
diff --git a/changelog/README.md b/changelog/README.md
@@ -25,6 +25,7 @@
 - [bug] JAVA-1100: Exception when connecting with shaded java driver in OSGI
 - [bug] JAVA-819: Expose more errors in RetryPolicy + provide idempotent-aware wrapper.
 - [improvement] JAVA-1040: SimpleStatement parameters support in QueryLogger.
+- [bug] JAVA-1064: getTable create statement doesn't properly handle quotes in primary key.
 
 Merged from 2.0 branch:
 

diff --git a/driver-core/src/main/java/com/datastax/driver/core/Metadata.java b/driver-core/src/main/java/com/datastax/driver/core/Metadata.java
@@ -47,6 +47,15 @@ public class Metadata {
     private static final Pattern alphanumeric = Pattern.compile("\\w+"); // this includes _
     private static final Pattern lowercaseAlphanumeric = Pattern.compile("[a-z][a-z0-9_]*");
 
+    private static final Set<String> RESERVED_KEYWORDS = ImmutableSet.of(
+            "add", "allow", "alter", "and", "any", "apply", "asc", "authorize", "batch", "begin", "by",
+            "columnfamily", "create", "delete", "desc", "drop", "each_quorum", "from", "grant", "in",
+            "index", "inet", "infinity", "insert", "into", "keyspace", "keyspaces", "limit", "local_one",
+            "local_quorum", "modify", "nan", "norecursive", "of", "on", "one", "order", "password",
+            "primary", "quorum", "rename", "revoke", "schema", "select", "set", "table", "to",
+            "token", "three", "truncate", "two", "unlogged", "update", "use", "using", "where", "with"
+    );
+
     Metadata(Cluster.Manager cluster) {
         this.cluster = cluster;
     }
@@ -404,7 +413,7 @@ static String handleId(String id) {
             return id.toLowerCase();
 
         // Check if it's enclosed in quotes. If it is, remove them and unescape internal double quotes
-        if (id.charAt(0) == '"' && id.charAt(id.length() - 1) == '"')
+        if (!id.isEmpty() && id.charAt(0) == '"' && id.charAt(id.length() - 1) == '"')
             return id.substring(1, id.length() - 1).replaceAll("\"\"", "\"");
 
         // Otherwise, just return the id.
@@ -418,8 +427,11 @@ static String handleId(String id) {
     // tables. Because it comes from Cassandra, we could just always quote it,
     // but to get a nicer output we don't do it if it's not necessary.
     static String escapeId(String ident) {
-        // we don't need to escape if it's lowercase and match non-quoted CQL3 ids.
-        return lowercaseAlphanumeric.matcher(ident).matches() ? ident : quote(ident);
+        // we don't need to escape if it's lowercase and match non-quoted CQL3 ids,
+        // and if it's not a CQL reserved keyword
+        return lowercaseAlphanumeric.matcher(ident).matches()
+                && !isReservedCqlKeyword(ident) ?
+                ident : quote(ident);
     }
 
     /**
@@ -430,16 +442,40 @@ static String escapeId(String ident) {
      * the identifier in double quotes (see the
      * <a href="http://cassandra.apache.org/doc/cql3/CQL.html#identifiers">CQL documentation</a>
      * for details). If you are using case sensitive identifiers, this method
-     * can be used to enclose such identifier in double quotes, making it case
+     * can be used to enclose such identifiers in double quotes, making them case
      * sensitive.
+     * <p/>
+     * Note that
+     * <a href="https://docs.datastax.com/en/cql/3.0/cql/cql_reference/keywords_r.html">reserved CQL keywords</a>
+     * should also be quoted. You can check if a given identifier is a reserved keyword
+     * by calling {@link #isReservedCqlKeyword(String)}.
      *
      * @param id the keyspace or table identifier.
      * @return {@code id} enclosed in double-quotes, for use in methods like
      * {@link #getReplicas}, {@link #getKeyspace}, {@link KeyspaceMetadata#getTable}
      * or even {@link Cluster#connect(String)}.
      */
     public static String quote(String id) {
-        return '"' + id + '"';
+        return '"' + id.replace("\"", "\"\"") + '"';
+    }
+
+    /**
+     * Checks whether an identifier is a known reserved CQL keyword or not.
+     * <p/>
+     * The check is case-insensitive, i.e., the word "{@code KeYsPaCe}"
+     * would be considered as a reserved CQL keyword just as "{@code keyspace}".
+     * <p/>
+     * Note: The list of reserved CQL keywords is subject to change in future
+     * versions of Cassandra. As a consequence, this method is provided solely as a
+     * convenience utility and should not be considered as an authoritative
+     * source of truth for checking reserved CQL keywords.
+     *
+     * @param id the identifier to check; should not be {@code null}.
+     * @return {@code true} if the given identifier is a known reserved
+     * CQL keyword, {@code false} otherwise.
+     */
+    public static boolean isReservedCqlKeyword(String id) {
+        return id != null && RESERVED_KEYWORDS.contains(id.toLowerCase());
     }
 
     /**

diff --git a/driver-core/src/main/java/com/datastax/driver/core/TableMetadata.java b/driver-core/src/main/java/com/datastax/driver/core/TableMetadata.java
@@ -435,12 +435,12 @@ private String asCQLQuery(boolean formatted) {
         sb.append("CREATE TABLE ").append(Metadata.escapeId(keyspace.getName())).append('.').append(Metadata.escapeId(name)).append(" (");
         newLine(sb, formatted);
         for (ColumnMetadata cm : columns.values())
-            newLine(sb.append(spaces(4, formatted)).append(cm).append(','), formatted);
+            newLine(sb.append(spaces(4, formatted)).append(cm).append(',').append(spaces(1, !formatted)), formatted);
 
         // PK
         sb.append(spaces(4, formatted)).append("PRIMARY KEY (");
         if (partitionKey.size() == 1) {
-            sb.append(partitionKey.get(0).getName());
+            sb.append(Metadata.escapeId(partitionKey.get(0).getName()));
         } else {
             sb.append('(');
             boolean first = true;

diff --git a/driver-core/src/main/java/com/datastax/driver/core/querybuilder/QueryBuilder.java b/driver-core/src/main/java/com/datastax/driver/core/querybuilder/QueryBuilder.java
@@ -15,6 +15,7 @@
  */
 package com.datastax.driver.core.querybuilder;
 
+import com.datastax.driver.core.Metadata;
 import com.datastax.driver.core.RegularStatement;
 import com.datastax.driver.core.TableMetadata;
 import com.datastax.driver.core.exceptions.InvalidQueryException;
@@ -226,9 +227,10 @@ public static Truncate truncate(TableMetadata table) {
      *
      * @param columnName the column name to quote.
      * @return the quoted column name.
+     * @see Metadata#quote(String)
      */
     public static String quote(String columnName) {
-        return '"' + columnName + '"';
+        return Metadata.quote(columnName);
     }
 
     /**

diff --git a/driver-core/src/main/java/com/datastax/driver/core/schemabuilder/SchemaStatement.java b/driver-core/src/main/java/com/datastax/driver/core/schemabuilder/SchemaStatement.java
@@ -15,13 +15,12 @@
  */
 package com.datastax.driver.core.schemabuilder;
 
+import com.datastax.driver.core.Metadata;
 import com.datastax.driver.core.ProtocolVersion;
 import com.datastax.driver.core.RegularStatement;
 import com.google.common.base.Strings;
 
 import java.nio.ByteBuffer;
-import java.util.Arrays;
-import java.util.List;
 
 /**
  * A DDL statement generated by {@link SchemaBuilder}.
@@ -31,12 +30,6 @@ public abstract class SchemaStatement extends RegularStatement {
     static final String STATEMENT_START = "\n\t";
     static final String COLUMN_FORMATTING = "\n\t\t";
 
-    static final List<String> RESERVED_KEYWORDS = Arrays.asList(
-            ("add,allow,alter,and,any,apply,asc,authorize,batch,begin,by,columnfamily,create,delete,desc,drop,each_quorum,from,grant,in,index,inet,infinity," +
-                    "insert,into,keyspace,keyspaces,limit,local_one,local_quorum,modify,nan,norecursive,of,on,order,password,primary,quorum,rename,revoke,schema," +
-                    "select,set,table,three,to,token,truncate,two,unlogged,update,use,using,where,with")
-                    .split(","));
-
     private volatile String cache;
 
     abstract String buildInternal();
@@ -85,7 +78,7 @@ static void validateNotNull(Object value, String label) {
     }
 
     static void validateNotKeyWord(String label, String message) {
-        if (RESERVED_KEYWORDS.contains(label)) {
+        if (Metadata.isReservedCqlKeyword(label)) {
             throw new IllegalArgumentException(message);
         }
     }

diff --git a/driver-core/src/test/java/com/datastax/driver/core/TableMetadataTest.java b/driver-core/src/test/java/com/datastax/driver/core/TableMetadataTest.java
@@ -17,7 +17,7 @@
 
 import org.testng.annotations.Test;
 
-import static org.assertj.core.api.Assertions.assertThat;
+import static com.datastax.driver.core.Assertions.assertThat;
 
 @CCMConfig(clusterProvider = "createClusterBuilderNoDebouncing")
 public class TableMetadataTest extends CCMTestsSupport {
@@ -35,4 +35,76 @@ public void should_escape_single_quote_table_comment() {
         TableMetadata table = cluster().getMetadata().getKeyspace(keyspace).getTable("single_quote");
         assertThat(table.asCQLQuery()).contains("comment with single quote '' should work");
     }
+
+    /**
+     * Validates that a table with case-sensitive column names and column names
+     * consisting of (quoted) reserved keywords is correctly parsed
+     * and that the generated CQL is valid.
+     *
+     * @jira_ticket JAVA-1064
+     * @test_category metadata
+     */
+    @Test(groups = "short")
+    public void should_parse_table_with_case_sensitive_column_names_and_reserved_keywords() throws Exception {
+        // given
+        String c1 = Metadata.quote("quotes go \"\" here \"\" ");
+        String c2 = Metadata.quote("\\x00\\x25");
+        String c3 = Metadata.quote("columnfamily");
+        String c4 = Metadata.quote("select");
+        String c5 = Metadata.quote("who''s there'? ");
+        String c6 = Metadata.quote("faux )");
+        String c7 = Metadata.quote("COMPACT STORAGE");
+        // single partition key
+        String cql1 = String.format("CREATE TABLE %s.\"MyTable1\" ("
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "PRIMARY KEY (%s, %s, %s, %s, %s, %s)"
+                + ")", keyspace, c1, c2, c3, c4, c5, c6, c7, c1, c2, c3, c4, c5, c6);
+        // composite partition key
+        String cql2 = String.format("CREATE TABLE %s.\"MyTable2\" ("
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "%s text, "
+                + "PRIMARY KEY ((%s, %s), %s, %s, %s, %s)"
+                + ")", keyspace, c1, c2, c3, c4, c5, c6, c7, c1, c2, c3, c4, c5, c6);
+        // when
+        execute(cql1, cql2);
+        TableMetadata table1 = cluster().getMetadata().getKeyspace(keyspace).getTable("\"MyTable1\"");
+        TableMetadata table2 = cluster().getMetadata().getKeyspace(keyspace).getTable("\"MyTable2\"");
+        // then
+        assertThat(table1)
+                .hasColumn(c1)
+                .hasColumn(c2)
+                .hasColumn(c3)
+                .hasColumn(c4)
+                .hasColumn(c5)
+                .hasColumn(c6)
+                .hasColumn(c7);
+        assertThat(table1.asCQLQuery()).startsWith(cql1);
+        assertThat(table2)
+                .hasColumn(c1)
+                .hasColumn(c2)
+                .hasColumn(c3)
+                .hasColumn(c4)
+                .hasColumn(c5)
+                .hasColumn(c6)
+                .hasColumn(c7);
+        assertThat(table2.asCQLQuery()).startsWith(cql2);
+        execute(
+                "DROP TABLE \"MyTable1\"",
+                "DROP TABLE \"MyTable2\"",
+                table1.asCQLQuery(),
+                table2.asCQLQuery()
+        );
+    }
+
 }
diff --git a/driver-core/src/test/java/com/datastax/driver/core/querybuilder/QueryBuilderTest.java b/driver-core/src/test/java/com/datastax/driver/core/querybuilder/QueryBuilderTest.java
@@ -865,7 +865,7 @@ public void should_quote_complex_column_names() {
     public void should_quote_column_names_with_escaped_quotes() {
         // A column name can include quotes as long as it is escaped with another set of quotes, so "foo""bar" is a valid name.
         String query = "SELECT * FROM foo WHERE \"foo \"\" bar\"=1;";
-        Statement statement = select().from("foo").where(eq(quote("foo \"\" bar"), 1));
+        Statement statement = select().from("foo").where(eq(quote("foo \" bar"), 1));
 
         assertThat(statement.toString()).isEqualTo(query);
     }

diff --git a/driver-core/src/test/java/com/datastax/driver/core/schemabuilder/CreateTest.java b/driver-core/src/test/java/com/datastax/driver/core/schemabuilder/CreateTest.java
@@ -545,54 +545,54 @@ public void should_fail_if_static_column_in_non_clustered_table() throws Excepti
     }
 
     @Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
-            expectedExceptionsMessageRegExp = "The keyspace name 'add' is not allowed because it is a reserved keyword")
+            expectedExceptionsMessageRegExp = "The keyspace name 'ADD' is not allowed because it is a reserved keyword")
     public void should_fail_if_keyspace_name_is_a_reserved_keyword() throws Exception {
-        createTable("add", "test")
+        createTable("ADD", "test")
                 .addPartitionKey("pk", DataType.bigint())
                 .addColumn("col", DataType.text()).getQueryString();
     }
 
     @Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
-            expectedExceptionsMessageRegExp = "The table name 'add' is not allowed because it is a reserved keyword")
+            expectedExceptionsMessageRegExp = "The table name 'ADD' is not allowed because it is a reserved keyword")
     public void should_fail_if_table_name_is_a_reserved_keyword() throws Exception {
-        createTable("add")
+        createTable("ADD")
                 .addPartitionKey("pk", DataType.bigint())
                 .addColumn("col", DataType.text()).getQueryString();
     }
 
     @Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
-            expectedExceptionsMessageRegExp = "The partition key name 'add' is not allowed because it is a reserved keyword")
+            expectedExceptionsMessageRegExp = "The partition key name 'ADD' is not allowed because it is a reserved keyword")
     public void should_fail_if_partition_key_is_a_reserved_keyword() throws Exception {
         createTable("test")
-                .addPartitionKey("add", DataType.bigint())
+                .addPartitionKey("ADD", DataType.bigint())
                 .addColumn("col", DataType.text()).getQueryString();
     }
 
     @Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
-            expectedExceptionsMessageRegExp = "The clustering column name 'add' is not allowed because it is a reserved keyword")
+            expectedExceptionsMessageRegExp = "The clustering column name 'ADD' is not allowed because it is a reserved keyword")
     public void should_fail_if_clustering_key_is_a_reserved_keyword() throws Exception {
         createTable("test")
                 .addPartitionKey("pk", DataType.bigint())
-                .addClusteringColumn("add", DataType.uuid())
+                .addClusteringColumn("ADD", DataType.uuid())
                 .addColumn("col", DataType.text()).getQueryString();
     }
 
     @Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
-            expectedExceptionsMessageRegExp = "The column name 'add' is not allowed because it is a reserved keyword")
+            expectedExceptionsMessageRegExp = "The column name 'ADD' is not allowed because it is a reserved keyword")
     public void should_fail_if_simple_column_is_a_reserved_keyword() throws Exception {
         createTable("test")
                 .addPartitionKey("pk", DataType.bigint())
                 .addClusteringColumn("cluster", DataType.uuid())
-                .addColumn("add", DataType.text()).getQueryString();
+                .addColumn("ADD", DataType.text()).getQueryString();
     }
 
     @Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
-            expectedExceptionsMessageRegExp = "The static column name 'add' is not allowed because it is a reserved keyword")
+            expectedExceptionsMessageRegExp = "The static column name 'ADD' is not allowed because it is a reserved keyword")
     public void should_fail_if_static_column_is_a_reserved_keyword() throws Exception {
         createTable("test")
                 .addPartitionKey("pk", DataType.bigint())
                 .addClusteringColumn("cluster", DataType.uuid())
-                .addStaticColumn("add", DataType.text())
+                .addStaticColumn("ADD", DataType.text())
                 .addColumn("col", DataType.text()).getQueryString();
     }