adelapena commented on code in PR #2556:
URL: https://github.com/apache/cassandra/pull/2556#discussion_r1290161123
##########
src/java/org/apache/cassandra/db/MultiCBuilder.java:
##########
@@ -19,104 +19,155 @@
import java.nio.ByteBuffer;
import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import java.util.NavigableSet;
+import org.apache.cassandra.cql3.statements.Bound;
+import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.schema.ColumnMetadata;
import org.apache.cassandra.utils.ByteBufferUtil;
+import org.apache.cassandra.utils.UniqueComparator;
import org.apache.cassandra.utils.btree.BTreeSet;
/**
* Builder that allow to build multiple Clustering/ClusteringBound at the same
time.
*/
-public abstract class MultiCBuilder
+public class MultiCBuilder
{
+ /**
+ * Represents a building block of a clustering.
+ * Either a point (single value) or a bound.
+ * Knowing the kind of elements greatly simplifies the process of building
clustering bounds.
+ */
+ public static class Element
+ {
+ public enum Kind
+ {
+ POINT, INCL_START, EXCL_START, INCL_END, EXCL_END
+ }
+
+ public static Element BOTTOM = new Element(Collections.emptyList(),
Kind.INCL_START);
+ public static Element TOP = new Element(Collections.emptyList(),
Kind.INCL_END);
+ public static Element ROOT = new Element(Collections.emptyList(),
Kind.POINT);
+
+ final List<ByteBuffer> value;
+ final Kind kind;
+
+
+ private Element(List<ByteBuffer> values, Kind kind)
+ {
+ this.value = values;
Review Comment:
Nit: the attribute and the parameter should probably have the same name
##########
src/java/org/apache/cassandra/db/MultiCBuilder.java:
##########
@@ -126,15 +177,9 @@ protected void checkUpdateable()
*/
public int remainingCount()
{
- return comparator.size() - size;
+ return comparator.size() - columns.size();
Review Comment:
Nit: double blank line after this method
##########
src/java/org/apache/cassandra/db/MultiCBuilder.java:
##########
@@ -19,104 +19,155 @@
import java.nio.ByteBuffer;
import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import java.util.NavigableSet;
+import org.apache.cassandra.cql3.statements.Bound;
+import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.schema.ColumnMetadata;
import org.apache.cassandra.utils.ByteBufferUtil;
+import org.apache.cassandra.utils.UniqueComparator;
import org.apache.cassandra.utils.btree.BTreeSet;
/**
* Builder that allow to build multiple Clustering/ClusteringBound at the same
time.
*/
-public abstract class MultiCBuilder
+public class MultiCBuilder
{
+ /**
+ * Represents a building block of a clustering.
+ * Either a point (single value) or a bound.
+ * Knowing the kind of elements greatly simplifies the process of building
clustering bounds.
+ */
+ public static class Element
+ {
+ public enum Kind
+ {
+ POINT, INCL_START, EXCL_START, INCL_END, EXCL_END
+ }
+
+ public static Element BOTTOM = new Element(Collections.emptyList(),
Kind.INCL_START);
+ public static Element TOP = new Element(Collections.emptyList(),
Kind.INCL_END);
+ public static Element ROOT = new Element(Collections.emptyList(),
Kind.POINT);
+
+ final List<ByteBuffer> value;
+ final Kind kind;
+
+
+ private Element(List<ByteBuffer> values, Kind kind)
+ {
+ this.value = values;
+ this.kind = kind;
+ }
+
+ public static Element point(ByteBuffer value)
+ {
+ return point(Collections.singletonList(value));
+ }
+
+ public static Element point(List<ByteBuffer> value)
+ {
+ return new Element(value, Kind.POINT);
+ }
+
+ public static Element bound(ByteBuffer value, Bound bound, boolean
inclusive)
+ {
+ return bound(Collections.singletonList(value), bound, inclusive);
+ }
+
+ public static Element bound(List<ByteBuffer> value, Bound bound,
boolean inclusive)
+ {
+ Kind kind = bound.isStart()
+ ? (inclusive ? Kind.INCL_START : Kind.EXCL_START)
+ : (inclusive ? Kind.INCL_END : Kind.EXCL_END);
+ return new Element(value, kind);
+ }
+
+ public boolean isBound()
+ {
+ return kind != Kind.POINT;
+ }
+
+ public boolean isStart()
+ {
+ return kind == Element.Kind.EXCL_START ||
+ kind == Element.Kind.INCL_START;
+ }
+
+ public boolean isInclusive()
+ {
+ return kind == Kind.INCL_START ||
+ kind == Kind.INCL_END ||
+ kind == Kind.POINT;
+ }
+
+ public String toString()
+ {
+ return "Element{" +
+ "kind=" + kind +
+ ", value=" + value +
+ '}';
+ }
+ }
+
/**
* The table comparator.
*/
- protected final ClusteringComparator comparator;
+ private final ClusteringComparator comparator;
/**
- * The number of clustering elements that have been added.
+ * Columns corresponding to the already added elements.
*/
- protected int size;
+ private final List<ColumnMetadata> columns = new ArrayList<>();
+
+ /**
+ * The elements of the clusterings.
+ */
+ private List<Element> elements = Collections.singletonList(Element.ROOT);
+
/**
* <code>true</code> if the clusterings have been build,
<code>false</code> otherwise.
*/
- protected boolean built;
+ private boolean built;
/**
* <code>true</code> if the clusterings contains some <code>null</code>
elements.
*/
- protected boolean containsNull;
+ private boolean containsNull;
/**
* <code>true</code> if the composites contains some <code>unset</code>
elements.
*/
- protected boolean containsUnset;
+ private boolean containsUnset;
/**
- * <code>true</code> if some empty collection have been added.
+ * <code>true</code> if the composites contains some slice bound elements.
*/
- protected boolean hasMissingElements;
+ private boolean containsSliceBound;
- protected MultiCBuilder(ClusteringComparator comparator)
+
+ private MultiCBuilder(ClusteringComparator comparator)
{
this.comparator = comparator;
}
/**
* Creates a new empty {@code MultiCBuilder}.
*/
- public static MultiCBuilder create(ClusteringComparator comparator,
boolean forMultipleValues)
+ public static MultiCBuilder create(ClusteringComparator comparator)
{
- return forMultipleValues
- ? new MultiClusteringBuilder(comparator)
- : new OneClusteringBuilder(comparator);
+ return new MultiCBuilder(comparator);
Review Comment:
Nit: double blank line after this method
##########
src/java/org/apache/cassandra/index/sai/disk/EmptyIndex.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.index.sai.disk;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.cassandra.db.PartitionPosition;
+import org.apache.cassandra.db.virtual.SimpleDataSet;
+import org.apache.cassandra.dht.AbstractBounds;
+import org.apache.cassandra.index.sai.IndexContext;
+import org.apache.cassandra.index.sai.QueryContext;
+import org.apache.cassandra.index.sai.SSTableContext;
+import org.apache.cassandra.index.sai.iterators.KeyRangeIterator;
+import org.apache.cassandra.index.sai.plan.Expression;
+import org.apache.cassandra.utils.ByteBufferUtil;
Review Comment:
Nit: unused import. Running `ant check` shows nine other unused imports, I
guess CI hasn't been run yet for this patch?
##########
test/unit/org/apache/cassandra/index/sai/cql/CollectionIndexingTest.java:
##########
@@ -188,6 +253,7 @@ private void createPopulatedMap(String createIndex)
put(1, "v1");
put(2, "v3");
}});
+
Review Comment:
Nit: unneeded blank line
##########
src/java/org/apache/cassandra/db/MultiCBuilder.java:
##########
@@ -157,358 +202,159 @@ public boolean containsUnset()
}
/**
- * Checks if some empty list of values have been added
- * @return <code>true</code> if the clusterings have some missing
elements, <code>false</code> otherwise.
+ * Returns the current number of results when {@link #build()} is called
+ *
+ * @return the current number of build results
*/
- public boolean hasMissingElements()
+ public int buildSize()
{
- return hasMissingElements;
+ return elements.size();
}
/**
- * Builds the <code>clusterings</code>.
+ * Checks if some elements can still be added to the clusterings.
*
- * @return the clusterings
+ * @return <code>true</code> if it is possible to add more elements to the
clusterings, <code>false</code> otherwise.
*/
- public abstract NavigableSet<Clustering<?>> build();
+ public boolean hasRemaining()
+ {
+ return remainingCount() > 0;
+ }
Review Comment:
Nit: double blank line after this method
--
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]
