You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by al...@apache.org on 2020/07/27 15:25:20 UTC

[cassandra] branch trunk updated (a821214 -> 9e61249)

This is an automated email from the ASF dual-hosted git repository.

aleksey pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git.


    from a821214  CASSANDRA-15942 Improve tooling testing framework
     add 25fd7bd  Add support in jvm dtest to test thrift
     add ebf9c74  Merge branch 'cassandra-2.2' into cassandra-3.0
     add 8c61214  Merge branch 'cassandra-3.0' into cassandra-3.11
     add dc725bc  Forbid altering UDTs used in partition keys
     add c61f390  Merge branch 'cassandra-3.0' into cassandra-3.11
     new 9e61249  Merge branch 'cassandra-3.11' into trunk

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 CHANGES.txt                                        |  1 +
 .../cql3/statements/schema/AlterTypeStatement.java | 22 ++++++++++++++++++++++
 .../cql3/validation/operations/AlterTest.java      | 20 ++++++++++++++++++++
 3 files changed, 43 insertions(+)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


[cassandra] 01/01: Merge branch 'cassandra-3.11' into trunk

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

aleksey pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git

commit 9e61249764a3a7e08abded87602b118e2d4e4681
Merge: a821214 c61f390
Author: Aleksey Yeshchenko <al...@apache.org>
AuthorDate: Mon Jul 27 16:20:02 2020 +0100

    Merge branch 'cassandra-3.11' into trunk

 CHANGES.txt                                        |  1 +
 .../cql3/statements/schema/AlterTypeStatement.java | 22 ++++++++++++++++++++++
 .../cql3/validation/operations/AlterTest.java      | 20 ++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --cc CHANGES.txt
index a36f379,812e020..1acd5a2
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@@ -1,10 -1,7 +1,11 @@@
 -3.11.8
 +4.0-beta2
++ * Forbid altering UDTs used in partition keys (CASSANDRA-15933)
 + * Fix version parsing logic when upgrading from 3.0 (CASSANDRA-15973)
 + * Optimize NoSpamLogger use in hot paths (CASSANDRA-15766)
 + * Verify sstable components on startup (CASSANDRA-15945)
 +Merged from 3.11:
   * Frozen RawTuple is not annotated with frozen in the toString method (CASSANDRA-15857)
  Merged from 3.0:
 - * Forbid altering UDTs used in partition keys (CASSANDRA-15933)
   * Fix empty/null json string representation (CASSANDRA-15896)
  Merged from 2.2:
   * Fix CQL parsing of collections when the column type is reversed (CASSANDRA-15814)
