Skip to content

Commit cc4db43

Browse files
olim7tolim7t
committed
Merge pull request apache#582 from datastax/java1064
JAVA-1064: getTable create statement doesn't properly handle quotes in primary key.
2 parents 1269b0c + 882f2cc commit cc4db43

8 files changed

Lines changed: 135 additions & 31 deletions

File tree

‎changelog/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
- [bug] JAVA-1100: Exception when connecting with shaded java driver in OSGI
2626
- [bug] JAVA-819: Expose more errors in RetryPolicy + provide idempotent-aware wrapper.
2727
- [improvement] JAVA-1040: SimpleStatement parameters support in QueryLogger.
28+
- [bug] JAVA-1064: getTable create statement doesn't properly handle quotes in primary key.
2829

2930
Merged from 2.0 branch:
3031

‎driver-core/src/main/java/com/datastax/driver/core/Metadata.java‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ public class Metadata {
4747
private static final Pattern alphanumeric = Pattern.compile("\\w+"); // this includes _
4848
private static final Pattern lowercaseAlphanumeric = Pattern.compile("[a-z][a-z0-9_]*");
4949

50+
private static final Set<String> RESERVED_KEYWORDS = ImmutableSet.of(
51+
"add", "allow", "alter", "and", "any", "apply", "asc", "authorize", "batch", "begin", "by",
52+
"columnfamily", "create", "delete", "desc", "drop", "each_quorum", "from", "grant", "in",
53+
"index", "inet", "infinity", "insert", "into", "keyspace", "keyspaces", "limit", "local_one",
54+
"local_quorum", "modify", "nan", "norecursive", "of", "on", "one", "order", "password",
55+
"primary", "quorum", "rename", "revoke", "schema", "select", "set", "table", "to",
56+
"token", "three", "truncate", "two", "unlogged", "update", "use", "using", "where", "with"
57+
);
58+
5059
Metadata(Cluster.Manager cluster) {
5160
this.cluster = cluster;
5261
}
@@ -404,7 +413,7 @@ static String handleId(String id) {
404413
return id.toLowerCase();
405414

406415
// Check if it's enclosed in quotes. If it is, remove them and unescape internal double quotes
407-
if (id.charAt(0) == '"' && id.charAt(id.length() - 1) == '"')
416+
if (!id.isEmpty() && id.charAt(0) == '"' && id.charAt(id.length() - 1) == '"')
408417
return id.substring(1, id.length() - 1).replaceAll("\"\"", "\"");
409418

410419
// Otherwise, just return the id.
@@ -418,8 +427,11 @@ static String handleId(String id) {
418427
// tables. Because it comes from Cassandra, we could just always quote it,
419428
// but to get a nicer output we don't do it if it's not necessary.
420429
static String escapeId(String ident) {
421-
// we don't need to escape if it's lowercase and match non-quoted CQL3 ids.
422-
return lowercaseAlphanumeric.matcher(ident).matches() ? ident : quote(ident);
430+
// we don't need to escape if it's lowercase and match non-quoted CQL3 ids,
431+
// and if it's not a CQL reserved keyword
432+
return lowercaseAlphanumeric.matcher(ident).matches()
433+
&& !isReservedCqlKeyword(ident) ?
434+
ident : quote(ident);
423435
}
424436

425437
/**
@@ -430,16 +442,40 @@ static String escapeId(String ident) {
430442
* the identifier in double quotes (see the
431443
* <a href="http://cassandra.apache.org/doc/cql3/CQL.html#identifiers">CQL documentation</a>
432444
* for details). If you are using case sensitive identifiers, this method
433-
* can be used to enclose such identifier in double quotes, making it case
445+
* can be used to enclose such identifiers in double quotes, making them case
434446
* sensitive.
447+
* <p/>
448+
* Note that
449+
* <a href="https://docs.datastax.com/en/cql/3.0/cql/cql_reference/keywords_r.html">reserved CQL keywords</a>
450+
* should also be quoted. You can check if a given identifier is a reserved keyword
451+
* by calling {@link #isReservedCqlKeyword(String)}.
435452
*
436453
* @param id the keyspace or table identifier.
437454
* @return {@code id} enclosed in double-quotes, for use in methods like
438455
* {@link #getReplicas}, {@link #getKeyspace}, {@link KeyspaceMetadata#getTable}
439456
* or even {@link Cluster#connect(String)}.
440457
*/
441458
public static String quote(String id) {
442-
return '"' + id + '"';
459+
return '"' + id.replace("\"", "\"\"") + '"';
460+
}
461+
462+
/**
463+
* Checks whether an identifier is a known reserved CQL keyword or not.
464+
* <p/>
465+
* The check is case-insensitive, i.e., the word "{@code KeYsPaCe}"
466+
* would be considered as a reserved CQL keyword just as "{@code keyspace}".
467+
* <p/>
468+
* Note: The list of reserved CQL keywords is subject to change in future
469+
* versions of Cassandra. As a consequence, this method is provided solely as a
470+
* convenience utility and should not be considered as an authoritative
471+
* source of truth for checking reserved CQL keywords.
472+
*
473+
* @param id the identifier to check; should not be {@code null}.
474+
* @return {@code true} if the given identifier is a known reserved
475+
* CQL keyword, {@code false} otherwise.
476+
*/
477+
public static boolean isReservedCqlKeyword(String id) {
478+
return id != null && RESERVED_KEYWORDS.contains(id.toLowerCase());
443479
}
444480

445481
/**

‎driver-core/src/main/java/com/datastax/driver/core/TableMetadata.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,12 +435,12 @@ private String asCQLQuery(boolean formatted) {
435435
sb.append("CREATE TABLE ").append(Metadata.escapeId(keyspace.getName())).append('.').append(Metadata.escapeId(name)).append(" (");
436436
newLine(sb, formatted);
437437
for (ColumnMetadata cm : columns.values())
438-
newLine(sb.append(spaces(4, formatted)).append(cm).append(','), formatted);
438+
newLine(sb.append(spaces(4, formatted)).append(cm).append(',').append(spaces(1, !formatted)), formatted);
439439

440440
// PK
441441
sb.append(spaces(4, formatted)).append("PRIMARY KEY (");
442442
if (partitionKey.size() == 1) {
443-
sb.append(partitionKey.get(0).getName());
443+
sb.append(Metadata.escapeId(partitionKey.get(0).getName()));
444444
} else {
445445
sb.append('(');
446446
boolean first = true;

‎driver-core/src/main/java/com/datastax/driver/core/querybuilder/QueryBuilder.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.datastax.driver.core.querybuilder;
1717

18+
import com.datastax.driver.core.Metadata;
1819
import com.datastax.driver.core.RegularStatement;
1920
import com.datastax.driver.core.TableMetadata;
2021
import com.datastax.driver.core.exceptions.InvalidQueryException;
@@ -226,9 +227,10 @@ public static Truncate truncate(TableMetadata table) {
226227
*
227228
* @param columnName the column name to quote.
228229
* @return the quoted column name.
230+
* @see Metadata#quote(String)
229231
*/
230232
public static String quote(String columnName) {
231-
return '"' + columnName + '"';
233+
return Metadata.quote(columnName);
232234
}
233235

234236
/**

‎driver-core/src/main/java/com/datastax/driver/core/schemabuilder/SchemaStatement.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,12 @@
1515
*/
1616
package com.datastax.driver.core.schemabuilder;
1717

18+
import com.datastax.driver.core.Metadata;
1819
import com.datastax.driver.core.ProtocolVersion;
1920
import com.datastax.driver.core.RegularStatement;
2021
import com.google.common.base.Strings;
2122

2223
import java.nio.ByteBuffer;
23-
import java.util.Arrays;
24-
import java.util.List;
2524

2625
/**
2726
* A DDL statement generated by {@link SchemaBuilder}.
@@ -31,12 +30,6 @@ public abstract class SchemaStatement extends RegularStatement {
3130
static final String STATEMENT_START = "\n\t";
3231
static final String COLUMN_FORMATTING = "\n\t\t";
3332

34-
static final List<String> RESERVED_KEYWORDS = Arrays.asList(
35-
("add,allow,alter,and,any,apply,asc,authorize,batch,begin,by,columnfamily,create,delete,desc,drop,each_quorum,from,grant,in,index,inet,infinity," +
36-
"insert,into,keyspace,keyspaces,limit,local_one,local_quorum,modify,nan,norecursive,of,on,order,password,primary,quorum,rename,revoke,schema," +
37-
"select,set,table,three,to,token,truncate,two,unlogged,update,use,using,where,with")
38-
.split(","));
39-
4033
private volatile String cache;
4134

4235
abstract String buildInternal();
@@ -85,7 +78,7 @@ static void validateNotNull(Object value, String label) {
8578
}
8679

8780
static void validateNotKeyWord(String label, String message) {
88-
if (RESERVED_KEYWORDS.contains(label)) {
81+
if (Metadata.isReservedCqlKeyword(label)) {
8982
throw new IllegalArgumentException(message);
9083
}
9184
}

‎driver-core/src/test/java/com/datastax/driver/core/TableMetadataTest.java‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import org.testng.annotations.Test;
1919

20-
import static org.assertj.core.api.Assertions.assertThat;
20+
import static com.datastax.driver.core.Assertions.assertThat;
2121

2222
@CCMConfig(clusterProvider = "createClusterBuilderNoDebouncing")
2323
public class TableMetadataTest extends CCMTestsSupport {
@@ -35,4 +35,76 @@ public void should_escape_single_quote_table_comment() {
3535
TableMetadata table = cluster().getMetadata().getKeyspace(keyspace).getTable("single_quote");
3636
assertThat(table.asCQLQuery()).contains("comment with single quote '' should work");
3737
}
38+
39+
/**
40+
* Validates that a table with case-sensitive column names and column names
41+
* consisting of (quoted) reserved keywords is correctly parsed
42+
* and that the generated CQL is valid.
43+
*
44+
* @jira_ticket JAVA-1064
45+
* @test_category metadata
46+
*/
47+
@Test(groups = "short")
48+
public void should_parse_table_with_case_sensitive_column_names_and_reserved_keywords() throws Exception {
49+
// given
50+
String c1 = Metadata.quote("quotes go \"\" here \"\" ");
51+
String c2 = Metadata.quote("\\x00\\x25");
52+
String c3 = Metadata.quote("columnfamily");
53+
String c4 = Metadata.quote("select");
54+
String c5 = Metadata.quote("who''s there'? ");
55+
String c6 = Metadata.quote("faux )");
56+
String c7 = Metadata.quote("COMPACT STORAGE");
57+
// single partition key
58+
String cql1 = String.format("CREATE TABLE %s.\"MyTable1\" ("
59+
+ "%s text, "
60+
+ "%s text, "
61+
+ "%s text, "
62+
+ "%s text, "
63+
+ "%s text, "
64+
+ "%s text, "
65+
+ "%s text, "
66+
+ "PRIMARY KEY (%s, %s, %s, %s, %s, %s)"
67+
+ ")", keyspace, c1, c2, c3, c4, c5, c6, c7, c1, c2, c3, c4, c5, c6);
68+
// composite partition key
69+
String cql2 = String.format("CREATE TABLE %s.\"MyTable2\" ("
70+
+ "%s text, "
71+
+ "%s text, "
72+
+ "%s text, "
73+
+ "%s text, "
74+
+ "%s text, "
75+
+ "%s text, "
76+
+ "%s text, "
77+
+ "PRIMARY KEY ((%s, %s), %s, %s, %s, %s)"
78+
+ ")", keyspace, c1, c2, c3, c4, c5, c6, c7, c1, c2, c3, c4, c5, c6);
79+
// when
80+
execute(cql1, cql2);
81+
TableMetadata table1 = cluster().getMetadata().getKeyspace(keyspace).getTable("\"MyTable1\"");
82+
TableMetadata table2 = cluster().getMetadata().getKeyspace(keyspace).getTable("\"MyTable2\"");
83+
// then
84+
assertThat(table1)
85+
.hasColumn(c1)
86+
.hasColumn(c2)
87+
.hasColumn(c3)
88+
.hasColumn(c4)
89+
.hasColumn(c5)
90+
.hasColumn(c6)
91+
.hasColumn(c7);
92+
assertThat(table1.asCQLQuery()).startsWith(cql1);
93+
assertThat(table2)
94+
.hasColumn(c1)
95+
.hasColumn(c2)
96+
.hasColumn(c3)
97+
.hasColumn(c4)
98+
.hasColumn(c5)
99+
.hasColumn(c6)
100+
.hasColumn(c7);
101+
assertThat(table2.asCQLQuery()).startsWith(cql2);
102+
execute(
103+
"DROP TABLE \"MyTable1\"",
104+
"DROP TABLE \"MyTable2\"",
105+
table1.asCQLQuery(),
106+
table2.asCQLQuery()
107+
);
108+
}
109+
38110
}

‎driver-core/src/test/java/com/datastax/driver/core/querybuilder/QueryBuilderTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,7 @@ public void should_quote_complex_column_names() {
865865
public void should_quote_column_names_with_escaped_quotes() {
866866
// A column name can include quotes as long as it is escaped with another set of quotes, so "foo""bar" is a valid name.
867867
String query = "SELECT * FROM foo WHERE \"foo \"\" bar\"=1;";
868-
Statement statement = select().from("foo").where(eq(quote("foo \"\" bar"), 1));
868+
Statement statement = select().from("foo").where(eq(quote("foo \" bar"), 1));
869869

870870
assertThat(statement.toString()).isEqualTo(query);
871871
}

‎driver-core/src/test/java/com/datastax/driver/core/schemabuilder/CreateTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -545,54 +545,54 @@ public void should_fail_if_static_column_in_non_clustered_table() throws Excepti
545545
}
546546

547547
@Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
548-
expectedExceptionsMessageRegExp = "The keyspace name 'add' is not allowed because it is a reserved keyword")
548+
expectedExceptionsMessageRegExp = "The keyspace name 'ADD' is not allowed because it is a reserved keyword")
549549
public void should_fail_if_keyspace_name_is_a_reserved_keyword() throws Exception {
550-
createTable("add", "test")
550+
createTable("ADD", "test")
551551
.addPartitionKey("pk", DataType.bigint())
552552
.addColumn("col", DataType.text()).getQueryString();
553553
}
554554

