Maxwell-Guo commented on code in PR #3562: URL: https://github.com/apache/cassandra/pull/3562#discussion_r1917796044
########## 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 Review Comment: the keyword public can be removed ? ########## src/java/org/apache/cassandra/cql3/constraints/ColumnConstraints.java: ########## @@ -0,0 +1,179 @@ +/* + * 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; + + public ColumnConstraints(List<ColumnConstraint<?>> constraints) + { + this.constraints = new ArrayList<>(constraints); + } + + @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() Review Comment: I looked at the latest code. According to his latest vision, do we not need to worry about whether it is empty or not and the size? ``` public static class Noop extends ColumnConstraints { private Noop() { super(Collections.emptyList()); } @Override public boolean isEmpty() { return true; } @Override public int getSize() { return 0; } } ``` But if it is an empty list to construct, these two functions don't need to care about it. am i right ? ########## 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) + { Review Comment: as described above ########## src/java/org/apache/cassandra/schema/ColumnMetadata.java: ########## @@ -237,22 +280,27 @@ private static Comparator<CellPath> makeCellPathComparator(Kind kind, AbstractTy public ColumnMetadata copy() { - return new ColumnMetadata(ksName, cfName, name, type, position, kind, mask); + return new ColumnMetadata(ksName, cfName, name, type, position, kind, mask, columnConstraints); } public ColumnMetadata withNewName(ColumnIdentifier newName) { - return new ColumnMetadata(ksName, cfName, newName, type, position, kind, mask); + return new ColumnMetadata(ksName, cfName, newName, type, position, kind, mask, columnConstraints); } public ColumnMetadata withNewType(AbstractType<?> newType) { - return new ColumnMetadata(ksName, cfName, name, newType, position, kind, mask); + return new ColumnMetadata(ksName, cfName, name, newType, position, kind, mask, columnConstraints); } public ColumnMetadata withNewMask(@Nullable ColumnMask newMask) { - return new ColumnMetadata(ksName, cfName, name, type, position, kind, newMask); + return new ColumnMetadata(ksName, cfName, name, type, position, kind, newMask, columnConstraints); + } + + public ColumnMetadata withNewColumnConstraint(@Nullable ColumnConstraints newCqlConstraints) Review Comment: this method is not used ? so we can remove it ########## 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; +import java.util.Set; + +public class LengthConstraint implements ConstraintFunction +{ + public static final String FUNCTION_NAME = "LENGTH"; + private static final Set<Class<?>> SUPPORTED_TYPES = Sets.newHashSet(UTF8Type.class, + AsciiType.class, + BytesType.class); + + private final ColumnIdentifier columnName; + + public LengthConstraint(ColumnIdentifier columnName) + { + this.columnName = columnName; + } + + @Override + public String getName() + { + return FUNCTION_NAME; + } + + @Override + public void evaluate(Class<? extends AbstractType> valueType, Operator relationType, String term, Object columnValue) + { + int valueLength = getValueSize(columnValue, valueType); + int sizeConstraint = Integer.parseInt(term); + + switch (relationType) Review Comment: Can this code of Switch be changed to this: ``` int valueLength = getValueSize(columnValue, valueType); int sizeConstraint = Integer.parseInt(term); ByteBuffer buffera = ByteBufferUtil.bytes(valueLength); ByteBuffer bufferb = ByteBufferUtil.bytes(sizeConstraint); if (!relationType.isSatisfiedBy(Int32Type.instance, buffera, bufferb)) { throw new ConstraintViolationException("xxxx“); } ``` WDYT ? @smiklosovic @yifan-c -- 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]

