Skip to content

Commit

Permalink
Prevent mutations that create counter columns (#572)
Browse files Browse the repository at this point in the history
  • Loading branch information
@wi11dey
wi11dey authored Nov 12, 2024
1 parent cfbbb42 commit 6fdd4a7
Show file tree
Hide file tree
Showing 21 changed files with 25 additions and 1,380 deletions.
12 changes: 12 additions & 0 deletions src/java/org/apache/cassandra/config/CFMetaData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.palantir.logsafe.Preconditions;
import com.palantir.logsafe.SafeArg;
import org.apache.cassandra.cache.CachingOptions;
import org.apache.cassandra.cql3.ColumnIdentifier;
import org.apache.cassandra.cql3.QueryProcessor;
Expand Down Expand Up @@ -1048,6 +1050,16 @@ public CFMetaData validate() throws ConfigurationException
throw new ConfigurationException("Cannot add a counter column (" + def.name + ") in a non counter column family");
}

for (ColumnDefinition def : allColumns())
{
Preconditions.checkArgument(
!(def.type instanceof CounterColumnType),
"Palantir Cassandra does not support counter columns",
SafeArg.of("keyspace", ksName),
SafeArg.of("columnFamily", cfName),
SafeArg.of("columnName", def.name));
}

// initialize a set of names NOT in the CF under consideration
Set<String> indexNames = existingIndexNames(cfName);
for (ColumnDefinition c : allColumns())
Expand Down
14 changes: 1 addition & 13 deletions test/unit/org/apache/cassandra/SchemaLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ public static void schemaDefinition(String testName) throws ConfigurationExcepti
String ks6 = testName + "Keyspace6";
String ks_kcs = testName + "KeyCacheSpace";
String ks_rcs = testName + "RowCacheSpace";
String ks_ccs = testName + "CounterCacheSpace";
String ks_nocommit = testName + "NoCommitlogSpace";
String ks_prsi = testName + "PerRowSecondaryIndex";
String ks_cql = testName + "cql_keyspace";
Expand Down Expand Up @@ -166,8 +165,6 @@ public static void schemaDefinition(String testName) throws ConfigurationExcepti
indexCFMD(ks1, "Indexed2", false),
CFMetaData.denseCFMetaData(ks1, "StandardInteger1", IntegerType.instance),
CFMetaData.denseCFMetaData(ks1, "StandardLong3", IntegerType.instance),
CFMetaData.denseCFMetaData(ks1, "Counter1", bytes).defaultValidator(CounterColumnType.instance),
CFMetaData.denseCFMetaData(ks1, "SuperCounter1", bytes, bytes).defaultValidator(CounterColumnType.instance),
superCFMD(ks1, "SuperDirectGC", BytesType.instance).gcGraceSeconds(0),
jdbcSparseCFMD(ks1, "JdbcInteger", IntegerType.instance).addColumnDefinition(integerColumn(ks1, "JdbcInteger")),
jdbcSparseCFMD(ks1, "JdbcUtf8", UTF8Type.instance).addColumnDefinition(utf8Column(ks1, "JdbcUtf8")),
Expand Down Expand Up @@ -234,9 +231,7 @@ public static void schemaDefinition(String testName) throws ConfigurationExcepti
schema.add(KSMetaData.testMetadata(ks5,
simple,
opts_rf2,
standardCFMD(ks5, "Standard1"),
standardCFMD(ks5, "Counter1")
.defaultValidator(CounterColumnType.instance)));
standardCFMD(ks5, "Standard1")));

// Keyspace 6
schema.add(KSMetaData.testMetadata(ks6,
Expand All @@ -263,13 +258,6 @@ public static void schemaDefinition(String testName) throws ConfigurationExcepti
caching(new CachingOptions(new CachingOptions.KeyCache(CachingOptions.KeyCache.Type.ALL),
new CachingOptions.RowCache(CachingOptions.RowCache.Type.HEAD, 100)))));

// CounterCacheSpace
schema.add(KSMetaData.testMetadata(ks_ccs,
simple,
opts_rf1,
standardCFMD(ks_ccs, "Counter1").defaultValidator(CounterColumnType.instance),
standardCFMD(ks_ccs, "Counter2").defaultValidator(CounterColumnType.instance)));