555555
@Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
556-
expectedExceptionsMessageRegExp = "The table name 'add' is not allowed because it is a reserved keyword")
556+
expectedExceptionsMessageRegExp = "The table name 'ADD' is not allowed because it is a reserved keyword")
557557
public void should_fail_if_table_name_is_a_reserved_keyword() throws Exception {
558-
createTable("add")
558+
createTable("ADD")
559559
.addPartitionKey("pk", DataType.bigint())
560560
.addColumn("col", DataType.text()).getQueryString();
561561
}
562562

563563
@Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
564-
expectedExceptionsMessageRegExp = "The partition key name 'add' is not allowed because it is a reserved keyword")
564+
expectedExceptionsMessageRegExp = "The partition key name 'ADD' is not allowed because it is a reserved keyword")
565565
public void should_fail_if_partition_key_is_a_reserved_keyword() throws Exception {
566566
createTable("test")
567-
.addPartitionKey("add", DataType.bigint())
567+
.addPartitionKey("ADD", DataType.bigint())
568568
.addColumn("col", DataType.text()).getQueryString();
569569
}
570570

571571
@Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
572-
expectedExceptionsMessageRegExp = "The clustering column name 'add' is not allowed because it is a reserved keyword")
572+
expectedExceptionsMessageRegExp = "The clustering column name 'ADD' is not allowed because it is a reserved keyword")
573573
public void should_fail_if_clustering_key_is_a_reserved_keyword() throws Exception {
574574
createTable("test")
575575
.addPartitionKey("pk", DataType.bigint())
576-
.addClusteringColumn("add", DataType.uuid())
576+
.addClusteringColumn("ADD", DataType.uuid())
577577
.addColumn("col", DataType.text()).getQueryString();
578578
}
579579

