CASSANDRA-20155: To improve accord interoperability test coverage, nehttps://github.com/dcapwell/cassandra/pull/new/CASSANDRA-20155

src/java/org/apache/cassandra/cql3/statements/TransactionStatement.java

@@ -387,7 +387,15 @@ public ResultMessage execute(QueryState state, QueryOptions options, Dispatcher.
 
         // check again since now we have query options; note that statements are quaranted to be single partition reads at this point
         for (NamedSelect assignment : assignments)
+        {
             checkFalse(isSelectingMultipleClusterings(assignment.select, options), INCOMPLETE_PRIMARY_KEY_SELECT_MESSAGE, "LET assignment", assignment.select.source);
+            if (assignment.select.getRestrictions().keyIsInRelation())
+                checkTrue(assignment.select.getLimit(options) == DataLimits.NO_LIMIT, NO_PARTITION_IN_CLAUSE_WITH_LIMIT, "SELECT", assignment.select.source);
+        }
+        if (returningSelect != null && returningSelect.select.getRestrictions().keyIsInRelation())
+        {
+            checkTrue(returningSelect.select.getLimit(options) == DataLimits.NO_LIMIT, NO_PARTITION_IN_CLAUSE_WITH_LIMIT, "SELECT", returningSelect.select.source);
+        }
 
         Txn txn = createTxn(state.getClientState(), options);
 
@@ -492,8 +500,9 @@ private static void validate(SelectStatement prepared)
         if (prepared.hasAggregation())
             throw invalidRequest(NO_AGGREGATION_IN_TXNS_MESSAGE, "SELECT", prepared.source);
 
+        // when "LIMIT ?" this check can't be performed, so need to do again once the options are known
         if (prepared.getRestrictions().keyIsInRelation())
-            checkTrue(prepared.getLimit(null) == DataLimits.NO_LIMIT, NO_PARTITION_IN_CLAUSE_WITH_LIMIT, "SELECT", prepared.source);
+            checkTrue(prepared.isLimitMarker() || prepared.getLimit(null) == DataLimits.NO_LIMIT, NO_PARTITION_IN_CLAUSE_WITH_LIMIT, "SELECT", prepared.source);
     }
 
     public static class Parsed extends QualifiedStatement.Composite

test/distributed/org/apache/cassandra/distributed/test/accord/NewSchemaTest.java

@@ -75,7 +75,7 @@ private static Query read(String ks, String table, List<ByteBuffer> keys)
         assert !keys.isEmpty();
         Txn.Builder builder = new Txn.Builder();
         for (int i = 0; i < keys.size(); i++)
-            builder.addLet("row" + i, new Select.Builder().withWildcard().withTable(ks, table).withColumnEquals("pk", keys.get(i)));
+            builder.addLet("row" + i, new Select.Builder().wildcard().table(ks, table).value("pk", keys.get(i)));
         builder.addReturnReferences("row0.pk");
         Txn txn = builder.build();
         ByteBuffer[] binds = txn.bindsEncoded();

test/distributed/org/apache/cassandra/distributed/test/metrics/CoordinatorReadLatencyMetricTest.java

@@ -39,8 +39,7 @@
 import org.apache.cassandra.service.paxos.Paxos;
 import org.assertj.core.api.Assertions;
 
-import static org.apache.cassandra.cql3.ast.Where.Inequalities.EQUAL;
-import static org.apache.cassandra.cql3.ast.Where.Inequalities.LESS_THAN;
+import static org.apache.cassandra.cql3.ast.Conditional.Where.Inequality.LESS_THAN;
 import static org.junit.Assert.assertTrue;
 
 public class CoordinatorReadLatencyMetricTest extends TestBaseImpl
@@ -57,10 +56,10 @@ public void singleRowTest() throws IOException
             var select = Select.builder()
                                //TODO (now, correctness, coverage): count(v) breaks accord as we get mutliple rows rather than the count of rows...
 //                               .withSelection(FunctionCall.count("v"))
-                               .withTable(KEYSPACE, "tbl")
-                               .withWhere("pk", EQUAL, 0)
-                               .withWhere("ck", LESS_THAN, 42)
-                               .withLimit(1)
+                               .table(KEYSPACE, "tbl")
+                               .value("pk", 0)
+                               .where("ck", LESS_THAN, 42)
+                               .limit(1)
                                .build();
 
             verifyTableLatency(cluster, 1, () -> verifyLatencyMetrics(cluster, select.toCQL(), ConsistencyLevel.QUORUM));

