This is an automated email from the ASF dual-hosted git repository.
jmckenzie pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 09c89e5 Further restrict schema column drop/recreate conversions
09c89e5 is described below
commit 09c89e5f5f8604301c233130dfb6e82a36ae30f3
Author: Aleksey Yeschenko <[email protected]>
AuthorDate: Wed Sep 1 12:22:31 2021 -0400
Further restrict schema column drop/recreate conversions
patch by Aleksey Yeschenko; reviewed by Blake Eggleston, Sam Tunnicliffe,
and Caleb Rackliffe for CASSANDRA-16905
Co-authored by Aleksey Yeschenko ([email protected])
Co-authored by Josh McKenzie ([email protected])
---
.../statements/schema/AlterTableStatement.java | 2 +-
.../apache/cassandra/db/marshal/AbstractType.java | 19 +++++++++++--
.../cassandra/db/marshal/CollectionType.java | 11 +++++++-
.../apache/cassandra/db/marshal/ReversedType.java | 6 ----
.../cql3/validation/operations/AlterTest.java | 32 ++++++++++++++++++++++
5 files changed, 60 insertions(+), 10 deletions(-)
diff --git
a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
index f361b64..c8cf045 100644
---
a/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
+++
b/src/java/org/apache/cassandra/cql3/statements/schema/AlterTableStatement.java
@@ -206,7 +206,7 @@ public abstract class AlterTableStatement extends
AlterSchemaStatement
{
// After #8099, not safe to re-add columns of incompatible
types - until *maybe* deser logic with dropped
// columns is pushed deeper down the line. The latter would
still be problematic in cases of schema races.
- if (!type.isValueCompatibleWith(droppedColumn.type))
+ if (!type.isSerializationCompatibleWith(droppedColumn.type))
{
throw ire("Cannot re-add previously dropped column '%s' of
type %s, incompatible with previous type %s",
name,
diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractType.java
b/src/java/org/apache/cassandra/db/marshal/AbstractType.java
index f171e7f..3a18f76 100644
--- a/src/java/org/apache/cassandra/db/marshal/AbstractType.java
+++ b/src/java/org/apache/cassandra/db/marshal/AbstractType.java
@@ -32,6 +32,7 @@ import org.apache.cassandra.cql3.AssignmentTestable;
import org.apache.cassandra.cql3.CQL3Type;
import org.apache.cassandra.cql3.ColumnSpecification;
import org.apache.cassandra.cql3.Term;
+import org.apache.cassandra.db.rows.Cell;
import org.apache.cassandra.exceptions.SyntaxException;
import org.apache.cassandra.io.util.DataInputPlus;
import org.apache.cassandra.io.util.DataOutputPlus;
@@ -318,9 +319,11 @@ public abstract class AbstractType<T> implements
Comparator<ByteBuffer>, Assignm
*
* Note that a type should be compatible with at least itself.
*/
- public boolean isValueCompatibleWith(AbstractType<?> otherType)
+ public boolean isValueCompatibleWith(AbstractType<?> previous)
{
- return isValueCompatibleWithInternal((otherType instanceof
ReversedType) ? ((ReversedType) otherType).baseType : otherType);
+ AbstractType<?> thisType = isReversed() ? ((ReversedType<?>)
this).baseType : this;
+ AbstractType<?> thatType = previous.isReversed() ? ((ReversedType<?>)
previous).baseType : previous;
+ return thisType.isValueCompatibleWithInternal(thatType);
}
/**
@@ -333,6 +336,18 @@ public abstract class AbstractType<T> implements
Comparator<ByteBuffer>, Assignm
}
/**
+ * Similar to {@link #isValueCompatibleWith(AbstractType)}, but takes into
account {@link Cell} encoding.
+ * In particular, this method doesn't consider two types serialization
compatible if one of them has fixed
+ * length (overrides {@link #valueLengthIfFixed()}, and the other one
doesn't.
+ */
+ public boolean isSerializationCompatibleWith(AbstractType<?> previous)
+ {
+ return isValueCompatibleWith(previous)
+ && valueLengthIfFixed() == previous.valueLengthIfFixed()
+ && isMultiCell() == previous.isMultiCell();
+ }
+
+ /**
* An alternative comparison function used by CollectionsType in
conjunction with CompositeType.
*
* This comparator is only called to compare components of a
CompositeType. It gets the value of the
diff --git a/src/java/org/apache/cassandra/db/marshal/CollectionType.java
b/src/java/org/apache/cassandra/db/marshal/CollectionType.java
index 0d627a5..c52cddc 100644
--- a/src/java/org/apache/cassandra/db/marshal/CollectionType.java
+++ b/src/java/org/apache/cassandra/db/marshal/CollectionType.java
@@ -175,7 +175,7 @@ public abstract class CollectionType<T> extends
AbstractType<T>
return false;
// the value comparator is only used for Cell values, so sorting
doesn't matter
- return
this.valueComparator().isValueCompatibleWith(tprev.valueComparator());
+ return
this.valueComparator().isSerializationCompatibleWith(tprev.valueComparator());
}
@Override
@@ -199,6 +199,15 @@ public abstract class CollectionType<T> extends
AbstractType<T>
return isValueCompatibleWithFrozen(tprev);
}
+ @Override
+ public boolean isSerializationCompatibleWith(AbstractType<?> previous)
+ {
+ if (!isValueCompatibleWith(previous))
+ return false;
+
+ return
valueComparator().isSerializationCompatibleWith(((CollectionType<?>)previous).valueComparator());
+ }
+
/** A version of isCompatibleWith() to deal with non-multicell (frozen)
collections */
protected abstract boolean isCompatibleWithFrozen(CollectionType<?>
previous);
diff --git a/src/java/org/apache/cassandra/db/marshal/ReversedType.java
b/src/java/org/apache/cassandra/db/marshal/ReversedType.java
index 8a4b58d..ceea84a 100644
--- a/src/java/org/apache/cassandra/db/marshal/ReversedType.java
+++ b/src/java/org/apache/cassandra/db/marshal/ReversedType.java
@@ -106,12 +106,6 @@ public class ReversedType<T> extends AbstractType<T>
}
@Override
- public boolean isValueCompatibleWith(AbstractType<?> otherType)
- {
- return this.baseType.isValueCompatibleWith(otherType);
- }
-
- @Override
public CQL3Type asCQL3Type()
{
return baseType.asCQL3Type();
diff --git
a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
index 5297a41..c2a9d7a 100644
--- a/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
+++ b/test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
@@ -31,6 +31,7 @@ import org.apache.cassandra.db.ColumnFamilyStore;
import org.apache.cassandra.db.Keyspace;
import org.apache.cassandra.dht.OrderPreservingPartitioner;
import org.apache.cassandra.exceptions.ConfigurationException;
+import org.apache.cassandra.exceptions.InvalidRequestException;
import org.apache.cassandra.exceptions.SyntaxException;
import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.locator.TokenMetadata;
@@ -508,6 +509,37 @@ public class AlterTest extends CQLTester
alterTable("alter table %s add v1 int");
}
+ @Test(expected = InvalidRequestException.class)
+ public void testDropFixedAddVariable() throws Throwable
+ {
+ createTable("create table %s (k int, c int, v int, PRIMARY KEY (k,
c))");
+ execute("alter table %s drop v");
+ execute("alter table %s add v varint");
+ }
+
+ @Test(expected = InvalidRequestException.class)
+ public void testDropFixedCollectionAddVariableCollection() throws Throwable
+ {
+ createTable("create table %s (k int, c int, v list<int>, PRIMARY KEY
(k, c))");
+ execute("alter table %s drop v");
+ execute("alter table %s add v list<varint>");
+ }
+
+ @Test(expected = InvalidRequestException.class)
+ public void testDropSimpleAddComplex() throws Throwable
+ {
+ createTable("create table %s (k int, c int, v set<text>, PRIMARY KEY
(k, c))");
+ execute("alter table %s drop v");
+ execute("alter table %s add v blob");
+ }
+
+ @Test(expected = SyntaxException.class)
+ public void renameToEmptyTest() throws Throwable
+ {
+ createTable("CREATE TABLE %s (k int, c1 int, v int, PRIMARY KEY (k,
c1))");
+ execute("ALTER TABLE %s RENAME c1 TO \"\"");
+ }
+
@Test
// tests CASSANDRA-9565
public void testDoubleWith() throws Throwable
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
