natas_hw
/
cassandra
mirror of https://github.com/apache/cassandra
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix schema diffs when comments or security labels are modified 

Patch by Sam Tunnicliffe; reviewed by Marcus Eriksson for
CASSANDRA-21045
This commit is contained in:
Sam Tunnicliffe 2025-11-26 17:30:24 +00:00
parent 209288cebe
commit dbfcd52fd5
7 changed files with 309 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/java/org/apache/cassandra/cql3/ColumnSpecification.java
View File

@ -28,28 +28,17 @@ import java.util.Iterator;
public class ColumnSpecification
{
protected static final String EMPTY_COMMENT = "";
protected static final String EMPTY_SECURITY_LABEL = "";
public final String ksName;
public final String cfName;
public final ColumnIdentifier name;
public final AbstractType<?> type;
public final String comment;
public final String securityLabel;
public ColumnSpecification(String ksName, String cfName, ColumnIdentifier name, AbstractType<?> type)
{
this(ksName, cfName, name, type, EMPTY_COMMENT, EMPTY_SECURITY_LABEL);
}
public ColumnSpecification(String ksName, String cfName, ColumnIdentifier name, AbstractType<?> type, String comment, String securityLabel)
{
this.ksName = ksName;
this.cfName = cfName;
this.name = name;
this.type = type;
this.comment = comment == null ? EMPTY_COMMENT : comment;
this.securityLabel = securityLabel == null ? EMPTY_SECURITY_LABEL : securityLabel;
}
/**
@ -60,7 +49,7 @@ public class ColumnSpecification
*/
public ColumnSpecification withAlias(ColumnIdentifier alias)
{
return new ColumnSpecification(ksName, cfName, alias, type, comment, securityLabel);
return new ColumnSpecification(ksName, cfName, alias, type);
}
public boolean isReversedType()
@ -111,8 +100,6 @@ public class ColumnSpecification
return MoreObjects.toStringHelper(this)
.add("name", name)
.add("type", type)
.add("comment", comment)
.add("securityLabel", securityLabel)
.toString();
}
}

11
src/java/org/apache/cassandra/db/marshal/UserType.java
View File

