Repository: cassandra Updated Branches: refs/heads/trunk e77b70eec -> 51016876a
Accept subtypes for function results, type casts Patch by Tyler Hobbs; reviewed by Sylvain Lebresne for CASSANDRA-6766 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/9da742d5 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/9da742d5 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/9da742d5 Branch: refs/heads/trunk Commit: 9da742d5f83e1b2563be1c0d45a0e3d65a38ec44 Parents: f6c5e02 Author: Tyler Hobbs <[email protected]> Authored: Fri Jun 20 11:48:04 2014 -0500 Committer: Tyler Hobbs <[email protected]> Committed: Fri Jun 20 11:48:04 2014 -0500 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/cql3/TypeCast.java | 2 +- .../cassandra/cql3/functions/FunctionCall.java | 2 +- .../cassandra/cql3/functions/Functions.java | 4 +- .../cassandra/cql3/statements/Selection.java | 6 +- .../cassandra/db/marshal/AbstractType.java | 23 ++- .../apache/cassandra/db/marshal/BytesType.java | 2 +- .../cassandra/db/marshal/CompositeType.java | 8 +- .../apache/cassandra/db/marshal/DateType.java | 6 + .../cassandra/db/marshal/IntegerType.java | 6 + .../apache/cassandra/db/marshal/LongType.java | 6 + .../cassandra/db/marshal/ReversedType.java | 6 + .../cassandra/db/marshal/TimestampType.java | 6 + .../apache/cassandra/db/marshal/TupleType.java | 6 +- .../apache/cassandra/db/marshal/UUIDType.java | 6 + .../org/apache/cassandra/cql3/TypeTest.java | 144 +++++++++++++++++++ 16 files changed, 214 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 69cc46b..dac254e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -19,6 +19,7 @@ * Reference sstables before populating key cache (CASSANDRA-7234) * Account for range tombstones in min/max column names (CASSANDRA-7235) * Improve sub range repair validation (CASSANDRA-7317) + * Accept subtypes for function results, type casts (CASSANDRA-6766) Merged from 1.2: * Handle possible integer overflow in FastByteArrayOutputStream (CASSANDRA-7373) * cqlsh: 'ascii' values weren't formatted as text (CASSANDRA-7407) http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/cql3/TypeCast.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/TypeCast.java b/src/java/org/apache/cassandra/cql3/TypeCast.java index 66b5300..64261fa 100644 --- a/src/java/org/apache/cassandra/cql3/TypeCast.java +++ b/src/java/org/apache/cassandra/cql3/TypeCast.java @@ -48,7 +48,7 @@ public class TypeCast implements Term.Raw public boolean isAssignableTo(ColumnSpecification receiver) { - return receiver.type.asCQL3Type().equals(type); + return receiver.type.isValueCompatibleWith(type.getType()); } @Override http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java b/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java index 8db03e6..3abf65e 100644 --- a/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java +++ b/src/java/org/apache/cassandra/cql3/functions/FunctionCall.java @@ -142,7 +142,7 @@ public class FunctionCall extends Term.NonTerminal // is used as argument of another, existing, function. In that case, we return true here because we'll catch // the fact that the method is undefined latter anyway and with a more helpful error message that if we were // to return false here. - return returnType == null || receiver.type.asCQL3Type().equals(returnType.asCQL3Type()); + return returnType == null || receiver.type.isValueCompatibleWith(returnType); } @Override http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/cql3/functions/Functions.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/functions/Functions.java b/src/java/org/apache/cassandra/cql3/functions/Functions.java index 5f4201d..4f108cb 100644 --- a/src/java/org/apache/cassandra/cql3/functions/Functions.java +++ b/src/java/org/apache/cassandra/cql3/functions/Functions.java @@ -111,7 +111,7 @@ public abstract class Functions private static void validateTypes(Function fun, List<? extends AssignementTestable> providedArgs, ColumnSpecification receiver) throws InvalidRequestException { - if (!receiver.type.asCQL3Type().equals(fun.returnType().asCQL3Type())) + if (!receiver.type.isValueCompatibleWith(fun.returnType())) throw new InvalidRequestException(String.format("Type error: cannot assign result of function %s (type %s) to %s (type %s)", fun.name(), fun.returnType().asCQL3Type(), receiver, receiver.type.asCQL3Type())); if (providedArgs.size() != fun.argsType().size()) @@ -134,7 +134,7 @@ public abstract class Functions private static boolean isValidType(Function fun, List<? extends AssignementTestable> providedArgs, ColumnSpecification receiver) { - if (!receiver.type.asCQL3Type().equals(fun.returnType().asCQL3Type())) + if (!receiver.type.isValueCompatibleWith(fun.returnType())) return false; if (providedArgs.size() != fun.argsType().size()) http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/cql3/statements/Selection.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/statements/Selection.java b/src/java/org/apache/cassandra/cql3/statements/Selection.java index 123ddc3..37ab384 100644 --- a/src/java/org/apache/cassandra/cql3/statements/Selection.java +++ b/src/java/org/apache/cassandra/cql3/statements/Selection.java @@ -369,7 +369,7 @@ public abstract class Selection public boolean isAssignableTo(ColumnSpecification receiver) { - return type.asCQL3Type().equals(receiver.type.asCQL3Type()); + return receiver.type.isValueCompatibleWith(type); } @Override @@ -401,7 +401,7 @@ public abstract class Selection public boolean isAssignableTo(ColumnSpecification receiver) { - return fun.returnType().asCQL3Type().equals(receiver.type.asCQL3Type()); + return receiver.type.isValueCompatibleWith(fun.returnType()); } @Override @@ -446,7 +446,7 @@ public abstract class Selection public boolean isAssignableTo(ColumnSpecification receiver) { - return receiver.type.asCQL3Type().equals(isWritetime ? CQL3Type.Native.BIGINT : CQL3Type.Native.INT); + return receiver.type.isValueCompatibleWith(isWritetime ? LongType.instance : Int32Type.instance); } @Override http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/AbstractType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/AbstractType.java b/src/java/org/apache/cassandra/db/marshal/AbstractType.java index a38733c..e92f272 100644 --- a/src/java/org/apache/cassandra/db/marshal/AbstractType.java +++ b/src/java/org/apache/cassandra/db/marshal/AbstractType.java @@ -242,15 +242,28 @@ public abstract class AbstractType<T> implements Comparator<ByteBuffer> } /** - * Returns true if values of the previous AbstracType can be read by the this - * AbsractType. Note that this is a weaker version of isCompatibleWith, as it - * does not require that both type compare values the same way. + * Returns true if values of the other AbstractType can be read and "reasonably" interpreted by the this + * AbstractType. Note that this is a weaker version of isCompatibleWith, as it does not require that both type + * compare values the same way. + * + * The restriction on the other type being "reasonably" interpreted is to prevent, for example, IntegerType from + * being compatible with all other types. Even though any byte string is a valid IntegerType value, it doesn't + * necessarily make sense to interpret a UUID or a UTF8 string as an integer. * * Note that a type should be compatible with at least itself. */ - public boolean isValueCompatibleWith(AbstractType<?> previous) + public boolean isValueCompatibleWith(AbstractType<?> otherType) + { + return isValueCompatibleWithInternal((otherType instanceof ReversedType) ? ((ReversedType) otherType).baseType : otherType); + } + + /** + * Needed to handle ReversedType in value-compatibility checks. Subclasses should implement this instead of + * isValueCompatibleWith(). + */ + protected boolean isValueCompatibleWithInternal(AbstractType<?> otherType) { - return isCompatibleWith(previous); + return isCompatibleWith(otherType); } /** http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/BytesType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/BytesType.java b/src/java/org/apache/cassandra/db/marshal/BytesType.java index 9e122cc..7907c2d 100644 --- a/src/java/org/apache/cassandra/db/marshal/BytesType.java +++ b/src/java/org/apache/cassandra/db/marshal/BytesType.java @@ -66,7 +66,7 @@ public class BytesType extends AbstractType<ByteBuffer> } @Override - public boolean isValueCompatibleWith(AbstractType<?> previous) + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) { // BytesType can read anything return true; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/CompositeType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/CompositeType.java b/src/java/org/apache/cassandra/db/marshal/CompositeType.java index 2f537fb..946ba24 100644 --- a/src/java/org/apache/cassandra/db/marshal/CompositeType.java +++ b/src/java/org/apache/cassandra/db/marshal/CompositeType.java @@ -237,16 +237,16 @@ public class CompositeType extends AbstractCompositeType } @Override - public boolean isValueCompatibleWith(AbstractType<?> previous) + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) { - if (this == previous) + if (this == otherType) return true; - if (!(previous instanceof CompositeType)) + if (!(otherType instanceof CompositeType)) return false; // Extending with new components is fine - CompositeType cp = (CompositeType)previous; + CompositeType cp = (CompositeType) otherType; if (types.size() < cp.types.size()) return false; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/DateType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/DateType.java b/src/java/org/apache/cassandra/db/marshal/DateType.java index 0c97688..bf25d88 100644 --- a/src/java/org/apache/cassandra/db/marshal/DateType.java +++ b/src/java/org/apache/cassandra/db/marshal/DateType.java @@ -82,6 +82,12 @@ public class DateType extends AbstractType<Date> } @Override + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) + { + return this == otherType || otherType == TimestampType.instance || otherType == LongType.instance; + } + + @Override public CQL3Type asCQL3Type() { return CQL3Type.Native.TIMESTAMP; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/IntegerType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/IntegerType.java b/src/java/org/apache/cassandra/db/marshal/IntegerType.java index 726769b..ec1c7ad 100644 --- a/src/java/org/apache/cassandra/db/marshal/IntegerType.java +++ b/src/java/org/apache/cassandra/db/marshal/IntegerType.java @@ -136,6 +136,12 @@ public final class IntegerType extends AbstractType<BigInteger> return decompose(integerType); } + @Override + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) + { + return this == otherType || Int32Type.instance.isValueCompatibleWith(otherType) || LongType.instance.isValueCompatibleWith(otherType); + } + public CQL3Type asCQL3Type() { return CQL3Type.Native.VARINT; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/LongType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/LongType.java b/src/java/org/apache/cassandra/db/marshal/LongType.java index 9ad078e..efe7223 100644 --- a/src/java/org/apache/cassandra/db/marshal/LongType.java +++ b/src/java/org/apache/cassandra/db/marshal/LongType.java @@ -74,6 +74,12 @@ public class LongType extends AbstractType<Long> return decompose(longType); } + @Override + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) + { + return this == otherType || otherType == DateType.instance || otherType == TimestampType.instance; + } + public CQL3Type asCQL3Type() { return CQL3Type.Native.BIGINT; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/ReversedType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/ReversedType.java b/src/java/org/apache/cassandra/db/marshal/ReversedType.java index ef8fb92..cd61bbe 100644 --- a/src/java/org/apache/cassandra/db/marshal/ReversedType.java +++ b/src/java/org/apache/cassandra/db/marshal/ReversedType.java @@ -84,6 +84,12 @@ 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(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/TimestampType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/TimestampType.java b/src/java/org/apache/cassandra/db/marshal/TimestampType.java index 69ead8e..d7ce47b 100644 --- a/src/java/org/apache/cassandra/db/marshal/TimestampType.java +++ b/src/java/org/apache/cassandra/db/marshal/TimestampType.java @@ -74,6 +74,12 @@ public class TimestampType extends AbstractType<Date> return false; } + @Override + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) + { + return this == otherType || otherType == DateType.instance || otherType == LongType.instance; + } + public CQL3Type asCQL3Type() { return CQL3Type.Native.TIMESTAMP; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/TupleType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/TupleType.java b/src/java/org/apache/cassandra/db/marshal/TupleType.java index 74211c8..12457e9 100644 --- a/src/java/org/apache/cassandra/db/marshal/TupleType.java +++ b/src/java/org/apache/cassandra/db/marshal/TupleType.java @@ -234,13 +234,13 @@ public class TupleType extends AbstractType<ByteBuffer> } @Override - public boolean isValueCompatibleWith(AbstractType<?> previous) + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) { - if (!(previous instanceof TupleType)) + if (!(otherType instanceof TupleType)) return false; // Extending with new components is fine, removing is not - TupleType tt = (TupleType)previous; + TupleType tt = (TupleType) otherType; if (size() < tt.size()) return false; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/src/java/org/apache/cassandra/db/marshal/UUIDType.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/marshal/UUIDType.java b/src/java/org/apache/cassandra/db/marshal/UUIDType.java index 969ff17..b2caa04 100644 --- a/src/java/org/apache/cassandra/db/marshal/UUIDType.java +++ b/src/java/org/apache/cassandra/db/marshal/UUIDType.java @@ -189,6 +189,12 @@ public class UUIDType extends AbstractType<UUID> } } + @Override + public boolean isValueCompatibleWithInternal(AbstractType<?> otherType) + { + return this == otherType || otherType == TimeUUIDType.instance; + } + public CQL3Type asCQL3Type() { return CQL3Type.Native.UUID; http://git-wip-us.apache.org/repos/asf/cassandra/blob/9da742d5/test/unit/org/apache/cassandra/cql3/TypeTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/TypeTest.java b/test/unit/org/apache/cassandra/cql3/TypeTest.java new file mode 100644 index 0000000..f911a44 --- /dev/null +++ b/test/unit/org/apache/cassandra/cql3/TypeTest.java @@ -0,0 +1,144 @@ +/* + * 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 org.apache.cassandra.SchemaLoader; +import org.apache.cassandra.db.ConsistencyLevel; +import org.apache.cassandra.exceptions.RequestExecutionException; +import org.apache.cassandra.exceptions.RequestValidationException; +import org.apache.cassandra.gms.Gossiper; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.QueryState; +import org.apache.cassandra.transport.messages.ResultMessage; +import org.apache.cassandra.utils.MD5Digest; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.apache.cassandra.cql3.QueryProcessor.process; +import static org.apache.cassandra.cql3.QueryProcessor.processInternal; +import static org.junit.Assert.assertEquals; + +public class TypeTest +{ + private static final Logger logger = LoggerFactory.getLogger(TypeTest.class); + static ClientState clientState; + static String keyspace = "cql3_type_test"; + + @BeforeClass + public static void setUpClass() throws Throwable + { + SchemaLoader.loadSchema(); + executeSchemaChange("CREATE KEYSPACE IF NOT EXISTS %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}"); + clientState = ClientState.forInternalCalls(); + } + + @AfterClass + public static void stopGossiper() + { + Gossiper.instance.stop(); + } + + private static void executeSchemaChange(String query) throws Throwable + { + try + { + process(String.format(query, keyspace), ConsistencyLevel.ONE); + } catch (RuntimeException exc) + { + throw exc.getCause(); + } + } + + private static UntypedResultSet execute(String query) throws Throwable + { + try + { + return processInternal(String.format(query, keyspace)); + } catch (RuntimeException exc) + { + if (exc.getCause() != null) + throw exc.getCause(); + throw exc; + } + } + + private MD5Digest prepare(String query) throws RequestValidationException + { + ResultMessage.Prepared prepared = QueryProcessor.prepare(String.format(query, keyspace), clientState, false); + return prepared.statementId; + } + + private UntypedResultSet executePrepared(MD5Digest statementId, QueryOptions options) throws RequestValidationException, RequestExecutionException + { + CQLStatement statement = QueryProcessor.instance.getPrepared(statementId); + ResultMessage message = statement.executeInternal(QueryState.forInternalCalls(), options); + + if (message instanceof ResultMessage.Rows) + return new UntypedResultSet(((ResultMessage.Rows)message).result); + else + return null; + } + + @Test + public void testNowToUUIDCompatibility() throws Throwable + { + executeSchemaChange("CREATE TABLE IF NOT EXISTS %s.uuid_now (a int, b uuid, PRIMARY KEY (a, b))"); + String insert = "INSERT INTO %s.uuid_now (a, b) VALUES (0, now())"; + String select = "SELECT * FROM %s.uuid_now WHERE a=0 AND b < now()"; + execute(insert); + UntypedResultSet results = execute(select); + assertEquals(1, results.size()); + + executePrepared(prepare(insert), QueryOptions.DEFAULT); + results = executePrepared(prepare(select), QueryOptions.DEFAULT); + assertEquals(2, results.size()); + } + + @Test + public void testDateCompatibility() throws Throwable + { + executeSchemaChange("CREATE TABLE IF NOT EXISTS %s.date_compatibility (a int, b timestamp, c bigint, d varint, PRIMARY KEY (a, b, c, d))"); + String insert = "INSERT INTO %s.date_compatibility (a, b, c, d) VALUES (0, unixTimestampOf(now()), dateOf(now()), dateOf(now()))"; + String select = "SELECT * FROM %s.date_compatibility WHERE a=0 AND b < unixTimestampOf(now())"; + execute(insert); + UntypedResultSet results = execute(select); + assertEquals(1, results.size()); + + executePrepared(prepare(insert), QueryOptions.DEFAULT); + results = executePrepared(prepare(select), QueryOptions.DEFAULT); + assertEquals(2, results.size()); + } + + @Test + public void testReversedTypeCompatibility() throws Throwable + { + executeSchemaChange("CREATE TABLE IF NOT EXISTS %s.uuid_now_reversed (a int, b timeuuid, PRIMARY KEY (a, b)) WITH CLUSTERING ORDER BY (b DESC)"); + String insert = "INSERT INTO %s.uuid_now_reversed (a, b) VALUES (0, now())"; + String select = "SELECT * FROM %s.uuid_now_reversed WHERE a=0 AND b < now()"; + execute(insert); + UntypedResultSet results = execute(select); + assertEquals(1, results.size()); + + executePrepared(prepare(insert), QueryOptions.DEFAULT); + results = executePrepared(prepare(select), QueryOptions.DEFAULT); + assertEquals(2, results.size()); + } +} \ No newline at end of file