580580
@Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
581-
expectedExceptionsMessageRegExp = "The column name 'add' is not allowed because it is a reserved keyword")
581+
expectedExceptionsMessageRegExp = "The column name 'ADD' is not allowed because it is a reserved keyword")
582582
public void should_fail_if_simple_column_is_a_reserved_keyword() throws Exception {
583583
createTable("test")
584584
.addPartitionKey("pk", DataType.bigint())
585585
.addClusteringColumn("cluster", DataType.uuid())
586-
.addColumn("add", DataType.text()).getQueryString();
586+
.addColumn("ADD", DataType.text()).getQueryString();
587587
}
588588

589589
@Test(groups = "unit", expectedExceptions = IllegalArgumentException.class,
590-
expectedExceptionsMessageRegExp = "The static column name 'add' is not allowed because it is a reserved keyword")
590+
expectedExceptionsMessageRegExp = "The static column name 'ADD' is not allowed because it is a reserved keyword")
591591
public void should_fail_if_static_column_is_a_reserved_keyword() throws Exception {
592592
createTable("test")
593593
.addPartitionKey("pk", DataType.bigint())
594594
.addClusteringColumn("cluster", DataType.uuid())
595-
.addStaticColumn("add", DataType.text())
595+
.addStaticColumn("ADD", DataType.text())
596596
.addColumn("col", DataType.text()).getQueryString();
597597
}
598598

0 commit comments

Comments
 (0)