@ -366,7 +366,7 @@ public class UserType extends TupleType implements SchemaElement
@Override
public int hashCode()
{
return Objects.hashCode(keyspace, name, fieldNames, types, isMultiCell);
return Objects.hashCode(keyspace, name, fieldNames, types, isMultiCell, comment, securityLabel, fieldComments, fieldSecurityLabels);
}
@Override
@ -413,11 +413,18 @@ public class UserType extends TupleType implements SchemaElement
return name.equals(other.name)
&& fieldNames.equals(other.fieldNames)
&& keyspace.equals(other.keyspace)
&& isMultiCell == other.isMultiCell;
&& isMultiCell == other.isMultiCell
&& comment.equals(other.comment)
&& securityLabel.equals(other.securityLabel)
&& fieldComments.equals(other.fieldComments)
&& fieldSecurityLabels.equals(other.fieldSecurityLabels);
}
public boolean equalsWithOutKs(UserType other)
{
// Doesn't consider comments or security labels at either the
// type or field level as this method is used to check compatibility of
// UserTypes in different keyspaces for validation in CopyTableStatement
return name.equals(other.name)
&& fieldNames.equals(other.fieldNames)
&& types.equals(other.types)

19
src/java/org/apache/cassandra/schema/ColumnMetadata.java
View File

@ -76,6 +76,8 @@ public final class ColumnMetadata extends ColumnSpecification implements Selecta
public static final int NO_POSITION = -1;
public static final int NO_UNIQUE_ID = Integer.MIN_VALUE;
public static final String EMPTY_COMMENT = "";
public static final String EMPTY_SECURITY_LABEL = "";
public enum ClusteringOrder
{
@ -118,6 +120,9 @@ public final class ColumnMetadata extends ColumnSpecification implements Selecta
private final int position;
public final int uniqueId;
public final String comment;
public final String securityLabel;
private final Comparator<CellPath> cellPathComparator;
private final Comparator<Object> asymmetricCellPathComparator;
private final Comparator<? super Cell<?>> cellComparator;
@ -266,7 +271,7 @@ public final class ColumnMetadata extends ColumnSpecification implements Selecta
String comment,
String securityLabel)
{
super(ksName, cfName, name, type, comment, securityLabel);
super(ksName, cfName, name, type);
this.uniqueId = uniqueId;
assert name != null && type != null && kind != null;
assert (position == NO_POSITION) == !kind.isPrimaryKeyKind(); // The position really only make sense for partition and clustering columns (and those must have one),
@ -286,6 +291,8 @@ public final class ColumnMetadata extends ColumnSpecification implements Selecta
this.mask = mask;
this.columnConstraints = columnConstraints;
this.columnConstraints.setColumnName(name);
this.comment = comment;
this.securityLabel = securityLabel;
}
private static Comparator<CellPath> makeCellPathComparator(Kind kind, AbstractType<?> type)
@ -458,7 +465,9 @@ public final class ColumnMetadata extends ColumnSpecification implements Selecta
&& ksName.equals(other.ksName)
&& cfName.equals(other.cfName)
&& Objects.equals(mask, other.mask)
&& Objects.equals(columnConstraints, other.columnConstraints);
&& Objects.equals(columnConstraints, other.columnConstraints)
&& comment.equals(other.comment)
&& securityLabel.equals(other.securityLabel);
}
Optional<Difference> compare(ColumnMetadata other)
@ -490,6 +499,8 @@ public final class ColumnMetadata extends ColumnSpecification implements Selecta
result = 31 * result + position;
result = 31 * result + (mask == null ? 0 : mask.hashCode());
result = 31 * result + (columnConstraints == null ? 0 : columnConstraints.hashCode());
result = 31 * result + (comment == null ? 0 : comment.hashCode());
result = 31 * result + (securityLabel == null ? 0 : securityLabel.hashCode());
hash = result;
}
return result;
@ -508,6 +519,10 @@ public final class ColumnMetadata extends ColumnSpecification implements Selecta
.add("type", type)
.add("kind", kind)
.add("position", position)
.add("mask", mask)
.add("columnConstraints", columnConstraints)
.add("comment", comment)
.add("securityLabel", securityLabel)
.toString();
}

8
src/java/org/apache/cassandra/schema/KeyspaceParams.java
View File

@ -161,13 +161,17 @@ public final class KeyspaceParams
KeyspaceParams p = (KeyspaceParams) o;
return durableWrites == p.durableWrites && replication.equals(p.replication) && fastPath.equals(p.fastPath);
return durableWrites == p.durableWrites
&& replication.equals(p.replication)
&& fastPath.equals(p.fastPath)
&& comment.equals(p.comment)
&& securityLabel.equals(p.securityLabel);
}
@Override
public int hashCode()
{
return Objects.hashCode(durableWrites, replication, fastPath);
return Objects.hashCode(durableWrites, replication, fastPath, comment, securityLabel);
}
@Override

132
test/unit/org/apache/cassandra/cql3/CommentAndSecurityLabelClientTest.java Normal file
View File