diff --cc src/java/org/apache/cassandra/cql3/statements/schema/AlterTypeStatement.java
index f883038,0000000..9228f34
mode 100644,000000..100644
--- a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTypeStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTypeStatement.java
@@@ -1,234 -1,0 +1,256 @@@
 +/*
 + * Licensed to the Apache Software Foundation (ASF) under one
 + * or more contributor license agreements.  See the NOTICE file
 + * distributed with this work for additional information
 + * regarding copyright ownership.  The ASF licenses this file
 + * to you 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.apache.cassandra.cql3.statements.schema;
 +
 +import java.util.ArrayList;
++import java.util.Collection;
 +import java.util.HashMap;
 +import java.util.List;
 +import java.util.Map;
 +
 +import org.apache.cassandra.audit.AuditLogContext;
 +import org.apache.cassandra.audit.AuditLogEntryType;
 +import org.apache.cassandra.auth.Permission;
 +import org.apache.cassandra.cql3.*;
 +import org.apache.cassandra.db.marshal.AbstractType;
 +import org.apache.cassandra.db.marshal.UserType;
 +import org.apache.cassandra.schema.KeyspaceMetadata;
 +import org.apache.cassandra.schema.Keyspaces;
++import org.apache.cassandra.schema.TableMetadata;
 +import org.apache.cassandra.service.ClientState;
 +import org.apache.cassandra.transport.Event.SchemaChange;
 +import org.apache.cassandra.transport.Event.SchemaChange.Change;
 +import org.apache.cassandra.transport.Event.SchemaChange.Target;
 +
++import static com.google.common.collect.Iterables.any;
++import static com.google.common.collect.Iterables.filter;
++import static com.google.common.collect.Iterables.transform;
 +import static java.lang.String.join;
 +import static java.util.function.Predicate.isEqual;
 +import static java.util.stream.Collectors.toList;
 +
 +import static org.apache.cassandra.utils.ByteBufferUtil.bytes;
 +
 +public abstract class AlterTypeStatement extends AlterSchemaStatement
 +{
 +    protected final String typeName;
 +
 +    public AlterTypeStatement(String keyspaceName, String typeName)
 +    {
 +        super(keyspaceName);
 +        this.typeName = typeName;
 +    }
 +
 +    public void authorize(ClientState client)
 +    {
 +        client.ensureKeyspacePermission(keyspaceName, Permission.ALTER);
 +    }
 +
 +    SchemaChange schemaChangeEvent(Keyspaces.KeyspacesDiff diff)
 +    {
 +        return new SchemaChange(Change.UPDATED, Target.TYPE, keyspaceName, typeName);
 +    }
 +
 +    public Keyspaces apply(Keyspaces schema)
 +    {
 +        KeyspaceMetadata keyspace = schema.getNullable(keyspaceName);
 +
 +        UserType type = null == keyspace
 +                      ? null
 +                      : keyspace.types.getNullable(bytes(typeName));
 +
 +        if (null == type)
 +            throw ire("Type %s.%s doesn't exist", keyspaceName, typeName);
 +
 +        return schema.withAddedOrUpdated(keyspace.withUpdatedUserType(apply(keyspace, type)));
 +    }
 +
 +    abstract UserType apply(KeyspaceMetadata keyspace, UserType type);
 +
 +    @Override
 +    public AuditLogContext getAuditLogContext()
 +    {
 +        return new AuditLogContext(AuditLogEntryType.ALTER_TYPE, keyspaceName, typeName);
 +    }
 +
 +    public String toString()
 +    {
 +        return String.format("%s (%s, %s)", getClass().getSimpleName(), keyspaceName, typeName);
 +    }
 +
 +    private static final class AddField extends AlterTypeStatement
 +    {
 +        private final FieldIdentifier fieldName;
 +        private final CQL3Type.Raw type;
 +
 +        private AddField(String keyspaceName, String typeName, FieldIdentifier fieldName, CQL3Type.Raw type)
 +        {
 +            super(keyspaceName, typeName);
 +            this.fieldName = fieldName;
 +            this.type = type;
 +        }
 +
 +        UserType apply(KeyspaceMetadata keyspace, UserType userType)
 +        {
 +            if (userType.fieldPosition(fieldName) >= 0)
 +                throw ire("Cannot add field %s to type %s: a field with name %s already exists", fieldName, userType.getCqlTypeName(), fieldName);
 +
 +            AbstractType<?> fieldType = type.prepare(keyspaceName, keyspace.types).getType();
 +            if (fieldType.referencesUserType(userType.name))
 +                throw ire("Cannot add new field %s of type %s to user type %s as it would create a circular reference", fieldName, type, userType.getCqlTypeName());
 +
++            Collection<TableMetadata> tablesWithTypeInPartitionKey = findTablesReferencingTypeInPartitionKey(keyspace, userType);
++            if (!tablesWithTypeInPartitionKey.isEmpty())
++            {
++                throw ire("Cannot add new field %s of type %s to user type %s as the type is being used in partition key by the following tables: %s",
++                          fieldName, type, userType.getCqlTypeName(),
++                          String.join(", ", transform(tablesWithTypeInPartitionKey, TableMetadata::toString)));
++            }
++
 +            List<FieldIdentifier> fieldNames = new ArrayList<>(userType.fieldNames()); fieldNames.add(fieldName);
 +            List<AbstractType<?>> fieldTypes = new ArrayList<>(userType.fieldTypes()); fieldTypes.add(fieldType);
 +
 +            return new UserType(keyspaceName, userType.name, fieldNames, fieldTypes, true);
 +        }
++
++        private static Collection<TableMetadata> findTablesReferencingTypeInPartitionKey(KeyspaceMetadata keyspace, UserType userType)
++        {
++            Collection<TableMetadata> tables = new ArrayList<>();
++            filter(keyspace.tablesAndViews(),
++                   table -> any(table.partitionKeyColumns(), column -> column.type.referencesUserType(userType.name)))
++                  .forEach(tables::add);
++            return tables;
++        }
 +    }
 +
 +    private static final class RenameFields extends AlterTypeStatement
 +    {
 +        private final Map<FieldIdentifier, FieldIdentifier> renamedFields;
 +
 +        private RenameFields(String keyspaceName, String typeName, Map<FieldIdentifier, FieldIdentifier> renamedFields)
 +        {
 +            super(keyspaceName, typeName);
 +            this.renamedFields = renamedFields;
 +        }
 +
 +        UserType apply(KeyspaceMetadata keyspace, UserType userType)
 +        {
 +            List<String> dependentAggregates =
 +                keyspace.functions
 +                        .udas()
 +                        .filter(uda -> null != uda.initialCondition() && uda.stateType().referencesUserType(userType.name))
 +                        .map(uda -> uda.name().toString())
 +                        .collect(toList());
 +
 +            if (!dependentAggregates.isEmpty())
 +            {
 +                throw ire("Cannot alter user type %s as it is still used in INITCOND by aggregates %s",
 +                          userType.getCqlTypeName(),
 +                          join(", ", dependentAggregates));
 +            }
 +
 +            List<FieldIdentifier> fieldNames = new ArrayList<>(userType.fieldNames());
 +
 +            renamedFields.forEach((oldName, newName) ->
 +            {
 +                int idx = userType.fieldPosition(oldName);
 +                if (idx < 0)
 +                    throw ire("Unkown field %s in user type %s", oldName, keyspaceName, userType.getCqlTypeName());
 +                fieldNames.set(idx, newName);
 +            });
 +
 +            fieldNames.forEach(name ->
 +            {
 +                if (fieldNames.stream().filter(isEqual(name)).count() > 1)
 +                    throw ire("Duplicate field name %s in type %s", name, keyspaceName, userType.getCqlTypeName());
 +            });
 +
 +            return new UserType(keyspaceName, userType.name, fieldNames, userType.fieldTypes(), true);
 +        }
 +    }
 +
 +    private static final class AlterField extends AlterTypeStatement
 +    {
 +        private AlterField(String keyspaceName, String typeName)
 +        {
 +            super(keyspaceName, typeName);
 +        }
 +
 +        UserType apply(KeyspaceMetadata keyspace, UserType userType)
 +        {
 +            throw ire("Alterting field types is no longer supported");
 +        }
 +    }
 +
 +    public static final class Raw extends CQLStatement.Raw
 +    {
 +        private enum Kind
 +        {
 +            ADD_FIELD, RENAME_FIELDS, ALTER_FIELD
 +        }
 +
 +        private final UTName name;
 +
 +        private Kind kind;
 +
 +        // ADD
 +        private FieldIdentifier newFieldName;
 +        private CQL3Type.Raw newFieldType;
 +
 +        // RENAME
 +        private final Map<FieldIdentifier, FieldIdentifier> renamedFields = new HashMap<>();
 +
 +        public Raw(UTName name)
 +        {
 +            this.name = name;
 +        }
 +
 +        public AlterTypeStatement prepare(ClientState state)
 +        {
 +            String keyspaceName = name.hasKeyspace() ? name.getKeyspace() : state.getKeyspace();
 +            String typeName = name.getStringTypeName();
 +
 +            switch (kind)
 +            {
 +                case     ADD_FIELD: return new AddField(keyspaceName, typeName, newFieldName, newFieldType);
 +                case RENAME_FIELDS: return new RenameFields(keyspaceName, typeName, renamedFields);
 +                case   ALTER_FIELD: return new AlterField(keyspaceName, typeName);
 +            }
 +
 +            throw new AssertionError();
 +        }
 +
 +        public void add(FieldIdentifier name, CQL3Type.Raw type)
 +        {
 +            kind = Kind.ADD_FIELD;
 +            newFieldName = name;
 +            newFieldType = type;
 +        }
 +
 +        public void rename(FieldIdentifier from, FieldIdentifier to)
 +        {
 +            kind = Kind.RENAME_FIELDS;
 +            renamedFields.put(from, to);
 +        }
 +
 +        public void alter(FieldIdentifier name, CQL3Type.Raw type)
 +        {
 +            kind = Kind.ALTER_FIELD;
 +        }
 +    }
 +}
diff --cc test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
index fc3a61e,1d6b064..069669d
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
@@@ -631,4 -496,74 +631,24 @@@ public class AlterTest extends CQLTeste
  
          assertEmpty(execute("SELECT * FROM %s"));
      }
+ 
 -    /**
 -     * Test for CASSANDRA-13917
 -     */
 -    @Test
 -    public void testAlterWithCompactStaticFormat() throws Throwable
 -    {
 -        createTable("CREATE TABLE %s (a int PRIMARY KEY, b int, c int) WITH COMPACT STORAGE");
 -
 -        assertInvalidMessage("Cannot rename unknown column column1 in keyspace",
 -                             "ALTER TABLE %s RENAME column1 TO column2");
 -
 -        assertInvalidMessage("Cannot rename unknown column value in keyspace",
 -                             "ALTER TABLE %s RENAME value TO value2");
 -    }
 -
 -    /**
 -     * Test for CASSANDRA-13917
 -     */
 -    @Test
 -    public void testAlterWithCompactNonStaticFormat() throws Throwable
 -    {
 -        createTable("CREATE TABLE %s (a int, b int, PRIMARY KEY (a, b)) WITH COMPACT STORAGE");
 -        assertInvalidMessage("Cannot rename unknown column column1 in keyspace",
 -                             "ALTER TABLE %s RENAME column1 TO column2");
 -
 -        createTable("CREATE TABLE %s (a int, b int, v int, PRIMARY KEY (a, b)) WITH COMPACT STORAGE");
 -        assertInvalidMessage("Cannot rename unknown column column1 in keyspace",
 -                             "ALTER TABLE %s RENAME column1 TO column2");
 -    }
 -
 -    @Test
 -    public void testAlterTableAlterType() throws Throwable
 -    {
 -        createTable("CREATE TABLE %s (a int, b int, PRIMARY KEY (a,b)) WITH COMPACT STORAGE");
 -        assertInvalidMessage(String.format("Compact value type can only be changed to BytesType, but %s was given.",
 -                                           IntegerType.instance),
 -                             "ALTER TABLE %s ALTER value TYPE 'org.apache.cassandra.db.marshal.IntegerType'");
 -
 -        execute("ALTER TABLE %s ALTER value TYPE 'org.apache.cassandra.db.marshal.BytesType'");
 -
 -        createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a,b)) WITH COMPACT STORAGE");
 -        assertInvalidMessage("Altering of types is not allowed",
 -                             "ALTER TABLE %s ALTER c TYPE 'org.apache.cassandra.db.marshal.BytesType'");
 -
 -        createTable("CREATE TABLE %s (a int, value int, PRIMARY KEY (a,value)) WITH COMPACT STORAGE");
 -        assertInvalidMessage("Altering of types is not allowed",
 -                             "ALTER TABLE %s ALTER value TYPE 'org.apache.cassandra.db.marshal.IntegerType'");
 -        execute("ALTER TABLE %s ALTER value1 TYPE 'org.apache.cassandra.db.marshal.BytesType'");
 -    }
 -
