Expand DDL statements on coordinator before submission to the CMS

Patch by Alex Petrov; reviewed by Stefan Miklosovic and Sam Tunnicliffe for CASSANDRA-19592
apache · May 23, 2024 · 7fe30fc · 7fe30fc
1 parent df78296
commit 7fe30fc
Show file tree

Hide file tree

Showing 23 changed files with 580 additions and 69 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,4 +1,5 @@
 5.1
+ * Expand DDL statements on coordinator before submission to the CMS (CASSANDRA-19592)
  * Fix number of arguments of String.format() in various classes (CASSANDRA-19645)
  * Remove unused fields from config (CASSANDRA-19599)
  * Refactor Relation and Restriction hierarchies (CASSANDRA-19341)

diff --git a/src/java/org/apache/cassandra/cql3/SchemaElement.java b/src/java/org/apache/cassandra/cql3/SchemaElement.java
@@ -89,9 +89,10 @@ default String elementKeyspaceQuotedIfNeeded()
     /**
      * Returns a CQL representation of this element
      *
-     * @param withInternals if the internals part of the CQL should be exposed.
-     * @param ifNotExists if "IF NOT EXISTS" should be included.
+     * @param withWarnings if commented warnings should be included
+     * @param withInternals if the internals part of the CQL should be exposed
+     * @param ifNotExists if "IF NOT EXISTS" should be included
      * @return a CQL representation of this element
      */
-    String toCqlString(boolean withInternals, boolean ifNotExists);
+    String toCqlString(boolean withWarnings, boolean withInternals, boolean ifNotExists);
 }
diff --git a/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java b/src/java/org/apache/cassandra/cql3/functions/UDAggregate.java
@@ -350,7 +350,7 @@ public SchemaElementType elementType()
     }
 
     @Override
-    public String toCqlString(boolean withInternals, boolean ifNotExists)
+    public String toCqlString(boolean withWarnings, boolean withInternals, boolean ifNotExists)
     {
         CqlBuilder builder = new CqlBuilder();
         builder.append("CREATE AGGREGATE ");

diff --git a/src/java/org/apache/cassandra/cql3/functions/UDFunction.java b/src/java/org/apache/cassandra/cql3/functions/UDFunction.java
@@ -324,7 +324,7 @@ public SchemaElementType elementType()
     }
 
     @Override
-    public String toCqlString(boolean withInternals, boolean ifNotExists)
+    public String toCqlString(boolean withWarnings, boolean withInternals, boolean ifNotExists)
     {
         CqlBuilder builder = new CqlBuilder();
         builder.append("CREATE FUNCTION ");

diff --git a/src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java b/src/java/org/apache/cassandra/cql3/statements/DescribeStatement.java
@@ -370,7 +370,7 @@ protected List<ByteBuffer> toRow(SchemaElement element, boolean withInternals)
                 return ImmutableList.of(bytes(element.elementKeyspaceQuotedIfNeeded()),
                                         bytes(element.elementType().toString()),
                                         bytes(element.elementNameQuotedIfNeeded()),
-                                        bytes(element.toCqlString(withInternals, false)));
+                                        bytes(element.toCqlString(true, withInternals, false)));
             }
         };
     }
@@ -420,7 +420,7 @@ protected List<ByteBuffer> toRow(SchemaElement element, boolean withInternals)
             return ImmutableList.of(bytes(element.elementKeyspaceQuotedIfNeeded()),
                                     bytes(element.elementType().toString()),
                                     bytes(element.elementNameQuotedIfNeeded()),
-                                    bytes(element.toCqlString(withInternals, false)));
+                                    bytes(element.toCqlString(true, withInternals, false)));
         }
     }
 
@@ -570,7 +570,7 @@ public String elementName()
                     }
 
                     @Override
-                    public String toCqlString(boolean withInternals, boolean ifNotExists)
+                    public String toCqlString(boolean withWarnings, boolean withInternals, boolean ifNotExists)
                     {
                         return index.toCqlString(table, ifNotExists);
                     }

diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateIndexStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateIndexStatement.java
@@ -85,6 +85,7 @@ public final class CreateIndexStatement extends AlterSchemaStatement
     private final List<IndexTarget.Raw> rawIndexTargets;
     private final IndexAttributes attrs;
     private final boolean ifNotExists;
+    private String expandedCql;
 
     private ClientState state;
 