schema.add(KSMetaData.testMetadataNotDurable(ks_nocommit,
simple,
opts_rf1,
Expand Down
143 changes: 1 addition & 142 deletions test/unit/org/apache/cassandra/cql3/validation/ThriftIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ public void setupSuperColumnFamily() throws Throwable
{
final String denseTableName = createTableName();
final String sparseTableName = currentSparseTable();
final String counterTableName = currentCounterTable();

CfDef cfDef = new CfDef().setColumn_type("Super")
.setSubcomparator_type(Int32Type.instance.toString())
Expand All @@ -87,17 +86,9 @@ public void setupSuperColumnFamily() throws Throwable
.setKeyspace(KEYSPACE)
.setName(sparseTableName);

CfDef counterCfDef = new CfDef().setColumn_type("Super")
.setSubcomparator_type(AsciiType.instance.toString())
.setComparator_type(AsciiType.instance.toString())
.setDefault_validation_class(CounterColumnType.instance.toString())
.setKey_validation_class(AsciiType.instance.toString())
.setKeyspace(KEYSPACE)
.setName(counterTableName);

KsDef ksDef = new KsDef(KEYSPACE,
SimpleStrategy.class.getName(),
Arrays.asList(cfDef, sparseCfDef, counterCfDef));
Arrays.asList(cfDef, sparseCfDef));

ksDef.setStrategy_options(Collections.singletonMap("replication_factor", "1"));

Expand All @@ -112,103 +103,6 @@ public void tearDown() throws Throwable
getClient().send_system_drop_keyspace(KEYSPACE);
}

@Test
public void testReadCounter() throws Throwable
{
populateCounterTable();

UntypedResultSet resultSet = execute(String.format("select * from %s.%s", KEYSPACE, currentCounterTable()));
assertRows(resultSet,
row("key1", "ck1", "counter1", 10L),
row("key1", "ck1", "counter2", 5L),
row("key2", "ck1", "counter1", 10L),
row("key2", "ck1", "counter2", 5L));
}

@Test
public void testCounterTableThriftUpdates() throws Throwable
{
populateCounterTable();

Cassandra.Client client = getClient();
Mutation mutation = new Mutation();
ColumnOrSuperColumn csoc = new ColumnOrSuperColumn();
csoc.setCounter_super_column(new CounterSuperColumn(ByteBufferUtil.bytes("ck1"),
Arrays.asList(new CounterColumn(ByteBufferUtil.bytes("counter1"), 1))));
mutation.setColumn_or_supercolumn(csoc);

Mutation mutation2 = new Mutation();
ColumnOrSuperColumn csoc2 = new ColumnOrSuperColumn();
csoc2.setCounter_super_column(new CounterSuperColumn(ByteBufferUtil.bytes("ck1"),
Arrays.asList(new CounterColumn(ByteBufferUtil.bytes("counter1"), 100))));
mutation2.setColumn_or_supercolumn(csoc2);
client.batch_mutate(Collections.singletonMap(ByteBufferUtil.bytes("key1"),
Collections.singletonMap(currentCounterTable(), Arrays.asList(mutation))),
ONE);
client.batch_mutate(Collections.singletonMap(ByteBufferUtil.bytes("key2"),
Collections.singletonMap(currentCounterTable(), Arrays.asList(mutation2))),
ONE);

UntypedResultSet resultSet = execute(String.format("select * from %s.%s", KEYSPACE, currentCounterTable()));
assertRows(resultSet,
row("key1", "ck1", "counter1", 11L),
row("key1", "ck1", "counter2", 5L),
row("key2", "ck1", "counter1", 110L),
row("key2", "ck1", "counter2", 5L));
}

@Test
public void testCounterTableCqlUpdates() throws Throwable
{
populateCounterTable();

execute(String.format("UPDATE %s.%s set value = value + 1 WHERE key = ? AND column1 = ? AND column2 = ?", KEYSPACE, currentCounterTable()),
"key1", "ck1", "counter1");
execute(String.format("UPDATE %s.%s set value = value + 100 WHERE key = 'key2' AND column1 = 'ck1' AND column2 = 'counter1'", KEYSPACE, currentCounterTable()));

execute(String.format("UPDATE %s.%s set value = value - ? WHERE key = 'key1' AND column1 = 'ck1' AND column2 = 'counter2'", KEYSPACE, currentCounterTable()), 2L);
execute(String.format("UPDATE %s.%s set value = value - ? WHERE key = 'key2' AND column1 = 'ck1' AND column2 = 'counter2'", KEYSPACE, currentCounterTable()), 100L);

UntypedResultSet resultSet = execute(String.format("select * from %s.%s", KEYSPACE, currentCounterTable()));
assertRows(resultSet,
row("key1", "ck1", "counter1", 11L),
row("key1", "ck1", "counter2", 3L),
row("key2", "ck1", "counter1", 110L),
row("key2", "ck1", "counter2", -95L));
}

@Test
public void testCounterTableCqlDeletes() throws Throwable
{
populateCounterTable();

assertRows(execute(String.format("select * from %s.%s", KEYSPACE, currentCounterTable())),
row("key1", "ck1", "counter1", 10L),
row("key1", "ck1", "counter2", 5L),
row("key2", "ck1", "counter1", 10L),
row("key2", "ck1", "counter2", 5L));

execute(String.format("DELETE value FROM %s.%s WHERE key = ? AND column1 = ? AND column2 = ?", KEYSPACE, currentCounterTable()),
"key1", "ck1", "counter1");

assertRows(execute(String.format("select * from %s.%s", KEYSPACE, currentCounterTable())),
row("key1", "ck1", "counter2", 5L),
row("key2", "ck1", "counter1", 10L),
row("key2", "ck1", "counter2", 5L));

execute(String.format("DELETE FROM %s.%s WHERE key = ? AND column1 = ?", KEYSPACE, currentCounterTable()),
"key1", "ck1");

assertRows(execute(String.format("select * from %s.%s", KEYSPACE, currentCounterTable())),
row("key2", "ck1", "counter1", 10L),
row("key2", "ck1", "counter2", 5L));

execute(String.format("DELETE FROM %s.%s WHERE key = ?", KEYSPACE, currentCounterTable()),
"key2");

assertEmpty(execute(String.format("select * from %s.%s", KEYSPACE, currentCounterTable())));
}

@Test
public void alterDenseTable() throws Throwable
{
Expand Down Expand Up @@ -826,41 +720,6 @@ private void populateSparseTable() throws Throwable
ONE);
}