test/distributed/org/apache/cassandra/fuzz/topology/AccordTopologyMixupTest.java

@@ -46,8 +46,15 @@
 import accord.utils.RandomSource;
 import org.apache.cassandra.config.CassandraRelevantProperties;
 import org.apache.cassandra.config.Config;
+import org.apache.cassandra.cql3.ast.CQLFormatter;
 import org.apache.cassandra.cql3.ast.Mutation;
+import org.apache.cassandra.cql3.ast.Select;
+import org.apache.cassandra.cql3.ast.StandardVisitors;
 import org.apache.cassandra.cql3.ast.Statement;
+import org.apache.cassandra.cql3.ast.Txn;
+import org.apache.cassandra.db.marshal.AsciiType;
+import org.apache.cassandra.db.marshal.BytesType;
+import org.apache.cassandra.db.marshal.UTF8Type;
 import org.apache.cassandra.distributed.Cluster;
 import org.apache.cassandra.distributed.api.ConsistencyLevel;
 import org.apache.cassandra.distributed.api.IInstanceConfig;
@@ -59,23 +66,38 @@
 import org.apache.cassandra.service.accord.api.AccordAgent;
 import org.apache.cassandra.service.consensus.TransactionalMode;
 import org.apache.cassandra.utils.ASTGenerators;
+import org.apache.cassandra.utils.AbstractTypeGenerators;
 import org.apache.cassandra.utils.CassandraGenerators;
+import org.apache.cassandra.utils.FastByteOperations;
+import org.apache.cassandra.utils.Generators;
 import org.apache.cassandra.utils.Isolated;
 import org.apache.cassandra.utils.Shared;
+import org.quicktheories.generators.SourceDSL;
 
