Minor fixes for CQL3 patch by slebresne; reviewed by jbellis for CASSANDRA-4185
- Fix float parsing - Refuse more invalidate create statements - Fix count in select - Disallow counters in primary keys Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5f1c5ad3 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5f1c5ad3 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5f1c5ad3 Branch: refs/heads/cassandra-1.1 Commit: 5f1c5ad393a0ce5e455f865a7883303ca5e51690 Parents: e53d980 Author: Sylvain Lebresne <sylv...@datastax.com> Authored: Thu May 3 09:18:59 2012 +0200 Committer: Sylvain Lebresne <sylv...@datastax.com> Committed: Thu May 3 09:23:00 2012 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + src/java/org/apache/cassandra/cql3/Cql.g | 14 ++++++++++++-- .../cql3/statements/AlterTableStatement.java | 7 ++++++- .../statements/CreateColumnFamilyStatement.java | 14 +++++++++++++- .../cassandra/cql3/statements/SelectStatement.java | 15 ++++++++------- 5 files changed, 40 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/5f1c5ad3/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 2bbe377..9a0569f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -26,6 +26,7 @@ * (cql3) Fix order by for reversed queries (CASSANDRA-4160) * (cql3) Add ReversedType support (CASSANDRA-4004) * (cql3) Add timeuuid type (CASSANDRA-4194) + * (cql3) Minor fixes (CASSANDRA-4185) Merged from 1.0: * Fix super columns bug where cache is not updated (CASSANDRA-4190) * fix maxTimestamp to include row tombstones (CASSANDRA-4116) http://git-wip-us.apache.org/repos/asf/cassandra/blob/5f1c5ad3/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 f72aeda..3220c1a 100644 --- a/src/java/org/apache/cassandra/cql3/Cql.g +++ b/src/java/org/apache/cassandra/cql3/Cql.g @@ -50,6 +50,11 @@ options { recognitionErrors.add(hdr + " " + msg); } + public void addRecognitionError(String msg) + { + recognitionErrors.add(msg); + } + public List<String> getRecognitionErrors() { return recognitionErrors; @@ -159,7 +164,7 @@ selectStatement returns [SelectStatement.RawStatement expr] int limit = 10000; Map<ColumnIdentifier, Boolean> orderings = new LinkedHashMap<ColumnIdentifier, Boolean>(); } - : K_SELECT ( sclause=selectClause | (K_COUNT '(' sclause=selectClause ')' { isCount = true; }) ) + : K_SELECT ( sclause=selectClause | (K_COUNT '(' sclause=selectCountClause ')' { isCount = true; }) ) K_FROM cf=columnFamilyName ( K_USING K_CONSISTENCY K_LEVEL { cLevel = ConsistencyLevel.valueOf($K_LEVEL.text.toUpperCase()); } )? ( K_WHERE wclause=whereClause )? @@ -179,6 +184,11 @@ selectClause returns [List<ColumnIdentifier> expr] | '\*' { $expr = Collections.<ColumnIdentifier>emptyList();} ; +selectCountClause returns [List<ColumnIdentifier> expr] + : c=selectClause { $expr = c; } + | i=INTEGER { if (!i.getText().equals("1")) addRecognitionError("Only COUNT(1) is supported, got COUNT(" + i.getText() + ")"); $expr = Collections.<ColumnIdentifier>emptyList();} + ; + whereClause returns [List<Relation> clause] @init{ $clause = new ArrayList<Relation>(); } : first=relation { $clause.add(first); } (K_AND next=relation { $clause.add(next); })* @@ -618,7 +628,7 @@ QMARK * to support multiple (see @lexer::members near the top of the grammar). */ FLOAT - : INTEGER '.' INTEGER + : INTEGER '.' DIGIT* ; IDENT http://git-wip-us.apache.org/repos/asf/cassandra/blob/5f1c5ad3/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java index bb2c7e8..b1291fe 100644 --- a/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/AlterTableStatement.java @@ -24,7 +24,9 @@ import java.util.*; import org.apache.cassandra.cql3.*; import org.apache.cassandra.config.*; import org.apache.cassandra.io.compress.CompressionParameters; +import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.CompositeType; +import org.apache.cassandra.db.marshal.CounterColumnType; import org.apache.cassandra.service.MigrationManager; import org.apache.cassandra.thrift.CfDef; import org.apache.cassandra.thrift.ColumnDef; @@ -90,7 +92,10 @@ public class AlterTableStatement extends SchemaAlteringStatement switch (name.kind) { case KEY_ALIAS: - cfm.keyValidator(CFPropDefs.parseType(validator)); + AbstractType<?> newType = CFPropDefs.parseType(validator); + if (newType instanceof CounterColumnType) + throw new InvalidRequestException(String.format("counter type is not supported for PRIMARY KEY part %s", columnName)); + cfm.keyValidator(newType); break; case COLUMN_ALIAS: throw new InvalidRequestException(String.format("Cannot alter PRIMARY KEY part %s", columnName)); http://git-wip-us.apache.org/repos/asf/cassandra/blob/5f1c5ad3/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java b/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java index eb0aeb4..9ee0f94 100644 --- a/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/CreateColumnFamilyStatement.java @@ -36,6 +36,7 @@ import org.apache.cassandra.db.ColumnFamilyType; import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.CompositeType; import org.apache.cassandra.db.marshal.ReversedType; +import org.apache.cassandra.db.marshal.CounterColumnType; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.service.MigrationManager; import org.apache.cassandra.thrift.CqlResult; @@ -175,6 +176,8 @@ public class CreateColumnFamilyStatement extends SchemaAlteringStatement stmt.keyAlias = keyAliases.get(0).key; stmt.keyValidator = getTypeAndRemove(stmt.columns, keyAliases.get(0)); + if (stmt.keyValidator instanceof CounterColumnType) + throw new InvalidRequestException(String.format("counter type is not supported for PRIMARY KEY part %s", stmt.keyAlias)); // Handle column aliases if (columnAliases != null && !columnAliases.isEmpty()) @@ -185,6 +188,8 @@ public class CreateColumnFamilyStatement extends SchemaAlteringStatement { stmt.columnAliases.add(columnAliases.get(0).key); stmt.comparator = getTypeAndRemove(stmt.columns, columnAliases.get(0)); + if (stmt.comparator instanceof CounterColumnType) + throw new InvalidRequestException(String.format("counter type is not supported for PRIMARY KEY part %s", stmt.columnAliases.get(0))); } else { @@ -192,7 +197,11 @@ public class CreateColumnFamilyStatement extends SchemaAlteringStatement for (ColumnIdentifier t : columnAliases) { stmt.columnAliases.add(t.key); - types.add(getTypeAndRemove(stmt.columns, t)); + + AbstractType<?> type = getTypeAndRemove(stmt.columns, t); + if (type instanceof CounterColumnType) + throw new InvalidRequestException(String.format("counter type is not supported for PRIMARY KEY part %s", t.key)); + types.add(type); } // For sparse, we must add the last UTF8 component if (!useCompactStorage) @@ -210,6 +219,9 @@ public class CreateColumnFamilyStatement extends SchemaAlteringStatement if (useCompactStorage) { + // There should at least have been one column alias + if (stmt.columnAliases.isEmpty()) + throw new InvalidRequestException("COMPACT STORAGE requires at least one column part of the clustering key, none found"); // There should be only one column definition remaining, which gives us the default validator. if (stmt.columns.isEmpty()) throw new InvalidRequestException("COMPACT STORAGE requires one definition not part of the PRIMARY KEY, none found"); http://git-wip-us.apache.org/repos/asf/cassandra/blob/5f1c5ad3/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 00c3b24..561db9a 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -145,6 +145,10 @@ public class SelectStatement implements CQLStatement CqlResult result = new CqlResult(); result.type = CqlResultType.ROWS; + // Even for count, we need to process the result as it'll group some column together in sparse column families + CqlMetadata schema = createSchema(); + List<CqlRow> cqlRows = process(rows, schema, variables); + // count resultset is a single column named "count" if (parameters.isCount) { @@ -152,15 +156,15 @@ public class SelectStatement implements CQLStatement Collections.<ByteBuffer, String>emptyMap(), "AsciiType", "LongType"); - List<Column> columns = Collections.singletonList(new Column(countColumn).setValue(ByteBufferUtil.bytes((long) rows.size()))); + List<Column> columns = Collections.singletonList(new Column(countColumn).setValue(ByteBufferUtil.bytes((long) cqlRows.size()))); result.rows = Collections.singletonList(new CqlRow(countColumn, columns)); return result; } else { // otherwise create resultset from query results - result.schema = createSchema(); - result.rows = process(rows, result.schema, variables); + result.schema = schema; + result.rows = cqlRows; return result; } } @@ -822,10 +826,7 @@ public class SelectStatement implements CQLStatement // Select clause if (parameters.isCount) { - if (selectClause.size() != 1) - throw new InvalidRequestException("Only COUNT(*) and COUNT(1) operations are currently supported."); - String columnName = selectClause.get(0).toString(); - if (!columnName.equals("*") && !columnName.equals("1")) + if (!selectClause.isEmpty()) throw new InvalidRequestException("Only COUNT(*) and COUNT(1) operations are currently supported."); } else
