DATACASS-192 - Fix syntax error when adding columns using CQL generators.

The Add-Column CQL generator now generates correct CQL statements.

Original pull request: #69.
This commit is contained in:
Mark Paluch
2016-06-21 15:11:15 +02:00
committed by John Blum
parent 6ee70916ff
commit 38fef212e4
9 changed files with 241 additions and 168 deletions

View File

@@ -1,3 +1,18 @@
/*
* Copyright 2016 the original author or authors.
*
* Licensed 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.springframework.cassandra.core.cql;
import java.io.Serializable;
@@ -23,6 +38,7 @@ import com.datastax.driver.core.TableMetadata;
* @see #toString()
* @author John McPeek
* @author Matthew T. Adams
* @author Mark Paluch
*/
public final class CqlIdentifier implements Comparable<CqlIdentifier>, Serializable {
@@ -110,10 +126,10 @@ public final class CqlIdentifier implements Comparable<CqlIdentifier>, Serializa
*/
private void setIdentifier(CharSequence identifier, boolean forceQuoting) {
Assert.notNull(identifier);
Assert.notNull(identifier, "Identifier must not be null");
String string = identifier.toString();
Assert.hasText(string);
Assert.hasText(string, "Identifier must not be empty");
if (forceQuoting || isQuotedIdentifier(string)) {
this.unquoted = string;

View File

@@ -23,6 +23,7 @@ import org.springframework.cassandra.core.keyspace.AddColumnSpecification;
* CQL generator for generating an <code>ADD</code> clause of an <code>ALTER TABLE</code> statement.
*
* @author Matthew T. Adams
* @author Mark Paluch
*/
public class AddColumnCqlGenerator extends ColumnChangeCqlGenerator<AddColumnSpecification> {
@@ -32,6 +33,6 @@ public class AddColumnCqlGenerator extends ColumnChangeCqlGenerator<AddColumnSpe
@Override
public StringBuilder toCql(StringBuilder cql) {
return noNull(cql).append("ADD ").append(spec().getName()).append(" TYPE ").append(spec().getType().getName());
return noNull(cql).append("ADD ").append(spec().getName()).append(' ').append(spec().getType().asFunctionParameterString());
}
}

View File

@@ -31,6 +31,6 @@ public class AlterColumnCqlGenerator extends ColumnChangeCqlGenerator<AlterColum
}
public StringBuilder toCql(StringBuilder cql) {
return noNull(cql).append("ALTER ").append(spec().getName()).append(" TYPE ").append(spec().getType().getName());
return noNull(cql).append("ALTER ").append(spec().getName()).append(" TYPE ").append(spec().getType().asFunctionParameterString());
}
}

View File

@@ -19,12 +19,30 @@ import org.springframework.cassandra.core.cql.CqlIdentifier;
import com.datastax.driver.core.DataType;
/**
* Specification to add a column.
*
* @author Matthew Adams
* @author Mark Paluch
*/
public class AddColumnSpecification extends ColumnTypeChangeSpecification {
/**
* Creates a new {@link AddColumnSpecification} for the given {@code name} and {@link type}
*
* @param name must not be empty or {@literal null}.
* @param type must not be {@literal null}.
*/
public AddColumnSpecification(String name, DataType type) {
super(name, type);
}
/**
* Creates a new {@link AddColumnSpecification} for the given {@code name} and {@link type}
*
* @param name must not be {@literal null}.
* @param type must not be {@literal null}.
*/
public AddColumnSpecification(CqlIdentifier name, DataType type) {
super(name, type);
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2014 the original author or authors.
* Copyright 2013-2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,33 +15,53 @@
*/
package org.springframework.cassandra.core.keyspace;
import static org.springframework.cassandra.core.cql.CqlIdentifier.cqlId;
import static org.springframework.cassandra.core.cql.CqlIdentifier.*;
import org.springframework.cassandra.core.cql.CqlIdentifier;
import org.springframework.util.Assert;
/**
* Base class for column change specifications.
* Base value object class for column change specifications.
*
* @author Matthew T. Adams
* @author Mark Paluch
*/
public abstract class ColumnChangeSpecification {
protected CqlIdentifier name;
/**
* Creates a new {@link ColumnChangeSpecification}.
*
* @param name must not be empty or {@literal null}.
*/
protected ColumnChangeSpecification(String name) {
this(cqlId(name));
}
/**
* Creates a new {@link ColumnChangeSpecification}.
*
* @param name must not be {@literal null}.
*/
protected ColumnChangeSpecification(CqlIdentifier name) {
setName(name);
}
/**
* Sets the column name.
*
* @param name must not be {@literal null}.
*/
protected void setName(CqlIdentifier name) {
Assert.notNull(name);
Assert.notNull(name, "Name must not be null");
this.name = name;
}
/**
* @return the column name.
*/
public CqlIdentifier getName() {
return name;
}

View File

@@ -15,7 +15,7 @@
*/
package org.springframework.cassandra.core.keyspace;
import static org.springframework.cassandra.core.cql.CqlIdentifier.cqlId;
import static org.springframework.cassandra.core.cql.CqlIdentifier.*;
import org.springframework.cassandra.core.cql.CqlIdentifier;
import org.springframework.util.Assert;
@@ -23,28 +23,41 @@ import org.springframework.util.Assert;
import com.datastax.driver.core.DataType;
/**
* Base class for column changes that include {@link DataType} information.
* Base value object class for column changes that include {@link DataType} information.
*
* @author Matthew T. Adams
*/
public abstract class ColumnTypeChangeSpecification extends ColumnChangeSpecification {
private DataType type;
private final DataType type;
/**
* Creates a new {@link ColumnTypeChangeSpecification} for the given {@code name} and {@link type}
*
* @param name must not be empty or {@literal null}.
* @param type must not be {@literal null}.
*/
public ColumnTypeChangeSpecification(String name, DataType type) {
this(cqlId(name), type);
}
/**
* Creates a new {@link ColumnTypeChangeSpecification} for the given {@code name} and {@link type}
*
* @param name must not be {@literal null}.
* @param type must not be {@literal null}.
*/
public ColumnTypeChangeSpecification(CqlIdentifier name, DataType type) {
super(name);
setType(type);
}
private void setType(DataType type) {
Assert.notNull(type);
Assert.notNull(type, "DataType must not be null");
this.type = type;
}
/**
* @return the {@literal DataType}.
*/
public DataType getType() {
return type;
}

View File

@@ -0,0 +1,125 @@
/*
* Copyright 2016 the original author or authors.
*
* Licensed 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.springframework.cassandra.core.cql.generator;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import org.junit.Before;
import org.junit.Test;
import org.springframework.cassandra.core.keyspace.AlterTableSpecification;
import org.springframework.cassandra.test.integration.AbstractKeyspaceCreatingIntegrationTest;
import com.datastax.driver.core.ColumnMetadata;
import com.datastax.driver.core.DataType;
import com.datastax.driver.core.KeyspaceMetadata;
import com.datastax.driver.core.TableMetadata;
/**
* Integration tests tests for {@link AlterTableCqlGenerator}.
*
* @author Mark Paluch
*/
public class AlterTableCqlGeneratorIntegrationTests extends AbstractKeyspaceCreatingIntegrationTest {
@Before
public void setUp() throws Exception {
session.execute("DROP TABLE IF EXISTS addamsFamily;");
session.execute("DROP TABLE IF EXISTS users;");
}
/**
* @see DATACASS-192
*/
@Test
public void alterTableAlterColumnType() {
session.execute("CREATE TABLE addamsFamily (name varchar PRIMARY KEY, gender varchar,\n"
+ " lastknownlocation bigint);");
AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").alter("lastKnownLocation",
DataType.varint());
execute(spec);
ColumnMetadata column = getTableMetadata("addamsFamily").getColumn("lastKnownLocation");
assertThat(column.getType(), is(equalTo(DataType.varint())));
}
/**
* @see DATACASS-192
*/
@Test
public void alterTableAlterListColumnType() {
session.execute("CREATE TABLE addamsFamily (name varchar PRIMARY KEY, gender varchar,\n"
+ " lastknownlocation list<ascii>);");
AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").alter("lastKnownLocation",
DataType.list(DataType.varchar()));
execute(spec);
ColumnMetadata column = getTableMetadata("addamsFamily").getColumn("lastKnownLocation");
assertThat(column.getType(), is(equalTo((DataType) DataType.list(DataType.varchar()))));
}
/**
* @see DATACASS-192
*/
@Test
public void alterTableAddColumn() {
session.execute("CREATE TABLE addamsFamily (name varchar PRIMARY KEY, gender varchar,\n"
+ " lastknownlocation varchar);");
AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").add("gravesite",
DataType.varchar());
execute(spec);
ColumnMetadata column = getTableMetadata("addamsFamily").getColumn("gravesite");
assertThat(column.getType(), is(equalTo(DataType.varchar())));
}
/**
* @see DATACASS-192
*/
@Test
public void alterTableAddListColumn() {
session.execute("CREATE TABLE users (user_name varchar PRIMARY KEY);");
AlterTableSpecification spec = AlterTableSpecification.alterTable("users").add("top_places",
DataType.list(DataType.ascii()));
execute(spec);
ColumnMetadata column = getTableMetadata("users").getColumn("top_places");
assertThat(column.getType(), is(equalTo((DataType) DataType.list(DataType.ascii()))));
}
private void execute(AlterTableSpecification spec) {
session.execute(new AlterTableCqlGenerator(spec).toCql());
}
private TableMetadata getTableMetadata(String table) {
KeyspaceMetadata keyspace = session.getCluster().getMetadata().getKeyspace(session.getLoggedKeyspace());
return keyspace.getTable(table);
}
}

View File

@@ -15,21 +15,11 @@
*/
package org.springframework.cassandra.core.cql.generator;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import java.util.LinkedHashMap;
import java.util.Map;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.cassandra.core.keyspace.AlterTableSpecification;
import org.springframework.cassandra.core.keyspace.Option;
import org.springframework.cassandra.core.keyspace.TableOption;
import org.springframework.cassandra.core.keyspace.TableOption.CachingOption;
import org.springframework.cassandra.core.keyspace.TableOption.CompactionOption;
import org.springframework.cassandra.core.keyspace.TableOption.CompressionOption;
import org.springframework.cassandra.core.keyspace.TableOption.KeyCachingOption;
import com.datastax.driver.core.DataType;
@@ -38,139 +28,59 @@ import com.datastax.driver.core.DataType;
*
* @author Matthew T. Adams
* @author David Webb
* @author Mark Paluch
*/
public class AlterTableCqlGeneratorUnitTests {
private final static Logger log = LoggerFactory.getLogger(AlterTableCqlGeneratorUnitTests.class);
/**
* Asserts that the preamble is first & correctly formatted in the given CQL string.
* @see DATACASS-192
*/
public static void assertPreamble(String tableName, String cql) {
assertTrue(cql.startsWith("ALTER TABLE " + tableName + " "));
@Test
public void alterTableAlterColumnType() {
AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").alter("lastKnownLocation",
DataType.uuid());
assertThat(toCql(spec), is(equalTo("ALTER TABLE addamsfamily ALTER lastknownlocation TYPE uuid;")));
}
/**
* Asserts that the given list of columns definitions are contained in the given CQL string properly.
*
* @param columnSpec IE, "foo text, bar blob"
* @see DATACASS-192
*/
public static void assertColumnChanges(String columnSpec, String cql) {
assertTrue(cql.contains(""));
@Test
public void alterTableAlterListColumnType() {
AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").alter("lastKnownLocation",
DataType.list(DataType.ascii()));
assertThat(toCql(spec), is(equalTo("ALTER TABLE addamsfamily ALTER lastknownlocation TYPE list<ascii>;")));
}
/**
* Convenient base class that other test classes can use so as not to repeat the generics declarations.
* @see DATACASS-192
*/
public static abstract class AlterTableTest
extends AbstractTableOperationCqlGeneratorTest<AlterTableSpecification, AlterTableCqlGenerator> {}
@Test
public void alterTableAddColumn() {
public static class BasicTest extends AlterTableTest {
AlterTableSpecification spec = AlterTableSpecification.alterTable("addamsFamily").add("gravesite",
DataType.varchar());
public String name = "mytable";
public DataType alteredType = DataType.text();
public String altered = "altered";
public DataType addedType = DataType.text();
public String added = "added";
public String dropped = "dropped";
@Override
public AlterTableSpecification specification() {
return AlterTableSpecification.alterTable().name(name).alter(altered, alteredType).add(added, addedType);
}
@Override
public AlterTableCqlGenerator generator() {
return new AlterTableCqlGenerator(specification);
}
@Test
public void test() {
prepare();
assertPreamble(name, cql);
assertColumnChanges(
String.format("ALTER %s TYPE %s, ADD %s %s, DROP %s", altered, alteredType, added, addedType, dropped), cql);
}
assertThat(toCql(spec), is(equalTo("ALTER TABLE addamsfamily ADD gravesite varchar;")));
}
/**
* Fully test all available create table options
*
* @author David Webb
* @see DATACASS-192
*/
public static class MultipleOptionsTest extends AlterTableTest {
@Test
public void alterTableAddListColumn() {
public String name = "timeseries_table";
public DataType partitionKeyType0 = DataType.timeuuid();
public String partitionKey0 = "tid";
public DataType partitionKeyType1 = DataType.timestamp();
public String partitionKey1 = "create_timestamp";
public DataType columnType1 = DataType.text();
public String column1 = "data_point";
public Double readRepairChance = 0.6;
public Double dcLocalReadRepairChance = 0.8;
public Double bloomFilterFpChance = 0.002;
public Boolean replcateOnWrite = Boolean.FALSE;
public Long gcGraceSeconds = 1200l;
public String comment = "This is My Table";
public Map<Option, Object> compactionMap = new LinkedHashMap<Option, Object>();
public Map<Option, Object> compressionMap = new LinkedHashMap<Option, Object>();
public Map<Option, Object> cachingMap = new LinkedHashMap<Option, Object>();
AlterTableSpecification spec = AlterTableSpecification.alterTable("users").add("top_places",
DataType.list(DataType.ascii()));
@Override
public AlterTableSpecification specification() {
assertThat(toCql(spec), is(equalTo("ALTER TABLE users ADD top_places list<ascii>;")));
}
// Compaction
compactionMap.put(CompactionOption.CLASS, "SizeTieredCompactionStrategy");
compactionMap.put(CompactionOption.MIN_THRESHOLD, "4");
// Compression
compressionMap.put(CompressionOption.SSTABLE_COMPRESSION, "SnappyCompressor");
compressionMap.put(CompressionOption.CHUNK_LENGTH_KB, 128);
compressionMap.put(CompressionOption.CRC_CHECK_CHANCE, 0.75);
// Caching
cachingMap.put(CachingOption.KEYS, KeyCachingOption.ALL);
cachingMap.put(CachingOption.ROWS_PER_PARTITION, "10");
return AlterTableSpecification.alterTable().name(name)
// .with(TableOption.COMPACT_STORAGE)
.with(TableOption.READ_REPAIR_CHANCE, readRepairChance).with(TableOption.COMPACTION, compactionMap)
.with(TableOption.COMPRESSION, compressionMap).with(TableOption.BLOOM_FILTER_FP_CHANCE, bloomFilterFpChance)
.with(TableOption.CACHING, cachingMap).with(TableOption.COMMENT, comment)
.with(TableOption.DCLOCAL_READ_REPAIR_CHANCE, dcLocalReadRepairChance)
.with(TableOption.GC_GRACE_SECONDS, gcGraceSeconds);
}
@Test
public void test() {
prepare();
log.info(cql);
assertPreamble(name, cql);
// assertColumns(String.format("%s %s, %s %s, %s %s", partitionKey0, partitionKeyType0, partitionKey1,
// partitionKeyType1, column1, columnType1), cql);
// assertPrimaryKey(String.format("(%s, %s)", partitionKey0, partitionKey1), cql);
// assertNullOption(TableOption.COMPACT_STORAGE.getName(), cql);
// assertDoubleOption(TableOption.READ_REPAIR_CHANCE.getName(), readRepairChance, cql);
// assertDoubleOption(TableOption.DCLOCAL_READ_REPAIR_CHANCE.getName(), dcLocalReadRepairChance, cql);
// assertDoubleOption(TableOption.BLOOM_FILTER_FP_CHANCE.getName(), bloomFilterFpChance, cql);
// assertStringOption(TableOption.CACHING.getName(), CachingOption.KEYS_ONLY.getValue(), cql);
// assertStringOption(TableOption.REPLICATE_ON_WRITE.getName(), replcateOnWrite.toString(), cql);
// assertStringOption(TableOption.COMMENT.getName(), comment, cql);
// assertLongOption(TableOption.GC_GRACE_SECONDS.getName(), gcGraceSeconds, cql);
}
/* (non-Javadoc)
* @see org.springframework.cassandra.test.unit.core.cql.generator.TableOperationCqlGeneratorTest#generator()
*/
@Override
public AlterTableCqlGenerator generator() {
return new AlterTableCqlGenerator(specification);
}
private String toCql(AlterTableSpecification spec) {
return new AlterTableCqlGenerator(spec).toCql();
}
}

View File

@@ -67,36 +67,6 @@ public class TableLifecycleIntegrationTests extends AbstractKeyspaceCreatingInte
assertNoTable(dropTest.specification, keyspace, session);
}
@Test
public void testAlter() {
createTableTest.prepare();
log.info(createTableTest.cql);
session.execute(createTableTest.cql);
assertTable(createTableTest.specification, keyspace, session);
AlterTableCqlGeneratorUnitTests.MultipleOptionsTest alterTest = new AlterTableCqlGeneratorUnitTests.MultipleOptionsTest();
alterTest.prepare();
log.info(alterTest.cql);
session.execute(alterTest.cql);
// assertTable(alterTest.specification, keyspace, session);
DropTableTest dropTest = new DropTableTest();
dropTest.prepare();
log.info(dropTest.cql);
session.execute(dropTest.cql);
assertNoTable(dropTest.specification, keyspace, session);
}
public class DropTableTest extends DropTableCqlGeneratorUnitTests.DropTableTest {