beobal commented on code in PR #3921:
URL: https://github.com/apache/cassandra/pull/3921#discussion_r1983754198
##########
src/java/org/apache/cassandra/schema/TableMetadata.java:
##########
@@ -455,6 +455,14 @@ public ColumnMetadata getColumn(ColumnIdentifier name)
{
return columns.get(name.bytes);
}
+
+ /**
+ * Returns if {@code name} is in ColumnMetadata.
+ */
+ public boolean containtsColumn(ColumnIdentifier name)
+ {
+ return columns.containsKey(name.bytes);
+ }
/**
Review Comment:
nit: missing line break
##########
src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java:
##########
@@ -710,65 +710,46 @@ private void validateCanDropCompactStorage()
}
}
- public static class DropConstraints extends AlterTableStatement
- {
- final ColumnIdentifier columnName;
-
- DropConstraints(String keyspaceName, String tableName, boolean
ifTableExists, ColumnIdentifier columnName)
- {
- super(keyspaceName, tableName, ifTableExists);
- this.columnName = columnName;
- }
-
- @Override
- public KeyspaceMetadata apply(Epoch epoch, KeyspaceMetadata keyspace,
TableMetadata table, ClusterMetadata metadata)
- {
- ColumnMetadata columnMetadata = table.getColumn(columnName);
- columnMetadata.removeColumnConstraints();
-
- TableMetadata.Builder tableBuilder = table.unbuild().epoch(epoch);
- Views.Builder viewsBuilder = keyspace.views.unbuild();
- TableMetadata tableMetadata = tableBuilder.build();
- tableMetadata.validate();
-
- return
keyspace.withSwapped(keyspace.tables.withSwapped(tableMetadata))
- .withSwapped(viewsBuilder.build());
- }
- }
-
public static class AlterConstraints extends AlterTableStatement
{
final ColumnIdentifier columnName;
- final ColumnConstraints constraints;
+ final ColumnConstraints.Raw constraints;
+ final boolean ifColumnExists;
- AlterConstraints(String keyspaceName, String tableName, boolean
ifTableExists, ColumnIdentifier columnName, ColumnConstraints constraints)
+ AlterConstraints(String keyspaceName, String tableName, boolean
ifTableExists, boolean ifColumnExists, ColumnIdentifier columnName,
ColumnConstraints.Raw constraints)
{
super(keyspaceName, tableName, ifTableExists);
this.columnName = columnName;
this.constraints = constraints;
+ this.ifColumnExists = ifColumnExists;
}
@Override
public KeyspaceMetadata apply(Epoch epoch, KeyspaceMetadata keyspace,
TableMetadata table, ClusterMetadata metadata)
{
- TableMetadata.Builder tableBuilder = table.unbuild().epoch(epoch);
- for (ColumnMetadata column : tableBuilder.columns())
+ if (table.containtsColumn(columnName))
Review Comment:
The other 2 subclasses (`MaskColumn/DropColumns`) that do this verification
check whether `table.getColumn(columnName)` returns null. Any reason why you
didn't follow that precedent and chose to add a new method to `TableMetadata`
instead, especially as you actually do call `getColumn` anyway.
##########
src/java/org/apache/cassandra/schema/TableMetadata.java:
##########
@@ -455,6 +455,14 @@ public ColumnMetadata getColumn(ColumnIdentifier name)
{
return columns.get(name.bytes);
}
+
+ /**
+ * Returns if {@code name} is in ColumnMetadata.
+ */
+ public boolean containtsColumn(ColumnIdentifier name)
Review Comment:
typo: `containts`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