private void populateCounterTable() throws Throwable
{
Cassandra.Client client = getClient();

ColumnParent cp = new ColumnParent(currentCounterTable());
cp.setSuper_column(ByteBufferUtil.bytes("ck1"));
client.add(ByteBufferUtil.bytes("key1"),
cp,
new CounterColumn(ByteBufferUtil.bytes("counter1"), 10L),
ONE);
cp = new ColumnParent(currentCounterTable());
cp.setSuper_column(ByteBufferUtil.bytes("ck1"));
client.add(ByteBufferUtil.bytes("key1"),
cp,
new CounterColumn(ByteBufferUtil.bytes("counter2"), 5L),
ONE);
cp = new ColumnParent(currentCounterTable());
cp.setSuper_column(ByteBufferUtil.bytes("ck1"));
client.add(ByteBufferUtil.bytes("key2"),
cp,
new CounterColumn(ByteBufferUtil.bytes("counter1"), 10L),
ONE);
cp = new ColumnParent(currentCounterTable());
cp.setSuper_column(ByteBufferUtil.bytes("ck1"));
client.add(ByteBufferUtil.bytes("key2"),
cp,
new CounterColumn(ByteBufferUtil.bytes("counter2"), 5L),
ONE);
}

private String currentCounterTable()
{
return currentTable() + "_counter";
}