@@ -103,6 +104,14 @@ public CreateIndexStatement(String keyspaceName,
         this.ifNotExists = ifNotExists;
     }
 
+    @Override
+    public String cql()
+    {
+        if (expandedCql != null)
+            return expandedCql;
+        return super.cql();
+    }
+
     @Override
     public void validate(ClientState state)
     {
@@ -192,6 +201,8 @@ public Keyspaces apply(ClusterMetadata metadata)
             throw ire(INDEX_DUPLICATE_OF_EXISTING, index.name, equalIndex.name);
         }
 
+        this.expandedCql = index.toCqlString(table, ifNotExists);
+
         TableMetadata newTable = table.withSwapped(table.indexes.with(index));
         newTable.validate();
 

diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
@@ -49,6 +49,7 @@ public final class CreateKeyspaceStatement extends AlterSchemaStatement
 
     private final KeyspaceAttributes attrs;
     private final boolean ifNotExists;
+    private String expandedCql;
 
     public CreateKeyspaceStatement(String keyspaceName, KeyspaceAttributes attrs, boolean ifNotExists)
     {
@@ -57,6 +58,15 @@ public CreateKeyspaceStatement(String keyspaceName, KeyspaceAttributes attrs, bo
         this.ifNotExists = ifNotExists;
     }
 
+    @Override
+    public String cql()
+    {
+        if (expandedCql != null)
+            return expandedCql;
+        return super.cql();
+    }
+
+    @Override
     public Keyspaces apply(ClusterMetadata metadata)
     {
         attrs.validate();
@@ -76,17 +86,20 @@ public Keyspaces apply(ClusterMetadata metadata)
             throw new AlreadyExistsException(keyspaceName);
         }
 
-        KeyspaceMetadata keyspace = KeyspaceMetadata.create(keyspaceName, attrs.asNewKeyspaceParams());
+        KeyspaceMetadata keyspaceMetadata = KeyspaceMetadata.create(keyspaceName, attrs.asNewKeyspaceParams());
 
-        if (keyspace.params.replication.klass.equals(LocalStrategy.class))
+        if (keyspaceMetadata.params.replication.klass.equals(LocalStrategy.class))
             throw ire("Unable to use given strategy class: LocalStrategy is reserved for internal use.");
 
-        if (keyspace.params.replication.isMeta())
+        if (keyspaceMetadata.params.replication.isMeta())
             throw ire("Can not create a keyspace with MetaReplicationStrategy");
 
-        keyspace.params.validate(keyspaceName, state, metadata);
-        keyspace.replicationStrategy.validateExpectedOptions(metadata);
-        return schema.withAddedOrUpdated(keyspace);
+        keyspaceMetadata.params.validate(keyspaceName, state, metadata);
+        keyspaceMetadata.replicationStrategy.validateExpectedOptions(metadata);
+
+        this.expandedCql = keyspaceMetadata.toCqlString(false, true, ifNotExists);
+
+        return schema.withAddedOrUpdated(keyspaceMetadata);
     }
 
     SchemaChange schemaChangeEvent(KeyspacesDiff diff)

diff --git a/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java b/src/java/org/apache/cassandra/cql3/statements/schema/CreateTableStatement.java
@@ -65,6 +65,8 @@ public final class CreateTableStatement extends AlterSchemaStatement
     private final boolean ifNotExists;
     private final boolean useCompactStorage;
 
