maedhroz commented on code in PR #3095: URL: https://github.com/apache/cassandra/pull/3095#discussion_r1588584843
########## src/java/org/apache/cassandra/cql3/restrictions/ClusteringElements.java: ########## @@ -0,0 +1,473 @@ +/* + * 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.restrictions; + +import java.nio.ByteBuffer; +import java.util.List; +import java.util.Objects; + +import com.google.common.collect.BoundType; +import com.google.common.collect.ForwardingList; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Range; +import com.google.common.collect.RangeSet; +import com.google.common.collect.TreeRangeSet; + +import org.apache.cassandra.cql3.ColumnSpecification; +import org.apache.cassandra.db.BufferClusteringBound; +import org.apache.cassandra.db.ClusteringBound; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.schema.ColumnMetadata; + +/** + * One or more contiguous clusterings elements. A clustering element is composed of a clustering column and its + * associated value. In practice, that class is not only used for clustering elements but also for partition key elements + * and tokens expression. + * + * <p>There are some differences between how predicates are represented in a CQL query and how they have to be expressed + * internally. Those differences are: + * <ul> + * <li>The selected partition keys and clustering columns can be expressed with separate predicates in a CQL query + * but need to be grouped internally to define exact partition and clustering keys. For example, + * <pre>[..] WHERE pk1 = 2 AND pk2 = 3 AND c1 = 5 AND c2 IN (3, 4)</pre> will request 2 rows with clustering (5, 3) and + * (5, 4) from the partition (2, 3).</li> + * <li>When clustering slices are expressed in a CQL query they are expressed as if all columns were in + * ascending order. Internally, the engine handles slice according to the clustering columns real order. + * For example, if column c1 is descending and column c2 is ascending the predicate <pre>(c1, c2) >= (1, 2)</pre> + * should be translated internally in [(bottom) .. (1, bottom)),[(1, 2, bottom)..(1, top)].</li> + * </ul> + * This class is used to bridge the gap between the CQL expression and its internal representation. It allows for elements + * to be appended together through the {@code extend} method and translate CQL ranges into their internal representation. + * </p> + * <p>It is important to realize that Guava {@code Range} instances are used in a slightly different way than how they were + * designed to be used to deal with the tree model of Clusterings. {@code Range} returned by this class are never + * unbounded so the methods {@code hasLowerEndpoint} and {@code hasUpperEndpoint} should not be used on those. + * When a range does not have a lower endpoint its lower endpoint value will be (bottom) (e.g. ClusteringElements.of().bottom()) + * and if it does not have an upper endpoint its upper endpoint value will be (top) (e.g. ClusteringElements.of().top()). + * Therefore a missing endpoint will always return {@code true} to {@code isEmpty()}.</p> + */ +public class ClusteringElements extends ForwardingList<ByteBuffer> implements Comparable<ClusteringElements> +{ + /** + * The empty {@code ClusteringElements} instance used to avoid creating unecessary empty instances. + */ + private static final ClusteringElements EMPTY = new ClusteringElements(ImmutableList.of(), ImmutableList.of()); + + /** + * A range representing all {@code ClusteringElements}. + */ + private static final Range<ClusteringElements> ALL = Range.closed(EMPTY.bottom(), EMPTY.top()); + + /** + * The elements columns. + */ + // We use ColumnSpecification to allow support for things like Token expressions that do not match real columns directly. + private final ImmutableList<? extends ColumnSpecification> columns; + + /** + * The elements values. + */ + private final ImmutableList<ByteBuffer> values; + + private ClusteringElements(ImmutableList<? extends ColumnSpecification> columns, ImmutableList<ByteBuffer> values) + { + if (columns.size() != values.size()) + throw new IllegalArgumentException("columns and values should have the same size"); Review Comment: nit: This class looks pretty well-tested. The only thing coverage reports point out is the paths that throw exceptions not being tested. -- 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]
