diff --git a/changelog/README.md b/changelog/README.md index dda4cdcb778..721380ec8c6 100644 --- 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 index 06f9bda439c..6e9efb746ea 100644 --- 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 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,8 +442,13 @@ static String escapeId(String ident) { * the identifier in double quotes (see the * CQL documentation * 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. + *

+ * Note that + * reserved CQL keywords + * 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 @@ -439,7 +456,26 @@ static String escapeId(String ident) { * 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. + *

+ * The check is case-insensitive, i.e., the word "{@code KeYsPaCe}" + * would be considered as a reserved CQL keyword just as "{@code keyspace}". + *

+ * 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 index 1badcf86708..5b31a91fb90 100644 --- 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 index 4c2fcf66f09..af6eb055d97 100644 --- 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 index ad81c32a743..7f186641879 100755 --- 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 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 index 94e3caa9186..272129e68ca 100644 --- 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 index ad2ff2f313e..242c2d0da07 100644 --- 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 index 1bae966e628..8177e2a8148 100644 --- 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(); }