+import static org.apache.cassandra.utils.AbstractTypeGenerators.overridePrimitiveTypeSupport;
+import static org.apache.cassandra.utils.AbstractTypeGenerators.stringComparator;
 import static org.apache.cassandra.utils.AccordGenerators.fromQT;
 
 public class AccordTopologyMixupTest extends TopologyMixupTestBase<AccordTopologyMixupTest.Spec>
 {
     private static final Logger logger = LoggerFactory.getLogger(AccordTopologyMixupTest.class);
 
+    /**
+     * Should the history show the CQL?  By default, this is off as its very verbose, but when debugging this can be helpful.
+     */
+    private static final boolean HISTORY_SHOWS_CQL = false;
+
     static
     {
         CassandraRelevantProperties.ACCORD_AGENT_CLASS.setString(InterceptAgent.class.getName());
         // enable most expensive debugging checks
         CassandraRelevantProperties.ACCORD_KEY_PARANOIA_CPU.setString(Invariants.Paranoia.QUADRATIC.name());
         CassandraRelevantProperties.ACCORD_KEY_PARANOIA_MEMORY.setString(Invariants.Paranoia.QUADRATIC.name());
         CassandraRelevantProperties.ACCORD_KEY_PARANOIA_COSTFACTOR.setString(Invariants.ParanoiaCostFactor.HIGH.name());
+
+        overridePrimitiveTypeSupport(AsciiType.instance, AbstractTypeGenerators.TypeSupport.of(AsciiType.instance, SourceDSL.strings().ascii().ofLengthBetween(1, 10), stringComparator(AsciiType.instance)));
+        overridePrimitiveTypeSupport(UTF8Type.instance, AbstractTypeGenerators.TypeSupport.of(UTF8Type.instance, Generators.utf8(1, 10), stringComparator(UTF8Type.instance)));
+        overridePrimitiveTypeSupport(BytesType.instance, AbstractTypeGenerators.TypeSupport.of(BytesType.instance, Generators.bytes(1, 10), FastByteOperations::compareUnsigned));
     }
 
     private static final List<TransactionalMode> TRANSACTIONAL_MODES = Stream.of(TransactionalMode.values()).filter(t -> t.accordIsEnabled).collect(Collectors.toList());
@@ -97,13 +119,18 @@ private static Spec createSchemaSpec(RandomSource rs, Cluster cluster)
     {
         TransactionalMode mode = rs.pick(TRANSACTIONAL_MODES);
         boolean enableMigration = allowsMigration(mode) && rs.nextBoolean();
+        // This test puts a focus on topology / cluster operations, so schema "shouldn't matter"... limit the domain of the test to improve the ability to debug
+        AbstractTypeGenerators.TypeGenBuilder supportedTypes = AbstractTypeGenerators.withoutUnsafeEquality(AbstractTypeGenerators.builder()
+                                                                                                                                  .withTypeKinds(AbstractTypeGenerators.TypeKind.PRIMITIVE));
         TableMetadata metadata = fromQT(new CassandraGenerators.TableMetadataBuilder()
                                         .withKeyspaceName(KEYSPACE)
+                                        .withTableName("tbl")
                                         .withTableKinds(TableMetadata.Kind.REGULAR)
                                         .withKnownMemtables()
+                                        .withSimpleColumnNames()
                                         //TODO (coverage): include "fast_path = 'keyspace'" override
                                         .withTransactionalMode(enableMigration ? TransactionalMode.off : mode)
-                                        .withoutEmpty()
+                                        .withDefaultTypeGen(supportedTypes)
                                         .build())
                                  .next(rs);
         maybeCreateUDTs(cluster, metadata);
@@ -133,14 +160,16 @@ private static CommandGen<Spec> cqlOperations(Spec spec)
 
     private static Command<State<Spec>, Void, ?> cqlOperation(RandomSource rs, State<Spec> state, Gen<Statement> statementGen)
     {
-        Statement stmt = statementGen.next(rs);
-        String cql;
-        //TODO (usability): are there any transaction_modes that actually need simple mutations/select to be wrapped in a BEGIN TRANSACTION?  If not then this logica can be simplified
-        if (stmt.kind() == Statement.Kind.TXN || stmt.kind() == Statement.Kind.MUTATION && ((Mutation) stmt).isCas())
-            cql = stmt.toCQL();
-        else cql = wrapInTxn(stmt.toCQL());
+        Statement stmt = statementGen.map(s -> {
+            if (s.kind() == Statement.Kind.TXN || s.kind() == Statement.Kind.MUTATION && ((Mutation) s).isCas())
+                return s;
+            return s instanceof Select ? Txn.wrap((Select) s) : Txn.wrap((Mutation) s);
+        }).next(rs);
         IInvokableInstance node = state.cluster.get(rs.pickInt(state.topologyHistory.up()));
-        return new Property.SimpleCommand<>(node + ": " + stmt.kind() + "; epoch=" + state.currentEpoch.get(), s2 -> executeTxn(s2.cluster, node, cql, stmt.bindsEncoded()));
+        String msg = HISTORY_SHOWS_CQL ?
+                     "\n" + stmt.visit(StandardVisitors.DEBUG).toCQL(new CQLFormatter.PrettyPrint()) + "\n"
+                     : stmt.kind() == Statement.Kind.MUTATION ? ((Mutation) stmt).mutationKind().name() : stmt.kind().name();
+        return new Property.SimpleCommand<>(node + ":" + msg + "; epoch=" + state.currentEpoch.get(), s2 -> executeTxn(s2.cluster, node, stmt.toCQL(), stmt.bindsEncoded()));
     }
 
     private static boolean allowsMigration(TransactionalMode mode)
@@ -199,6 +228,12 @@ public String keyspace()
         {
             return metadata.keyspace;
         }
+
+        @Override
+        public String createSchema()
+        {
+            return metadata.toCqlString(false, false, false);
+        }
     }
 
     private static class AccordState extends State<Spec>

test/distributed/org/apache/cassandra/fuzz/topology/HarryTopologyMixupTest.java

@@ -266,6 +266,12 @@ public String keyspace()
         {
             return schema.keyspace;
         }
+
+        @Override
+        public String createSchema()
+        {
+            return schema.compile();
+        }
     }
 
     public class HarryState extends State<Spec>

test/distributed/org/apache/cassandra/fuzz/topology/TopologyMixupTestBase.java

@@ -469,6 +469,7 @@ public interface Schema
     {
         String table();
         String keyspace();
+        String createSchema();
     }
 
     protected interface CommandGen<S extends Schema>
@@ -733,6 +734,8 @@ public String toString()
         {
             StringBuilder sb = new StringBuilder();
             sb.append("Yaml Config:\n").append(YamlConfigurationLoader.toYaml(this.yamlConfigOverrides));
+            String cql = schema.createSchema();
+            sb.append("\n-- Setup Schema\n").append(cql);
             sb.append("\nTopology:\n").append(topologyHistory);
             sb.append("\nCMS Voting Group: ").append(Arrays.toString(cmsGroup));
             if (epochHistory != null)