jmckenzie-dev commented on code in PR #3562: URL: https://github.com/apache/cassandra/pull/3562#discussion_r1917283496
########## doc/modules/cassandra/pages/developing/cql/constraints.adoc: ########## @@ -0,0 +1,93 @@ += Constraints + +Constraints provide a way of specifying and enforcing conditions at a +column level in a table schema definition and enforcing them at write time. + +== CREATE CONSTRAINT + +Constraints can be created within the column definition, or as part +of the table properties. + +The main syntaxis to define a constraint is as follows: + +[source,bnf] +---- +CREATE TABLE keyspace.table ( + name text, + i int CHECK (condition) (AND (condition))* + ..., + +); +---- + +As shown in this syntax, more than one constraint can be defined for a given column using the AND keyword. + +== ALTER CONSTRAINT + +Altering a constraint is done by following the alter column CQL syntax: +[source,bnf] +---- +ALTER TABLE [IF EXISTS] <table> ALTER [IF EXISTS] <column> CHECK <condition>; +---- + +== DROP CONSTRAINT +And DROP can be used to drop constraints for a column as well. +[source,bnf] +---- +ALTER TABLE [IF EXISTS] <table> ALTER [IF EXISTS] <column> DROP CHECK; +---- + +== AVAILABLE CONSTRAINTS + +=== SCALAR CONSTRAINT + +Defines a comparator against a numeric type. It support all numeric types supported in Cassandra, with all the regular +comparators. + +For example, we can define constraints that ensure that i is bigger or equal than 100 but smaller than 1000. + +[source,bnf] +---- +CREATE TABLE keyspace.table ( + name text, + i int CHECK i < 1000 AND i > 100 + ..., +); +---- + +Altering that constraint can be done with: + +---- +ALTER TABLE keyspace.table ALTER i CHECK i >= 500; +---- + +Finally, the constraint can be removed: + +---- +ALTER TABLE keyspace.table ALTER i DROP CHECK; +---- + +=== LENGTH CONSTRAINT + +Defines a condition that checks the lenght of text or binary type. Review Comment: nit: length ########## src/java/org/apache/cassandra/cql3/constraints/FunctionColumnConstraint.java: ########## @@ -0,0 +1,165 @@ +/* + * 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.constraints; + +import java.io.IOException; + +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.cql3.Operator; +import org.apache.cassandra.db.TypeSizes; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +public class FunctionColumnConstraint implements ColumnConstraint<FunctionColumnConstraint> +{ + public static final Serializer serializer = new Serializer(); + + public final ConstraintFunction function; + public final ColumnIdentifier columnName; + public final Operator relationType; + public final String term; + + public final static class Raw + { + public final ConstraintFunction function; + public final ColumnIdentifier columnName; + public final Operator relationType; + public final String term; + + public Raw(ColumnIdentifier functionName, ColumnIdentifier columnName, Operator relationType, String term) + { + this.relationType = relationType; + this.columnName = columnName; + this.term = term; + function = createConstraintFunction(functionName.toCQLString(), columnName); + } + + public FunctionColumnConstraint prepare() + { + return new FunctionColumnConstraint(function, columnName, relationType, term); + } + } + + private static ConstraintFunction createConstraintFunction(String functionName, ColumnIdentifier columnName) + { + if (LengthConstraint.FUNCTION_NAME.equalsIgnoreCase(functionName)) + return new LengthConstraint(columnName); + throw new InvalidConstraintDefinitionException("Unrecognized constraint function: " + functionName); + } + + private FunctionColumnConstraint(ConstraintFunction function, ColumnIdentifier columnName, Operator relationType, String term) + { + this.function = function; + this.columnName = columnName; + this.relationType = relationType; + this.term = term; + } + + @Override + public void appendCqlTo(CqlBuilder builder) + { + builder.append(toString()); + } + + @Override + public IVersionedSerializer<FunctionColumnConstraint> serializer() + { + return serializer; + } + + @Override + public void evaluate(Class<? extends AbstractType> valueType, Object columnValue) + { + function.evaluate(valueType, relationType, term, columnValue); + } + + @Override + public void validate(ColumnMetadata columnMetadata) + { + validateArgs(columnMetadata); + function.validate(columnMetadata); + } + + @Override + public ConstraintType getConstraintType() + { + return ConstraintType.FUNCTION; + } + + void validateArgs(ColumnMetadata columnMetadata) + { + if (!columnMetadata.name.equals(columnName)) + throw new InvalidConstraintDefinitionException("Function parameter should be the column name"); + } + + @Override + public String toString() + { + return function.getName() + "(" + columnName + ") " + relationType + " " + term; + } + + public static class Serializer implements IVersionedSerializer<FunctionColumnConstraint> + { + @Override + public void serialize(FunctionColumnConstraint columnConstraint, DataOutputPlus out, int version) throws IOException + { + FunctionColumnConstraint condition = columnConstraint; Review Comment: Looks like `condition` is just a redundant alias of `columnConstraint`? Same with `serializedSize`? ########## src/java/org/apache/cassandra/cql3/constraints/LengthConstraint.java: ########## @@ -0,0 +1,118 @@ +/* + * 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.constraints; + +import com.google.common.collect.Sets; +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.Operator; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.AsciiType; +import org.apache.cassandra.db.marshal.BytesType; +import org.apache.cassandra.db.marshal.UTF8Type; +import org.apache.cassandra.schema.ColumnMetadata; + +import java.nio.ByteBuffer; Review Comment: nit: base java imports above o.a.c ########## src/java/org/apache/cassandra/schema/ColumnMetadata.java: ########## @@ -275,6 +318,11 @@ public boolean isMasked() return mask != null; } + public boolean hasConstraint() + { + return hasConstraints; Review Comment: Why cache this instead of always evaluating `!columnConstraints.isEmpty()`? ########## src/java/org/apache/cassandra/schema/ColumnMetadata.java: ########## @@ -627,11 +716,24 @@ public ColumnMetadata deserialize(DataInputPlus in, Types types, UserFunctions f boolean masked = in.readBoolean(); if (masked) mask = ColumnMask.serializer.deserialize(in, ksName, type, types, functions, version); - return new ColumnMetadata(ksName, tableName, new ColumnIdentifier(nameBB, name), type, position, kind, mask); + ColumnConstraints constraints; + if (version.isAtLeast(Version.V6) && in.readBoolean()) + constraints = ColumnConstraints.serializer.deserialize(in, version.asInt()); + else + constraints = ColumnConstraints.NO_OP; + return new ColumnMetadata(ksName, tableName, new ColumnIdentifier(nameBB, name), type, position, kind, mask, constraints); } public long serializedSize(ColumnMetadata t, Version version) { + long constraintsSize = 0; + if (version.isAtLeast(Version.V6)) Review Comment: How does our logic resolve / handle being in a mixed version cluster where some members are older versions and some newer? Does TCM keep the version at the lowest common denominator in that case? ########## doc/modules/cassandra/pages/developing/cql/constraints.adoc: ########## @@ -0,0 +1,93 @@ += Constraints + +Constraints provide a way of specifying and enforcing conditions at a +column level in a table schema definition and enforcing them at write time. + +== CREATE CONSTRAINT + +Constraints can be created within the column definition, or as part +of the table properties. + +The main syntaxis to define a constraint is as follows: Review Comment: nit: syntax ########## doc/modules/cassandra/pages/developing/cql/constraints.adoc: ########## @@ -0,0 +1,93 @@ += Constraints + +Constraints provide a way of specifying and enforcing conditions at a +column level in a table schema definition and enforcing them at write time. + +== CREATE CONSTRAINT + +Constraints can be created within the column definition, or as part +of the table properties. + +The main syntaxis to define a constraint is as follows: + +[source,bnf] +---- +CREATE TABLE keyspace.table ( + name text, + i int CHECK (condition) (AND (condition))* + ..., + +); +---- + +As shown in this syntax, more than one constraint can be defined for a given column using the AND keyword. + +== ALTER CONSTRAINT + +Altering a constraint is done by following the alter column CQL syntax: +[source,bnf] +---- +ALTER TABLE [IF EXISTS] <table> ALTER [IF EXISTS] <column> CHECK <condition>; +---- + +== DROP CONSTRAINT +And DROP can be used to drop constraints for a column as well. +[source,bnf] +---- +ALTER TABLE [IF EXISTS] <table> ALTER [IF EXISTS] <column> DROP CHECK; Review Comment: To confirm my understanding, if you have multiple checks on a column you have to drop all of them and recreate them, we don't allow aliasing specific check names and doing granular work modifying or dropping specifics? ########## doc/modules/cassandra/nav.adoc: ########## @@ -60,6 +60,7 @@ *** xref:cassandra:developing/cql/json.adoc[JSON] *** xref:cassandra:developing/cql/security.adoc[Security] *** xref:cassandra:developing/cql/triggers.adoc[Triggers] +*** xref:cassandra:developing/cql/constraints.adoc[Constraints] Review Comment: ... how are these organized in here? This isn't a CEP-42 problem btw, more of a general "this looks like a random pile of stuff" problem. :) ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraint.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.constraints; + +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.schema.ColumnMetadata; + +/** + * Common class for the conditions that a CQL Constraint needs to implement to be integrated in the + * CQL Constraints framework. + */ +public interface ColumnConstraint<T> +{ + + // Enum containing all the possible constraint serializers to help with serialization/deserialization + // of constraints. + public enum ConstraintType + { + // The order of that enum matters!! Review Comment: Add clarification on why (so future maintainers in both places can take this into account). ########## src/java/org/apache/cassandra/cql3/constraints/ScalarColumnConstraint.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.constraints; + + +import java.io.IOException; + +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.cql3.Operator; +import org.apache.cassandra.db.TypeSizes; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +public class ScalarColumnConstraint implements ColumnConstraint<ScalarColumnConstraint> +{ + public final ColumnIdentifier param; + public final Operator relationType; + public final String term; + + public final static Serializer serializer = new Serializer(); + + public final static class Raw + { + public final ColumnIdentifier param; + public final Operator relationType; + public final String term; + + public Raw(ColumnIdentifier param, Operator relationType, String term) + { + this.param = param; + this.relationType = relationType; + this.term = term; + } + + public ScalarColumnConstraint prepare() + { + return new ScalarColumnConstraint(param, relationType, term); + } + } + + private ScalarColumnConstraint(ColumnIdentifier param, Operator relationType, String term) + { + this.param = param; + this.relationType = relationType; + this.term = term; + } + + @Override + public void evaluate(Class<? extends AbstractType> valueType, Object columnValue) + { + Number columnValueNumber; + float sizeConstraint; + + try + { + columnValueNumber = (Number) columnValue; + sizeConstraint = Float.parseFloat(term); + } + catch (NumberFormatException exception) + { + throw new ConstraintViolationException(param + " and " + term + " need to be numbers."); + } + + switch (relationType) + { + case EQ: + if (Float.compare(columnValueNumber.floatValue(), sizeConstraint) != 0) + throw new ConstraintViolationException(param + " value should be exactly " + sizeConstraint); + break; + case NEQ: + if (Double.compare(columnValueNumber.floatValue(), sizeConstraint) == 0) Review Comment: Why are we using `Float.compare` on EQ case and `Double.compare` in NEQ? If intentional, should document why. If not, should fix / unify on `Float`. ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.constraints; + +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.CompositeType; +import org.apache.cassandra.db.marshal.DynamicCompositeType; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.TupleType; +import org.apache.cassandra.db.marshal.UserType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import com.google.common.collect.ImmutableSet; + +// group of constraints for the column +public class ColumnConstraints implements ColumnConstraint<ColumnConstraints> +{ + public static final Serializer serializer = new Serializer(); + + private static final Set<Class<? extends AbstractType>> UNSUPPORTED_TYPES = ImmutableSet.of(MapType.class, + TupleType.class, + UserType.class, + CompositeType.class, + DynamicCompositeType.class); + + + + private final List<ColumnConstraint<?>> constraints; + private final int constraintsSize; + private final boolean isEmpty; + + public ColumnConstraints(List<ColumnConstraint<?>> constraints) + { + this.constraints = new ArrayList<>(constraints); + // These are catched values to avoid making the calculations every time Review Comment: nit: cached ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.constraints; + +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.CompositeType; +import org.apache.cassandra.db.marshal.DynamicCompositeType; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.TupleType; +import org.apache.cassandra.db.marshal.UserType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +import java.io.IOException; Review Comment: nit: java base imports above o.a.c ########## doc/modules/cassandra/pages/developing/cql/constraints.adoc: ########## @@ -0,0 +1,93 @@ += Constraints + +Constraints provide a way of specifying and enforcing conditions at a +column level in a table schema definition and enforcing them at write time. + +== CREATE CONSTRAINT + +Constraints can be created within the column definition, or as part +of the table properties. + +The main syntaxis to define a constraint is as follows: + +[source,bnf] +---- +CREATE TABLE keyspace.table ( + name text, + i int CHECK (condition) (AND (condition))* Review Comment: The CEP indicated a couple different syntaxes to add constraints; did we keep those options or only allow for the one here? Link for reference: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=309496560#CEP42:ConstraintsFramework-Approach ########## src/java/org/apache/cassandra/cql3/Json.java: ########## @@ -69,6 +72,17 @@ public Prepared prepareAndCollectMarkers(TableMetadata metadata, Collection<Colu { return new PreparedLiteral(parseJson(text, receivers)); } + + @Override + public Prepared collectMarkers(TableMetadata metadata, Collection<ColumnMetadata> receivers, VariableSpecifications boundNames) + { + return new UnpreparedLiteral(parseUnpreparedJson(text, receivers)); + } + + public String getText() Review Comment: This looks to be unused. ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.constraints; + +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.CompositeType; +import org.apache.cassandra.db.marshal.DynamicCompositeType; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.TupleType; +import org.apache.cassandra.db.marshal.UserType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import com.google.common.collect.ImmutableSet; + +// group of constraints for the column +public class ColumnConstraints implements ColumnConstraint<ColumnConstraints> +{ + public static final Serializer serializer = new Serializer(); + + private static final Set<Class<? extends AbstractType>> UNSUPPORTED_TYPES = ImmutableSet.of(MapType.class, + TupleType.class, + UserType.class, + CompositeType.class, + DynamicCompositeType.class); + + + + private final List<ColumnConstraint<?>> constraints; + private final int constraintsSize; + private final boolean isEmpty; + + public ColumnConstraints(List<ColumnConstraint<?>> constraints) + { + this.constraints = new ArrayList<>(constraints); + // These are catched values to avoid making the calculations every time + this.constraintsSize = constraints.size(); + this.isEmpty = constraints.isEmpty(); Review Comment: Both of these should just be accessing an integer value inside an ArrayList. [Example from OpenJDK](https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/master/src/java.base/share/classes/java/util/ArrayList.java#L287-L294): ```java /** * Returns the number of elements in this list. * * @return the number of elements in this list */ public int size() { return size; } /** * Returns {@code true} if this list contains no elements. * * @return {@code true} if this list contains no elements */ public boolean isEmpty() { return size == 0; } ``` So I _think_ we can drop this caching. ########## src/java/org/apache/cassandra/cql3/Json.java: ########## @@ -323,4 +361,63 @@ static Map<ColumnIdentifier, Term> parseJson(String jsonString, Collection<Colum throw new InvalidRequestException(exc.getMessage()); } } + + static Term.Raw getUnpreparedTerm(ColumnSpecification spec, Object parsedJsonObject) + { + if (spec.type.isNumber()) + return Constants.Literal.floatingPoint(parsedJsonObject.toString()); Review Comment: Why are we treating all JSON #'s as floats here? ########## src/java/org/apache/cassandra/cql3/constraints/ConstraintFunction.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.constraints; + + +import org.apache.cassandra.cql3.Operator; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.schema.ColumnMetadata; + +/** + * Interface to be implemented by functions that are executed as part of CQL constraints. + */ +public interface ConstraintFunction +{ + /** + * @return the function name to be executed. + */ + String getName(); + + /** + * Method that provides the execution of the condition. It can either succeed or throw a {@link ConstraintViolationException}. + * + * @param valueType Review Comment: I'd just drop any `@param` that's self-explanatory and we don't add any description to. Alternatively feel free to add descriptions if helpful. Also, the top level javadoc on this is pretty redundant with the method signature; maybe elaborate on what happens in the innards of `evaluate` (i.e. for a given column value, applies the relation to it and throws an exception if input value is outside the bounds of the constraint or something like that). ########## src/java/org/apache/cassandra/schema/ColumnMetadata.java: ########## @@ -126,42 +135,60 @@ private static long comparisonOrder(Kind kind, boolean isComplex, long position, public static ColumnMetadata partitionKeyColumn(TableMetadata table, ByteBuffer name, AbstractType<?> type, int position) { - return new ColumnMetadata(table, name, type, position, Kind.PARTITION_KEY, null); + return new ColumnMetadata(table, name, type, position, Kind.PARTITION_KEY, null, ColumnConstraints.NO_OP); } public static ColumnMetadata partitionKeyColumn(String keyspace, String table, String name, AbstractType<?> type, int position) { - return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, position, Kind.PARTITION_KEY, null); + return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, position, Kind.PARTITION_KEY, null, ColumnConstraints.NO_OP); } public static ColumnMetadata clusteringColumn(TableMetadata table, ByteBuffer name, AbstractType<?> type, int position) { - return new ColumnMetadata(table, name, type, position, Kind.CLUSTERING, null); + return new ColumnMetadata(table, name, type, position, Kind.CLUSTERING, null, ColumnConstraints.NO_OP); } public static ColumnMetadata clusteringColumn(String keyspace, String table, String name, AbstractType<?> type, int position) { - return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, position, Kind.CLUSTERING, null); + return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, position, Kind.CLUSTERING, null, ColumnConstraints.NO_OP); } public static ColumnMetadata regularColumn(TableMetadata table, ByteBuffer name, AbstractType<?> type) { - return new ColumnMetadata(table, name, type, NO_POSITION, Kind.REGULAR, null); + return new ColumnMetadata(table, name, type, NO_POSITION, Kind.REGULAR, null, ColumnConstraints.NO_OP); } public static ColumnMetadata regularColumn(String keyspace, String table, String name, AbstractType<?> type) { - return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, NO_POSITION, Kind.REGULAR, null); + return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, NO_POSITION, Kind.REGULAR, null, ColumnConstraints.NO_OP); } public static ColumnMetadata staticColumn(TableMetadata table, ByteBuffer name, AbstractType<?> type) { - return new ColumnMetadata(table, name, type, NO_POSITION, Kind.STATIC, null); + return new ColumnMetadata(table, name, type, NO_POSITION, Kind.STATIC, null, ColumnConstraints.NO_OP); } public static ColumnMetadata staticColumn(String keyspace, String table, String name, AbstractType<?> type) { - return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, NO_POSITION, Kind.STATIC, null); + return new ColumnMetadata(keyspace, table, ColumnIdentifier.getInterned(name, true), type, NO_POSITION, Kind.STATIC, null, ColumnConstraints.NO_OP); + } + + public ColumnMetadata(TableMetadata table, Review Comment: Stylistic idea here. Right now you've added `ColumnConstraints.NO_OP` to all the method calls here which is a lot of noise with very little signal. What if we had: ```java static ColumnMetadata buildWithoutConstraints(...) {} static ColumnMetadata buildWithConstraints(..., @Nonnull ColumnConstraints columnConstraints){] ``` Where the 1st method we'd call w/all the callers in this class so we don't have to pass that redundant `NO_OP` in all of them, and we rely on method names to help disambiguate what kind of construction we're doing? Don't feel super strongly on this but got similar feedback from David a couple weeks ago on this w/callbacks and I liked the aesthetics of how the end result turned out. ########## src/java/org/apache/cassandra/cql3/statements/UpdateStatement.java: ########## @@ -261,6 +269,15 @@ protected ModificationStatement prepareInternal(ClientState state, false, false); + Json.Prepared unprepared = jsonValue.collectMarkers(metadata, defs, bindVariables); + List<ColumnIdentifier> columnNames = new ArrayList<>(); Review Comment: I don't see `columnNames` and `columnValues` being used; am I missing something? ########## test/unit/org/apache/cassandra/cql3/validation/operations/InsertTest.java: ########## @@ -132,12 +132,13 @@ public void testInsert() throws Throwable private void testInsert(boolean forceFlush) throws Throwable { createTable("CREATE TABLE %s (partitionKey int," + + "pk2 int," + Review Comment: No strong opinion here, but what was the reasoning behind modifying the `testInsert` here to use a composite primary key? Specifically as part of this patch for constraints here. ########## src/java/org/apache/cassandra/cql3/Json.java: ########## @@ -125,6 +145,24 @@ public Term.Raw getRawTermForColumn(ColumnMetadata def, boolean defaultUnset) } } + private static class UnpreparedLiteral extends Prepared Review Comment: `UnpreparedLiteral extends Prepared` had me scratching my head for a second. ;) Should we consider renaming `Prepared` and updating the javadoc for it if it no longer represents prepared data? ########## src/java/org/apache/cassandra/cql3/constraints/ScalarColumnConstraint.java: ########## @@ -0,0 +1,172 @@ +/* + * 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.constraints; + + +import java.io.IOException; + +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.cql3.Operator; +import org.apache.cassandra.db.TypeSizes; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +public class ScalarColumnConstraint implements ColumnConstraint<ScalarColumnConstraint> +{ + public final ColumnIdentifier param; + public final Operator relationType; + public final String term; + + public final static Serializer serializer = new Serializer(); + + public final static class Raw + { + public final ColumnIdentifier param; + public final Operator relationType; + public final String term; + + public Raw(ColumnIdentifier param, Operator relationType, String term) + { + this.param = param; + this.relationType = relationType; + this.term = term; + } + + public ScalarColumnConstraint prepare() + { + return new ScalarColumnConstraint(param, relationType, term); + } + } + + private ScalarColumnConstraint(ColumnIdentifier param, Operator relationType, String term) + { + this.param = param; + this.relationType = relationType; + this.term = term; + } + + @Override + public void evaluate(Class<? extends AbstractType> valueType, Object columnValue) + { + Number columnValueNumber; + float sizeConstraint; + + try + { + columnValueNumber = (Number) columnValue; + sizeConstraint = Float.parseFloat(term); + } + catch (NumberFormatException exception) + { + throw new ConstraintViolationException(param + " and " + term + " need to be numbers."); + } + + switch (relationType) + { + case EQ: + if (Float.compare(columnValueNumber.floatValue(), sizeConstraint) != 0) + throw new ConstraintViolationException(param + " value should be exactly " + sizeConstraint); + break; + case NEQ: + if (Double.compare(columnValueNumber.floatValue(), sizeConstraint) == 0) + throw new ConstraintViolationException(param + " value should be different from " + sizeConstraint); + break; + case GT: + if (columnValueNumber.floatValue() <= sizeConstraint) + throw new ConstraintViolationException(param + " value should be larger than " + sizeConstraint); + break; + case LT: + if (columnValueNumber.floatValue() >= sizeConstraint) + throw new ConstraintViolationException(param + " value should be smaller than " + sizeConstraint); + break; + case GTE: + if (columnValueNumber.floatValue() < sizeConstraint) + throw new ConstraintViolationException(param + " value should be larger or equal than " + sizeConstraint); + break; + case LTE: + if (columnValueNumber.floatValue() > sizeConstraint) + throw new ConstraintViolationException(param + " value should be smaller or equal than " + sizeConstraint); + break; + default: + throw new ConstraintViolationException("Invalid relation type: " + relationType); + } + } + + @Override + public void validate(ColumnMetadata columnMetadata) throws InvalidConstraintDefinitionException + { + if (!columnMetadata.type.isNumber()) + throw new InvalidConstraintDefinitionException(param + " is not a number"); + } + + @Override + public ConstraintType getConstraintType() + { + return ConstraintType.SCALAR; + } + + @Override + public String toString() + { + return param + " " + relationType + ' ' + term; Review Comment: nit: consistency with " " vs. ' ' ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.constraints; + +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.CompositeType; +import org.apache.cassandra.db.marshal.DynamicCompositeType; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.TupleType; +import org.apache.cassandra.db.marshal.UserType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import com.google.common.collect.ImmutableSet; + +// group of constraints for the column +public class ColumnConstraints implements ColumnConstraint<ColumnConstraints> +{ + public static final Serializer serializer = new Serializer(); + + private static final Set<Class<? extends AbstractType>> UNSUPPORTED_TYPES = ImmutableSet.of(MapType.class, + TupleType.class, + UserType.class, + CompositeType.class, + DynamicCompositeType.class); + + + + private final List<ColumnConstraint<?>> constraints; + private final int constraintsSize; + private final boolean isEmpty; + + public ColumnConstraints(List<ColumnConstraint<?>> constraints) + { + this.constraints = new ArrayList<>(constraints); + // These are catched values to avoid making the calculations every time + this.constraintsSize = constraints.size(); + this.isEmpty = constraints.isEmpty(); + } + + @Override + public IVersionedSerializer<ColumnConstraints> serializer() + { + return serializer; + } + + @Override + public void appendCqlTo(CqlBuilder builder) + { + for (ColumnConstraint<?> constraint : constraints) + constraint.appendCqlTo(builder); + } + + @Override + public void evaluate(Class<? extends AbstractType> valueType, Object columnValue) throws ConstraintViolationException + { + for (ColumnConstraint<?> constraint : constraints) + constraint.evaluate(valueType, columnValue); + } + + public List<ColumnConstraint<?>> getConstraints() + { + return constraints; + } + + public boolean isEmpty() + { + return isEmpty; + } + + public int getSize() + { + return constraintsSize; + } + + @Override + public void validate(ColumnMetadata columnMetadata) throws InvalidConstraintDefinitionException + { + if (UNSUPPORTED_TYPES.contains(columnMetadata.type.getClass())) + throw new InvalidConstraintDefinitionException("Constraint cannot be defined on column '" + + columnMetadata.name + "' with type: " + + columnMetadata.type.asCQL3Type()); + + for (ColumnConstraint<?> constraint : constraints) + constraint.validate(columnMetadata); + } + + @Override + public ConstraintType getConstraintType() + { + return ConstraintType.COMPOSED; + } + + public static class Noop extends ColumnConstraints + { + public static final Noop INSTANCE = new Noop(); + + public Noop() + { + super(Collections.emptyList()); + } + + @Override + public boolean isEmpty() + { + return true; + } + + @Override + public int getSize() + { + return 0; + } + } + + public final static class Raw + { + private final List<ColumnConstraint<?>> constraints; + + public Raw(List<ColumnConstraint<?>> constraints) + { + this.constraints = constraints; + } + + public Raw() + { + this.constraints = Collections.emptyList(); + } + + public ColumnConstraints prepare() + { + return new ColumnConstraints(constraints); + } + } + + public static class Serializer implements IVersionedSerializer<ColumnConstraints> + { + @Override + public void serialize(ColumnConstraints columnConstraint, DataOutputPlus out, int version) throws IOException + { + out.writeInt(columnConstraint.getSize()); + for (ColumnConstraint constraint : columnConstraint.getConstraints()) + { + // We serialize the serializer position in the enum to save space + out.writeInt(constraint.getConstraintType().ordinal()); + constraint.serializer().serialize(constraint, out, version); + } + } + + @Override + public ColumnConstraints deserialize(DataInputPlus in, int version) throws IOException Review Comment: On `serialize` we have: 1. int: columnConstraint.getSize() 2. per constraint: 2a. int: ordinal of constraint type 2b. Type T: in this case ColumnConstraint serializer On `deserialize` we have: 1. int: num constraints 2. for each constraint: 2a. _**short**_ serializer position 2b. Type T: in this case ColumnConstraint So the int ser in 2a. doesn't look to match the short deser in 2a. That a bug, or is there nuance to the "serialize serializer position" bit I'm missing? fwiw, the entire ser/deser here has me squinting and thinking hard in general. ########## src/resources/org/apache/cassandra/cql3/reserved_keywords.txt: ########## @@ -54,4 +54,5 @@ USE USING VIEW WHERE -WITH \ No newline at end of file +WITH +CONSTRAINT Review Comment: Do we want just `CONSTRAINT` or also include `CHECK`? ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.constraints; + +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.CompositeType; +import org.apache.cassandra.db.marshal.DynamicCompositeType; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.TupleType; +import org.apache.cassandra.db.marshal.UserType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import com.google.common.collect.ImmutableSet; + +// group of constraints for the column +public class ColumnConstraints implements ColumnConstraint<ColumnConstraints> +{ + public static final Serializer serializer = new Serializer(); + + private static final Set<Class<? extends AbstractType>> UNSUPPORTED_TYPES = ImmutableSet.of(MapType.class, + TupleType.class, + UserType.class, + CompositeType.class, + DynamicCompositeType.class); + + + + private final List<ColumnConstraint<?>> constraints; + private final int constraintsSize; + private final boolean isEmpty; + + public ColumnConstraints(List<ColumnConstraint<?>> constraints) + { + this.constraints = new ArrayList<>(constraints); + // These are catched values to avoid making the calculations every time + this.constraintsSize = constraints.size(); + this.isEmpty = constraints.isEmpty(); + } + + @Override + public IVersionedSerializer<ColumnConstraints> serializer() + { + return serializer; + } + + @Override + public void appendCqlTo(CqlBuilder builder) + { + for (ColumnConstraint<?> constraint : constraints) + constraint.appendCqlTo(builder); + } + + @Override + public void evaluate(Class<? extends AbstractType> valueType, Object columnValue) throws ConstraintViolationException + { + for (ColumnConstraint<?> constraint : constraints) + constraint.evaluate(valueType, columnValue); + } + + public List<ColumnConstraint<?>> getConstraints() + { + return constraints; + } + + public boolean isEmpty() + { + return isEmpty; + } + + public int getSize() + { + return constraintsSize; + } + + @Override + public void validate(ColumnMetadata columnMetadata) throws InvalidConstraintDefinitionException + { + if (UNSUPPORTED_TYPES.contains(columnMetadata.type.getClass())) + throw new InvalidConstraintDefinitionException("Constraint cannot be defined on column '" + + columnMetadata.name + "' with type: " + + columnMetadata.type.asCQL3Type()); + + for (ColumnConstraint<?> constraint : constraints) + constraint.validate(columnMetadata); + } + + @Override + public ConstraintType getConstraintType() + { + return ConstraintType.COMPOSED; + } + + public static class Noop extends ColumnConstraints + { + public static final Noop INSTANCE = new Noop(); + + public Noop() + { + super(Collections.emptyList()); + } + + @Override + public boolean isEmpty() + { + return true; + } + + @Override + public int getSize() + { + return 0; + } + } + + public final static class Raw + { + private final List<ColumnConstraint<?>> constraints; + + public Raw(List<ColumnConstraint<?>> constraints) + { + this.constraints = constraints; + } + + public Raw() + { + this.constraints = Collections.emptyList(); + } + + public ColumnConstraints prepare() + { + return new ColumnConstraints(constraints); + } + } + + public static class Serializer implements IVersionedSerializer<ColumnConstraints> + { + @Override + public void serialize(ColumnConstraints columnConstraint, DataOutputPlus out, int version) throws IOException + { + out.writeInt(columnConstraint.getSize()); + for (ColumnConstraint constraint : columnConstraint.getConstraints()) + { + // We serialize the serializer position in the enum to save space + out.writeInt(constraint.getConstraintType().ordinal()); + constraint.serializer().serialize(constraint, out, version); + } + } + + @Override + public ColumnConstraints deserialize(DataInputPlus in, int version) throws IOException + { + List<ColumnConstraint<?>> columnConstraints = new ArrayList<>(); + int numberOfConstraints = in.readInt(); + for (int i = 0; i < numberOfConstraints; i++) + { + int serializerPosition = in.readShort(); + ColumnConstraint<?> constraint = (ColumnConstraint<?>) ConstraintType + .getSerializer(serializerPosition) + .deserialize(in, version); + columnConstraints.add(constraint); + } + return new ColumnConstraints(columnConstraints); + } + + @Override + public long serializedSize(ColumnConstraints columnConstraint, int version) + { + long constraintsSize = 0; + for (ColumnConstraint constraint : columnConstraint.getConstraints()) Review Comment: did we forget to add the `out.writeInt(columnConstraint.getSize());` from `serialize` here? ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java: ########## @@ -0,0 +1,200 @@ +/* + * 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.constraints; + +import org.apache.cassandra.cql3.CqlBuilder; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.marshal.CompositeType; +import org.apache.cassandra.db.marshal.DynamicCompositeType; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.TupleType; +import org.apache.cassandra.db.marshal.UserType; +import org.apache.cassandra.io.IVersionedSerializer; +import org.apache.cassandra.io.util.DataInputPlus; +import org.apache.cassandra.io.util.DataOutputPlus; +import org.apache.cassandra.schema.ColumnMetadata; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import com.google.common.collect.ImmutableSet; + +// group of constraints for the column +public class ColumnConstraints implements ColumnConstraint<ColumnConstraints> +{ + public static final Serializer serializer = new Serializer(); + + private static final Set<Class<? extends AbstractType>> UNSUPPORTED_TYPES = ImmutableSet.of(MapType.class, + TupleType.class, + UserType.class, + CompositeType.class, + DynamicCompositeType.class); + + + + private final List<ColumnConstraint<?>> constraints; + private final int constraintsSize; + private final boolean isEmpty; + + public ColumnConstraints(List<ColumnConstraint<?>> constraints) + { + this.constraints = new ArrayList<>(constraints); + // These are catched values to avoid making the calculations every time + this.constraintsSize = constraints.size(); + this.isEmpty = constraints.isEmpty(); + } + + @Override + public IVersionedSerializer<ColumnConstraints> serializer() + { + return serializer; + } + + @Override + public void appendCqlTo(CqlBuilder builder) + { + for (ColumnConstraint<?> constraint : constraints) + constraint.appendCqlTo(builder); + } + + @Override + public void evaluate(Class<? extends AbstractType> valueType, Object columnValue) throws ConstraintViolationException + { + for (ColumnConstraint<?> constraint : constraints) + constraint.evaluate(valueType, columnValue); + } + + public List<ColumnConstraint<?>> getConstraints() + { + return constraints; + } + + public boolean isEmpty() + { + return isEmpty; + } + + public int getSize() + { + return constraintsSize; + } + + @Override + public void validate(ColumnMetadata columnMetadata) throws InvalidConstraintDefinitionException + { + if (UNSUPPORTED_TYPES.contains(columnMetadata.type.getClass())) + throw new InvalidConstraintDefinitionException("Constraint cannot be defined on column '" + + columnMetadata.name + "' with type: " + + columnMetadata.type.asCQL3Type()); + + for (ColumnConstraint<?> constraint : constraints) + constraint.validate(columnMetadata); + } + + @Override + public ConstraintType getConstraintType() + { + return ConstraintType.COMPOSED; + } + + public static class Noop extends ColumnConstraints + { + public static final Noop INSTANCE = new Noop(); + + public Noop() + { + super(Collections.emptyList()); + } + + @Override + public boolean isEmpty() + { + return true; + } + + @Override + public int getSize() + { + return 0; + } + } + + public final static class Raw + { + private final List<ColumnConstraint<?>> constraints; + + public Raw(List<ColumnConstraint<?>> constraints) + { + this.constraints = constraints; + } + + public Raw() + { + this.constraints = Collections.emptyList(); + } + + public ColumnConstraints prepare() + { + return new ColumnConstraints(constraints); + } + } + + public static class Serializer implements IVersionedSerializer<ColumnConstraints> + { + @Override + public void serialize(ColumnConstraints columnConstraint, DataOutputPlus out, int version) throws IOException + { + out.writeInt(columnConstraint.getSize()); + for (ColumnConstraint constraint : columnConstraint.getConstraints()) + { + // We serialize the serializer position in the enum to save space Review Comment: This comment confused me a bit. Do we mean to say we serialize the ordinal of the enum rather than text value to save space? Because I see: 1. Serialized ordinal 2. Serialized version No serialization of serializer's position. Could be misreading though. ########## src/java/org/apache/cassandra/schema/TableMetadata.java: ########## @@ -196,6 +200,9 @@ public enum Kind public final DataResource resource; public TableMetadataRef ref; + public final List<ColumnConstraint> partitionKeyConstraints; + public final List<ColumnMetadata> columnsWithConstraints; Review Comment: Recommend you hoist up the comment below around "We cache the columns with constraints to avoid iterations over columns" to explain the purpose of this here and why the asymmetry in naming. ########## src/java/org/apache/cassandra/schema/ColumnMetadata.java: ########## @@ -320,7 +387,8 @@ private boolean equalsWithoutType(ColumnMetadata other) && position == other.position && ksName.equals(other.ksName) && cfName.equals(other.cfName) - && Objects.equals(mask, other.mask); + && Objects.equals(mask, other.mask) + && Objects.equals(columnConstraints, other.columnConstraints); Review Comment: Just to confirm, do we want to explicitly only check memory address equality between columnConstraints instances rather than contents therein? Since we're logically allowing non-memory identical comparisons with other fields including relying on the `.equals` implementation within `ColumnMask`. Just curious what the thinking was here. ########## src/java/org/apache/cassandra/schema/ColumnMetadata.java: ########## @@ -115,6 +118,12 @@ public boolean isPrimaryKeyKind() @Nullable private final ColumnMask mask; + @Nonnull + private ColumnConstraints columnConstraints; + + @Nonnull Review Comment: Don't need @Nonnull on a primitive do you? It's nonnull by definition I thought. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