+     @Test
+     public void testAlterTypeUsedInPartitionKey() throws Throwable
+     {
+         // frozen UDT used directly in a partition key
+         String  type1 = createType("CREATE TYPE %s (v1 int)");
+         String table1 = createTable("CREATE TABLE %s (pk frozen<" + type1 + ">, val int, PRIMARY KEY(pk));");
+ 
+         // frozen UDT used in a frozen UDT used in a partition key
+         String  type2 = createType("CREATE TYPE %s (v1 frozen<" + type1 + ">, v2 frozen<" + type1 + ">)");
+         String table2 = createTable("CREATE TABLE %s (pk frozen<" + type2 + ">, val int, PRIMARY KEY(pk));");
+ 
+         // frozen UDT used in a frozen collection used in a partition key
+         String table3 = createTable("CREATE TABLE %s (pk frozen<list<frozen<" + type1 + ">>>, val int, PRIMARY KEY(pk));");
+ 
+         // assert that ALTER fails and that the error message contains all the names of the table referencing it
+         assertInvalidMessage(table1, format("ALTER TYPE %s.%s ADD v2 int;", keyspace(), type1));
+         assertInvalidMessage(table2, format("ALTER TYPE %s.%s ADD v2 int;", keyspace(), type1));
+         assertInvalidMessage(table3, format("ALTER TYPE %s.%s ADD v2 int;", keyspace(), type1));
+     }
  }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org