@ -0,0 +1,132 @@
/*
* 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;
import java.io.IOException;
import java.util.List;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.apache.cassandra.ServerTestUtils;
import org.apache.cassandra.db.ConsistencyLevel;
import org.apache.cassandra.transport.ClientNotificationsTest;
import org.apache.cassandra.transport.Event;
import org.apache.cassandra.transport.ProtocolVersion;
import org.apache.cassandra.transport.SimpleClient;
import org.apache.cassandra.transport.messages.RegisterMessage;
import org.apache.cassandra.transport.messages.ResultMessage;
import static java.lang.String.format;
import static org.apache.cassandra.transport.Event.SchemaChange.Change.UPDATED;
import static org.apache.cassandra.transport.Event.SchemaChange.Target.TABLE;
import static org.apache.cassandra.transport.Event.SchemaChange.Target.TYPE;
import static org.junit.Assert.assertEquals;
public class CommentAndSecurityLabelClientTest extends CQLTester
{
private static final String KEYSPACE_NAME = "ks_comment";
private static final String TABLE_NAME = "tbl_comment";
private static final String COLUMN_NAME = "name";
private static final String TYPE_NAME = "type_comment";
private static final String FIELD_NAME = "f1";
@BeforeClass
public static void setUpClass()
{
ServerTestUtils.daemonInitialization();
CQLTester.setUpClass();
}
@Before
public void setup()
{
requireNetwork();
}
@Test
public void testClientResponses() throws IOException
{
createSchema();
try (SimpleClient client = newSimpleClient(ProtocolVersion.CURRENT))
{
// Register to receive schema change notifications, the client uses a simple event handler to
// record and inspect these.
client.execute(new RegisterMessage(List.of(Event.Type.SCHEMA_CHANGE)));
ClientNotificationsTest.EventHandler handler = new ClientNotificationsTest.EventHandler();
client.setEventHandler(handler);
// Comments
makeRequestAndVerify(client, handler,
format("COMMENT ON KEYSPACE %s IS 'test comment'", KEYSPACE_NAME),
new Event.SchemaChange(UPDATED, KEYSPACE_NAME));
makeRequestAndVerify(client, handler,
format("COMMENT ON TABLE %s.%s IS 'test comment'", KEYSPACE_NAME, TABLE_NAME),
new Event.SchemaChange(UPDATED, TABLE, KEYSPACE_NAME, TABLE_NAME));
makeRequestAndVerify(client, handler,
format("COMMENT ON COLUMN %s.%s.%s IS 'test comment'", KEYSPACE_NAME, TABLE_NAME, COLUMN_NAME),
new Event.SchemaChange(UPDATED, TABLE, KEYSPACE_NAME, TABLE_NAME));
makeRequestAndVerify(client, handler,
format("COMMENT ON TYPE %s.%s IS 'test comment'", KEYSPACE_NAME, TYPE_NAME),
new Event.SchemaChange(UPDATED, TYPE, KEYSPACE_NAME, TYPE_NAME));
makeRequestAndVerify(client, handler,
format("COMMENT ON FIELD %s.%s.%s IS 'test comment'", KEYSPACE_NAME, TYPE_NAME, FIELD_NAME),
new Event.SchemaChange(UPDATED, TYPE, KEYSPACE_NAME, TYPE_NAME));
// Security labels
makeRequestAndVerify(client, handler,
format("SECURITY LABEL ON KEYSPACE %s IS 'test label'", KEYSPACE_NAME),
new Event.SchemaChange(UPDATED, KEYSPACE_NAME));
makeRequestAndVerify(client, handler,
format("SECURITY LABEL ON TABLE %s.%s IS 'test label'", KEYSPACE_NAME, TABLE_NAME),
new Event.SchemaChange(UPDATED, TABLE, KEYSPACE_NAME, TABLE_NAME));
makeRequestAndVerify(client, handler,
format("SECURITY LABEL ON COLUMN %s.%s.%s IS 'test label'", KEYSPACE_NAME, TABLE_NAME, COLUMN_NAME),
new Event.SchemaChange(UPDATED, TABLE, KEYSPACE_NAME, TABLE_NAME));
makeRequestAndVerify(client, handler,
format("SECURITY LABEL ON TYPE %s.%s IS 'test label'", KEYSPACE_NAME, TYPE_NAME),
new Event.SchemaChange(UPDATED, TYPE, KEYSPACE_NAME, TYPE_NAME));
makeRequestAndVerify(client, handler,
format("SECURITY LABEL ON FIELD %s.%s.%s IS 'test label'", KEYSPACE_NAME, TYPE_NAME, FIELD_NAME),
new Event.SchemaChange(UPDATED, TYPE, KEYSPACE_NAME, TYPE_NAME));
}
}
private void makeRequestAndVerify(SimpleClient client,
ClientNotificationsTest.EventHandler handler,
String cql,
Event.SchemaChange expected)
{
// Assert that the correct response type and content is received
ResultMessage result = client.execute(cql, ConsistencyLevel.ONE);
assertEquals(result.kind, ResultMessage.Kind.SCHEMA_CHANGE);
ResultMessage.SchemaChange message = (ResultMessage.SchemaChange) result;
assertEquals(message.change, expected);
// Verify that the expected notification was received and passed to the event handler
handler.assertNextEvent(expected);
}
private void createSchema()
{
createKeyspace(format("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}", KEYSPACE_NAME));
createTable(format("CREATE TABLE %s.%s (id int PRIMARY KEY, name text)", KEYSPACE_NAME, TABLE_NAME));
execute(format("CREATE TYPE %s.%s (%s int)", KEYSPACE_NAME, TYPE_NAME, FIELD_NAME));
}
}

142
test/unit/org/apache/cassandra/schema/SchemaChangesTest.java
View File

@ -20,11 +20,16 @@ package org.apache.cassandra.schema;
import java.nio.ByteBuffer;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Supplier;
import com.google.common.collect.ImmutableMap;
import org.apache.cassandra.cql3.FieldIdentifier;
import org.apache.cassandra.db.marshal.Int32Type;
import org.apache.cassandra.db.marshal.UserType;
import org.apache.cassandra.exceptions.InvalidRequestException;
import org.junit.BeforeClass;
import org.junit.Rule;
@ -49,6 +54,7 @@ import org.apache.cassandra.io.sstable.Descriptor;
import org.apache.cassandra.io.sstable.format.SSTableFormat.Components;
import org.apache.cassandra.io.util.File;
import org.apache.cassandra.locator.NetworkTopologyStrategy;
import org.apache.cassandra.utils.ByteBufferUtil;
import org.apache.cassandra.utils.FBUtilities;
import static org.apache.cassandra.Util.throwAssert;
@ -582,6 +588,142 @@ public class SchemaChangesTest
assertEquals(keyspace1, diff.altered.get(0).after);
}
@Test
public void testColumnComment()
{
TableMetadata t1 = addTestTable("ks1", "t", "");
ColumnIdentifier col = new ColumnIdentifier("col", false);
assertNotNull(t1.getColumn(col));
TableMetadata t2 = t1.unbuild().alterColumnComment(col, "added comment").build();
verifyTableDifferencesDetected(t1, t2);
}
@Test
public void testColumnSecurityLabel()
{
TableMetadata t1 = addTestTable("ks1", "t", "");
ColumnIdentifier col = new ColumnIdentifier("col", false);
assertNotNull(t1.getColumn(col));
TableMetadata t2 = t1.unbuild().alterColumnSecurityLabel(col, "added security label").build();
verifyTableDifferencesDetected(t1, t2);
}
@Test
public void testTableComment()
{
TableMetadata t1 = addTestTable("ks1", "t", "");
assertEquals("", t1.params.comment);
TableMetadata t2 = t1.unbuild().comment("added comment").build();
verifyTableDifferencesDetected(t1, t2);
}
@Test
public void testTableSecurityLabel()
{
TableMetadata t1 = addTestTable("ks1", "t", "");
assertEquals("", t1.params.comment);
TableMetadata t2 = t1.unbuild().securityLabel("added security label").build();
verifyTableDifferencesDetected(t1, t2);
}
@Test
public void testKeyspaceComment()
{
KeyspaceMetadata ks1 = KeyspaceMetadata.create("ks1", KeyspaceParams.simple(1));
KeyspaceMetadata ks2 = ks1.withSwapped(ks1.params.withComment("added comment"));
verifyKeyspaceDifferencesDetected(ks1, ks2);
}
@Test
public void testKeyspaceSecurityLabel()
{
KeyspaceMetadata ks1 = KeyspaceMetadata.create("ks1", KeyspaceParams.simple(1));
KeyspaceMetadata ks2 = ks1.withSwapped(ks1.params.withSecurityLabel("added security label"));
verifyKeyspaceDifferencesDetected(ks1, ks2);
}
@Test
public void testUserTypeComment()
{
UserType t1 = new UserType("ks1",
ByteBufferUtil.bytes("type_1"),
List.of(FieldIdentifier.forUnquoted("f1"), FieldIdentifier.forUnquoted("f2")),
List.of(Int32Type.instance, Int32Type.instance),
true);
UserType t2 = t1.withComment("added comment");
verifyUserTypeDifferencesDetected(t1, t2);
}
@Test
public void testUserTypeSecurityLabel()
{
UserType t1 = new UserType("ks1",
ByteBufferUtil.bytes("type_1"),
List.of(FieldIdentifier.forUnquoted("f1"), FieldIdentifier.forUnquoted("f2")),
List.of(Int32Type.instance, Int32Type.instance),
true);
UserType t2 = t1.withSecurityLabel("added comment");
verifyUserTypeDifferencesDetected(t1, t2);
}
@Test
public void testUserTypeFieldComment()
{
UserType t1 = new UserType("ks1",
ByteBufferUtil.bytes("type_1"),
List.of(FieldIdentifier.forUnquoted("f1"), FieldIdentifier.forUnquoted("f2")),
List.of(Int32Type.instance, Int32Type.instance),
true);
UserType t2 = t1.withFieldComment(FieldIdentifier.forUnquoted("f1"), "added comment");
verifyUserTypeDifferencesDetected(t1, t2);
}
@Test
public void testUserTypeFieldSecurityLabel()
{
UserType t1 = new UserType("ks1",
ByteBufferUtil.bytes("type_1"),
List.of(FieldIdentifier.forUnquoted("f1"), FieldIdentifier.forUnquoted("f2")),
List.of(Int32Type.instance, Int32Type.instance),
true);
UserType t2 = t1.withFieldSecurityLabel(FieldIdentifier.forUnquoted("f2"), "added comment");
verifyUserTypeDifferencesDetected(t1, t2);
}
private void verifyUserTypeDifferencesDetected(UserType t1, UserType t2)
{
assertFalse(t1.equals(t2));
Types types1 = Types.of(t1);
Types types2 = Types.of(t2);
Types.TypesDiff diff = Types.diff(types1, types2);
assertEquals(1, diff.altered.size());
Diff.Altered<UserType> altered = diff.altered.iterator().next();
assertEquals(t1, altered.before);
assertEquals(t2, altered.after);
}
private void verifyKeyspaceDifferencesDetected(KeyspaceMetadata ks1, KeyspaceMetadata ks2)
{
assertFalse(ks1.equals(ks2));
Optional<KeyspaceMetadata.KeyspaceDiff> opt = KeyspaceMetadata.diff(ks1, ks2);
assertTrue(opt.isPresent());
KeyspaceMetadata.KeyspaceDiff diff = opt.get();
assertEquals(ks1, diff.before);
assertEquals(ks2, diff.after);
}
private static void verifyTableDifferencesDetected(TableMetadata t1, TableMetadata t2)
{
assertFalse(t1.equals(t2));
Tables tables1 = Tables.of(t1);
Tables tables2 = Tables.of(t2);
Tables.TablesDiff diff = Tables.diff(tables1, tables2);
assertEquals(1, diff.altered.size());
Diff.Altered<TableMetadata> altered = diff.altered.iterator().next();
assertEquals(altered.before, t1);
assertEquals(altered.after, t2);
}
private TableMetadata addTestTable(String ks, String cf, String comment)
{
return

4
test/unit/org/apache/cassandra/transport/ClientNotificiationsTest.java → test/unit/org/apache/cassandra/transport/ClientNotificationsTest.java
View File

@ -37,7 +37,7 @@ import org.apache.cassandra.utils.FBUtilities;
import static org.junit.Assert.assertEquals;
@RunWith(Parameterized.class)
public class ClientNotificiationsTest extends CQLTester
public class ClientNotificationsTest extends CQLTester
{
private static Server.EventNotifier notifier = new Server.EventNotifier();
@ -101,7 +101,7 @@ public class ClientNotificiationsTest extends CQLTester
}
}
static class EventHandler extends SimpleClient.SimpleEventHandler
public static class EventHandler extends SimpleClient.SimpleEventHandler
{
public void assertNextEvent(Event expected)
{