+    private String expandedCql;
+
     public CreateTableStatement(String keyspaceName,
                                 String tableName,
                                 Map<ColumnIdentifier, ColumnProperties.Raw> rawColumns,
@@ -91,6 +93,14 @@ public CreateTableStatement(String keyspaceName,
         this.useCompactStorage = useCompactStorage;
     }
 
+    @Override
+    public String cql()
+    {
+        if (expandedCql != null)
+            return expandedCql;
+        return super.cql();
+    }
+
     public Keyspaces apply(ClusterMetadata metadata)
     {
         Keyspaces schema = metadata.schema.getKeyspaces();
@@ -113,7 +123,11 @@ public Keyspaces apply(ClusterMetadata metadata)
             ufBuilder.add(ksm.userFunctions);
 
         TableMetadata.Builder builder = builder(keyspace.types, ufBuilder.build()).epoch(metadata.nextEpoch());
-        if (!builder.hasId() && !DatabaseDescriptor.useDeterministicTableID())
+
+        // We do not want to set table ID here just yet, since we are using CQL for serialising a fully expanded CREATE TABLE statement.
+        this.expandedCql = builder.build().toCqlString(false, attrs.hasProperty(TableAttributes.ID), ifNotExists);
+
+        if (!attrs.hasProperty(TableAttributes.ID) && !DatabaseDescriptor.useDeterministicTableID())
             builder.id(TableId.get(metadata));
         TableMetadata table = builder.build();
         table.validate();

diff --git a/src/java/org/apache/cassandra/db/SchemaCQLHelper.java b/src/java/org/apache/cassandra/db/SchemaCQLHelper.java
@@ -70,14 +70,14 @@ public static String getTableMetadataAsCQL(TableMetadata metadata, KeyspaceMetad
              * first argument(withInternals) indicates to include table metadata id and clustering columns order,
              * second argument(ifNotExists) instructs to include IF NOT EXISTS statement within creation statements.
              */
-            return viewMetadata.toCqlString(true, true);
+            return viewMetadata.toCqlString(true, true, true);
         }
 
         /*
          * With addition to withInternals and ifNotExists arguments, includeDroppedColumns will include dropped
          * columns as ALTER TABLE statements appended into the snapshot.
          */
-        return metadata.toCqlString(true, true, true);
+        return metadata.toCqlString(true, true, true, true);
     }
 
     /**
@@ -122,7 +122,7 @@ public static Stream<String> getUserTypesAsCQL(TableMetadata metadata, Types typ
          */
         return metadata.getReferencedUserTypes()
                        .stream()
-                       .map(name -> getType(metadata, types, name).toCqlString(false, ifNotExists));
+                       .map(name -> getType(metadata, types, name).toCqlString(true, false, ifNotExists));
     }
 
     /**

diff --git a/src/java/org/apache/cassandra/db/marshal/UserType.java b/src/java/org/apache/cassandra/db/marshal/UserType.java
@@ -500,7 +500,7 @@ public String elementName()
     }
 
     @Override
-    public String toCqlString(boolean withInternals, boolean ifNotExists)
+    public String toCqlString(boolean withWarnings, boolean withInternals, boolean ifNotExists)
     {
         CqlBuilder builder = new CqlBuilder();
         builder.append("CREATE TYPE ");

diff --git a/src/java/org/apache/cassandra/schema/IndexMetadata.java b/src/java/org/apache/cassandra/schema/IndexMetadata.java
@@ -314,6 +314,10 @@ public void appendCqlTo(CqlBuilder builder, TableMetadata table, boolean ifNotEx
                    .append(" (")
                    .append(options.get(IndexTarget.TARGET_OPTION_NAME))
                    .append(')');
+
+            builder.append(" USING '")
+                   .append(CassandraIndex.NAME)
+                   .append("'");
         }
         builder.append(';');
     }

diff --git a/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java b/src/java/org/apache/cassandra/schema/KeyspaceMetadata.java
@@ -290,10 +290,10 @@ public String elementName()
     }
 
     @Override
-    public String toCqlString(boolean withInternals, boolean ifNotExists)
+    public String toCqlString(boolean withWarnings, boolean withInternals, boolean ifNotExists)
     {
         CqlBuilder builder = new CqlBuilder();
-        if (isVirtual())
+        if (isVirtual() && withWarnings)
         {
             builder.append("/*")
                    .newLine()

diff --git a/src/java/org/apache/cassandra/schema/TableId.java b/src/java/org/apache/cassandra/schema/TableId.java
@@ -67,13 +67,13 @@ public static TableId fromString(String idString)
         return new TableId(UUID.fromString(idString));
     }
 
-    public static TableId get(ClusterMetadata metadata)
+    public static TableId get(ClusterMetadata prev)
     {
         int i = 0;
         while (true)
         {
-            TableId tableId = TableId.fromLong(metadata.epoch.getEpoch() + i);
-            if (!tableIdExists(metadata, tableId))
+            TableId tableId = TableId.fromLong(prev.epoch.getEpoch() + i);
+            if (!tableIdExists(prev, tableId))
                 return tableId;
             i++;
         }
@@ -105,7 +105,7 @@ private static TableId fromHexString(String nonDashUUID)
         return fromUUID(new UUID(msb, lsb));
     }
 
-    private static TableId fromLong(long start)
+    public static TableId fromLong(long start)
     {
         return TableId.fromUUID(new UUID(MAGIC, start));
     }