private String currentSparseTable()
{
return currentTable() + "_sparse";
Expand Down
102 changes: 5 additions & 97 deletions test/unit/org/apache/cassandra/cql3/validation/entities/CountersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,106 +21,14 @@
import org.junit.Test;

import org.apache.cassandra.cql3.CQLTester;
import org.apache.cassandra.exceptions.ConfigurationException;
import org.apache.cassandra.exceptions.InvalidRequestException;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class CountersTest extends CQLTester
{
/**
* Check for a table with counters,
* migrated from cql_tests.py:TestCQL.counters_test()
*/
@Test
public void testCounters() throws Throwable
{
createTable("CREATE TABLE %s (userid int, url text, total counter, PRIMARY KEY (userid, url)) WITH COMPACT STORAGE");

execute("UPDATE %s SET total = total + 1 WHERE userid = 1 AND url = 'http://foo.com'");
assertRows(execute("SELECT total FROM %s WHERE userid = 1 AND url = 'http://foo.com'"),
row(1L));

execute("UPDATE %s SET total = total - 4 WHERE userid = 1 AND url = 'http://foo.com'");
assertRows(execute("SELECT total FROM %s WHERE userid = 1 AND url = 'http://foo.com'"),
row(-3L));

execute("UPDATE %s SET total = total+1 WHERE userid = 1 AND url = 'http://foo.com'");
assertRows(execute("SELECT total FROM %s WHERE userid = 1 AND url = 'http://foo.com'"),
row(-2L));

execute("UPDATE %s SET total = total -2 WHERE userid = 1 AND url = 'http://foo.com'");
assertRows(execute("SELECT total FROM %s WHERE userid = 1 AND url = 'http://foo.com'"),
row(-4L));
public void testCounterColumnsRejected() {
assertThatThrownBy(() -> createTable("CREATE TABLE %s (userid int, url text, total counter, PRIMARY KEY (userid, url)) WITH COMPACT STORAGE"))
.hasStackTraceContaining("Palantir Cassandra does not support counter columns");
}

/**
* Test for the validation bug of #4706,
* migrated from cql_tests.py:TestCQL.validate_counter_regular_test()
*/
@Test
public void testRegularCounters() throws Throwable
{
assertInvalidThrowMessage("Cannot add a non counter column",
ConfigurationException.class,
String.format("CREATE TABLE %s.%s (id bigint PRIMARY KEY, count counter, things set<text>)", KEYSPACE, createTableName()));
}

/**
* Migrated from cql_tests.py:TestCQL.collection_counter_test()
*/
@Test
public void testCountersOnCollections() throws Throwable
{
String tableName = KEYSPACE + "." + createTableName();
assertInvalidThrow(InvalidRequestException.class,
String.format("CREATE TABLE %s (k int PRIMARY KEY, l list<counter>)", tableName));

tableName = KEYSPACE + "." + createTableName();
assertInvalidThrow(InvalidRequestException.class,
String.format("CREATE TABLE %s (k int PRIMARY KEY, s set<counter>)", tableName));

tableName = KEYSPACE + "." + createTableName();
assertInvalidThrow(InvalidRequestException.class,
String.format("CREATE TABLE %s (k int PRIMARY KEY, m map<text, counter>)", tableName));
}

@Test
public void testCounterUpdatesWithUnset() throws Throwable
{
createTable("CREATE TABLE %s (k int PRIMARY KEY, c counter)");

// set up
execute("UPDATE %s SET c = c + 1 WHERE k = 10");
assertRows(execute("SELECT c FROM %s WHERE k = 10"),
row(1L)
);
// increment
execute("UPDATE %s SET c = c + ? WHERE k = 10", 1L);
assertRows(execute("SELECT c FROM %s WHERE k = 10"),
row(2L)
);
execute("UPDATE %s SET c = c + ? WHERE k = 10", unset());
assertRows(execute("SELECT c FROM %s WHERE k = 10"),
row(2L) // no change to the counter value
);
// decrement
execute("UPDATE %s SET c = c - ? WHERE k = 10", 1L);
assertRows(execute("SELECT c FROM %s WHERE k = 10"),
row(1L)
);
execute("UPDATE %s SET c = c - ? WHERE k = 10", unset());
assertRows(execute("SELECT c FROM %s WHERE k = 10"),
row(1L) // no change to the counter value
);
}

/**
* Test for the validation bug of #9395.
*/
@Test
public void testProhibitReversedCounterAsPartOfPrimaryKey() throws Throwable
{
assertInvalidThrowMessage("counter type is not supported for PRIMARY KEY part a",
InvalidRequestException.class, String.format("CREATE TABLE %s.%s (a counter, b int, PRIMARY KEY (b, a)) WITH CLUSTERING ORDER BY (a desc);", KEYSPACE, createTableName()));
}

}
10 changes: 0 additions & 10 deletions test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,16 +347,6 @@ public void testIndexOnCompoundRowKey() throws Throwable
row("t", 1, 4, 3));
}

/**
* Migrated from cql_tests.py:TestCQL.secondary_index_counters()
*/
@Test
public void testIndexOnCountersInvalid() throws Throwable
{
createTable("CREATE TABLE %s (k int PRIMARY KEY, c counter)");
assertInvalid("CREATE INDEX ON test(c)");
}

/**
* Migrated from cql_tests.py:TestCQL.collection_indexing_test()
*/
Expand Down
21 changes: 0 additions & 21 deletions test/unit/org/apache/cassandra/cql3/validation/entities/TimestampTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,27 +122,6 @@ public void testInvalidCustomTimestamp() throws Throwable

execute("INSERT INTO %s (k, v) VALUES(1, 0) IF NOT EXISTS");
assertInvalid("INSERT INTO %s (k, v) VALUES(1, 1) IF NOT EXISTS USING TIMESTAMP 5");

// Counters
createTable("CREATE TABLE %s (k int PRIMARY KEY, c counter)");

execute("UPDATE %s SET c = c + 1 WHERE k = 0");
assertInvalid("UPDATE %s USING TIMESTAMP 10 SET c = c + 1 WHERE k = 0");

execute("BEGIN COUNTER BATCH " +
"UPDATE %1$s SET c = c + 1 WHERE k = 0; " +
"UPDATE %1$s SET c = c + 1 WHERE k = 0; " +
"APPLY BATCH");

assertInvalid("BEGIN COUNTER BATCH " +
"UPDATE %1$s USING TIMESTAMP 3 SET c = c + 1 WHERE k = 0; " +
"UPDATE %1$s SET c = c + 1 WHERE k = 0; " +
"APPLY BATCH");

assertInvalid("BEGIN COUNTER BATCH " +
"USING TIMESTAMP 3 UPDATE %1$s SET c = c + 1 WHERE k = 0; " +
"UPDATE %1$s SET c = c + 1 WHERE k = 0; " +
"APPLY BATCH");
}

@Test
Expand Down
Loading

0 comments on commit 6fdd4a7

Please sign in to comment.