From ec2f2e687dde75b30c09e0a676bb03fd62ac0cbb Mon Sep 17 00:00:00 2001 From: Berenguer Blasi Date: Tue, 3 Nov 2020 14:25:11 +0100 Subject: [PATCH] Avoid storing the last schema change in CQLTester patch by Berenguer Blasi; reviewed by David Capwell and Benjamin Lerer for CASSANDRA-16114 --- .../org/apache/cassandra/cql3/CQLTester.java | 60 +++++++++++----- .../cql3/validation/entities/UFTest.java | 71 ++++++++++--------- .../operations/AggregationTest.java | 63 ++++++++-------- 3 files changed, 110 insertions(+), 84 deletions(-) diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index 416a4b2d87..332d185945 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -124,8 +124,6 @@ public abstract class CQLTester } } - public static ResultMessage lastSchemaChangeResult; - private List tables = new ArrayList<>(); private List types = new ArrayList<>(); private List functions = new ArrayList<>(); @@ -380,24 +378,46 @@ public abstract class CQLTester return typeName; } + protected String createFunctionName(String keyspace) + { + return String.format("%s.function_%02d", keyspace, seqNumber.getAndIncrement()); + } + + protected void registerFunction(String functionName, String argTypes) + { + functions.add(functionName + '(' + argTypes + ')'); + } + protected String createFunction(String keyspace, String argTypes, String query) throws Throwable { - String functionName = keyspace + ".function_" + seqNumber.getAndIncrement(); + String functionName = createFunctionName(keyspace); + createFunctionOverload(functionName, argTypes, query); return functionName; } protected void createFunctionOverload(String functionName, String argTypes, String query) throws Throwable { + registerFunction(functionName, argTypes); String fullQuery = String.format(query, functionName); - functions.add(functionName + '(' + argTypes + ')'); logger.info(fullQuery); schemaChange(fullQuery); } + protected String createAggregateName(String keyspace) + { + return String.format("%s.aggregate_%02d", keyspace, seqNumber.getAndIncrement()); + } + + protected void registerAggregate(String aggregateName, String argTypes) + { + aggregates.add(aggregateName + '(' + argTypes + ')'); + } + protected String createAggregate(String keyspace, String argTypes, String query) throws Throwable { - String aggregateName = keyspace + "." + "aggregate_" + seqNumber.getAndIncrement(); + String aggregateName = createAggregateName(keyspace); + createAggregateOverload(aggregateName, argTypes, query); return aggregateName; } @@ -405,7 +425,7 @@ public abstract class CQLTester protected void createAggregateOverload(String aggregateName, String argTypes, String query) throws Throwable { String fullQuery = String.format(query, aggregateName); - aggregates.add(aggregateName + '(' + argTypes + ')'); + registerAggregate(aggregateName, argTypes); logger.info(fullQuery); schemaChange(fullQuery); } @@ -508,20 +528,24 @@ public abstract class CQLTester schemaChange(fullQuery); } - protected void assertLastSchemaChange(Event.SchemaChange.Change change, Event.SchemaChange.Target target, - String keyspace, String name, - String... argTypes) + protected static void assertSchemaChange(String query, + Event.SchemaChange.Change expectedChange, + Event.SchemaChange.Target expectedTarget, + String expectedKeyspace, + String expectedName, + String... expectedArgTypes) { - Assert.assertTrue(lastSchemaChangeResult instanceof ResultMessage.SchemaChange); - ResultMessage.SchemaChange schemaChange = (ResultMessage.SchemaChange) lastSchemaChangeResult; - Assert.assertSame(change, schemaChange.change.change); - Assert.assertSame(target, schemaChange.change.target); - Assert.assertEquals(keyspace, schemaChange.change.keyspace); - Assert.assertEquals(name, schemaChange.change.name); - Assert.assertEquals(argTypes != null ? Arrays.asList(argTypes) : null, schemaChange.change.argTypes); + ResultMessage actual = schemaChange(query); + Assert.assertTrue(actual instanceof ResultMessage.SchemaChange); + Event.SchemaChange schemaChange = ((ResultMessage.SchemaChange) actual).change; + Assert.assertSame(expectedChange, schemaChange.change); + Assert.assertSame(expectedTarget, schemaChange.target); + Assert.assertEquals(expectedKeyspace, schemaChange.keyspace); + Assert.assertEquals(expectedName, schemaChange.name); + Assert.assertEquals(expectedArgTypes != null ? Arrays.asList(expectedArgTypes) : null, schemaChange.argTypes); } - protected static void schemaChange(String query) + protected static ResultMessage schemaChange(String query) { try { @@ -534,7 +558,7 @@ public abstract class CQLTester QueryOptions options = QueryOptions.forInternalCalls(Collections.emptyList()); - lastSchemaChangeResult = prepared.statement.executeInternal(queryState, options); + return prepared.statement.executeInternal(queryState, options); } catch (Exception e) { diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java index d4d2a10a77..3c9052fc80 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/UFTest.java @@ -46,6 +46,8 @@ import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.service.ClientState; import org.apache.cassandra.transport.Event; import org.apache.cassandra.transport.Server; +import org.apache.cassandra.transport.Event.SchemaChange.Change; +import org.apache.cassandra.transport.Event.SchemaChange.Target; import org.apache.cassandra.transport.messages.ResultMessage; import org.apache.cassandra.utils.UUIDGen; @@ -74,45 +76,46 @@ public class UFTest extends CQLTester @Test public void testSchemaChange() throws Throwable { - String f = createFunction(KEYSPACE, - "double, double", - "CREATE OR REPLACE FUNCTION %s(state double, val double) " + - "RETURNS NULL ON NULL INPUT " + - "RETURNS double " + - "LANGUAGE javascript " + - "AS '\"string\";';"); + String f = createFunctionName(KEYSPACE); + String functionName = shortFunctionName(f); + registerFunction(f, "double, double"); - assertLastSchemaChange(Event.SchemaChange.Change.CREATED, Event.SchemaChange.Target.FUNCTION, - KEYSPACE, parseFunctionName(f).name, - "double", "double"); + assertSchemaChange("CREATE OR REPLACE FUNCTION " + f + "(state double, val double) " + + "RETURNS NULL ON NULL INPUT " + + "RETURNS double " + + "LANGUAGE javascript " + + "AS '\"string\";';", + Change.CREATED, + Target.FUNCTION, + KEYSPACE, functionName, + "double", "double"); - createFunctionOverload(f, - "double, double", - "CREATE OR REPLACE FUNCTION %s(state int, val int) " + - "RETURNS NULL ON NULL INPUT " + - "RETURNS int " + - "LANGUAGE javascript " + - "AS '\"string\";';"); + registerFunction(f, "int, int"); - assertLastSchemaChange(Event.SchemaChange.Change.CREATED, Event.SchemaChange.Target.FUNCTION, - KEYSPACE, parseFunctionName(f).name, - "int", "int"); + assertSchemaChange("CREATE OR REPLACE FUNCTION " + f + "(state int, val int) " + + "RETURNS NULL ON NULL INPUT " + + "RETURNS int " + + "LANGUAGE javascript " + + "AS '\"string\";';", + Change.CREATED, + Target.FUNCTION, + KEYSPACE, functionName, + "int", "int"); - schemaChange("CREATE OR REPLACE FUNCTION " + f + "(state int, val int) " + - "RETURNS NULL ON NULL INPUT " + - "RETURNS int " + - "LANGUAGE javascript " + - "AS '\"string\";';"); + assertSchemaChange("CREATE OR REPLACE FUNCTION " + f + "(state int, val int) " + + "RETURNS NULL ON NULL INPUT " + + "RETURNS int " + + "LANGUAGE javascript " + + "AS '\"string1\";';", + Change.UPDATED, + Target.FUNCTION, + KEYSPACE, functionName, + "int", "int"); - assertLastSchemaChange(Event.SchemaChange.Change.UPDATED, Event.SchemaChange.Target.FUNCTION, - KEYSPACE, parseFunctionName(f).name, - "int", "int"); - - schemaChange("DROP FUNCTION " + f + "(double, double)"); - - assertLastSchemaChange(Event.SchemaChange.Change.DROPPED, Event.SchemaChange.Target.FUNCTION, - KEYSPACE, parseFunctionName(f).name, - "double", "double"); + assertSchemaChange("DROP FUNCTION " + f + "(double, double)", + Change.DROPPED, Target.FUNCTION, + KEYSPACE, functionName, + "double", "double"); } @Test diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java index e7f47a2d36..631a4ccdd6 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java @@ -38,7 +38,8 @@ import org.apache.cassandra.db.SystemKeyspace; import org.apache.cassandra.exceptions.FunctionExecutionException; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.service.ClientState; -import org.apache.cassandra.transport.Event; +import org.apache.cassandra.transport.Event.SchemaChange.Change; +import org.apache.cassandra.transport.Event.SchemaChange.Target; import org.apache.cassandra.transport.messages.ResultMessage; import static org.junit.Assert.assertEquals; @@ -409,42 +410,40 @@ public class AggregationTest extends CQLTester "LANGUAGE javascript " + "AS '\"string\";';"); - String a = createAggregate(KEYSPACE, - "double", - "CREATE OR REPLACE AGGREGATE %s(double) " + - "SFUNC " + shortFunctionName(f) + " " + - "STYPE double " + - "INITCOND 0"); + String a = createAggregateName(KEYSPACE); + String aggregateName = shortFunctionName(a); + registerAggregate(a, "double"); - assertLastSchemaChange(Event.SchemaChange.Change.CREATED, Event.SchemaChange.Target.AGGREGATE, - KEYSPACE, parseFunctionName(a).name, - "double"); + assertSchemaChange("CREATE OR REPLACE AGGREGATE " + a + "(double) " + + "SFUNC " + shortFunctionName(f) + " " + + "STYPE double " + + "INITCOND 0", + Change.CREATED, Target.AGGREGATE, + KEYSPACE, aggregateName, + "double"); - schemaChange("CREATE OR REPLACE AGGREGATE " + a + "(double) " + - "SFUNC " + shortFunctionName(f) + " " + - "STYPE double " + - "INITCOND 0"); + assertSchemaChange("CREATE OR REPLACE AGGREGATE " + a + "(double) " + + "SFUNC " + shortFunctionName(f) + " " + + "STYPE double " + + "INITCOND 1", + Change.UPDATED, Target.AGGREGATE, + KEYSPACE, aggregateName, + "double"); - assertLastSchemaChange(Event.SchemaChange.Change.UPDATED, Event.SchemaChange.Target.AGGREGATE, - KEYSPACE, parseFunctionName(a).name, - "double"); + registerAggregate(a, "int"); - createAggregateOverload(a, - "int", - "CREATE OR REPLACE AGGREGATE %s(int) " + - "SFUNC " + shortFunctionName(f) + " " + - "STYPE int " + - "INITCOND 0"); + assertSchemaChange("CREATE OR REPLACE AGGREGATE " + a + "(int) " + + "SFUNC " + shortFunctionName(f) + " " + + "STYPE int " + + "INITCOND 0", + Change.CREATED, Target.AGGREGATE, + KEYSPACE, aggregateName, + "int"); - assertLastSchemaChange(Event.SchemaChange.Change.CREATED, Event.SchemaChange.Target.AGGREGATE, - KEYSPACE, parseFunctionName(a).name, - "int"); - - schemaChange("DROP AGGREGATE " + a + "(double)"); - - assertLastSchemaChange(Event.SchemaChange.Change.DROPPED, Event.SchemaChange.Target.AGGREGATE, - KEYSPACE, parseFunctionName(a).name, - "double"); + assertSchemaChange("DROP AGGREGATE " + a + "(double)", + Change.DROPPED, Target.AGGREGATE, + KEYSPACE, aggregateName, + "double"); } @Test