CNDB-15280: Address review feedback on fixing AbstractReadQuery.toCQLString

Author: adelapena

src/java/org/apache/cassandra/db/Clustering.java

@@ -49,7 +49,7 @@ public default Clustering<?> clone(ByteBufferCloner cloner)
         ByteBuffer[] newValues = new ByteBuffer[size()];
         for (int i = 0; i < size(); i++)
         {
-            ByteBuffer val = accessor().toBuffer(get(i));
+            ByteBuffer val = bufferAt(i);
             newValues[i] = val == null ? null : cloner.clone(val);
         }
         return new BufferClustering(newValues);
@@ -84,7 +84,7 @@ public default String toCQLString(TableMetadata metadata)
         for (int i = 0; i < size(); i++)
         {
             ColumnMetadata c = metadata.clusteringColumns().get(i);
-            ByteBuffer value = accessor().toBuffer(get(i));
+            ByteBuffer value = bufferAt(i);
             sb.append(i == 0 ? "" : ", ").append(c.type.toCQLString(value));
         }
         return sb.toString();

src/java/org/apache/cassandra/db/ClusteringPrefix.java

@@ -358,11 +358,11 @@ default int dataSize()
     default ByteBuffer serializeAsPartitionKey()
     {
         if (size() == 1)
-            return accessor().toBuffer(get(0));
+            return bufferAt(0);
 
         ByteBuffer[] values = new ByteBuffer[size()];
         for (int i = 0; i < size(); i++)
-            values[i] = accessor().toBuffer(get(i));
+            values[i] = bufferAt(i);
         return CompositeType.build(ByteBufferAccessor.instance, values);
     }
 
@@ -752,4 +752,4 @@ public static boolean equals(ClusteringPrefix<?> prefix, Object o)
         return equals(prefix, (ClusteringPrefix<?>) o);
     }
 
-}
+}

src/java/org/apache/cassandra/db/MultiPartitionReadQuery.java

@@ -56,7 +56,7 @@ private boolean appendRanges(CqlBuilder builder)
     {
         List<DataRange> ranges = ranges();
         boolean hasRangeRestrictions = false;
-        if (ranges().size() == 1)
+        if (ranges.size() == 1)
         {
             String rangeString = ranges.get(0).toCQLString(metadata());
             if (!rangeString.isEmpty())

src/java/org/apache/cassandra/db/Slices.java

@@ -611,7 +611,7 @@ public String toCQLString(TableMetadata metadata, RowFilter rowFilter)
                     if (values.size() == 1)
                     {
                         sb.append(" = ").append(column.type.toCQLString(first.startValue));
-                        rowFilter = rowFilter.without(column, Operator.EQ, first.startValue);
+                        rowFilter = rowFilter.withoutFirstLevelExpression(column, Operator.EQ, first.startValue);
                     }
                     else
                     {
@@ -620,7 +620,7 @@ public String toCQLString(TableMetadata metadata, RowFilter rowFilter)
                         for (ByteBuffer value : values)
                         {
                             sb.append(j++ == 0 ? "" : ", ").append(column.type.toCQLString(value));
-                            rowFilter = rowFilter.without(column, Operator.EQ, value);
+                            rowFilter = rowFilter.withoutFirstLevelExpression(column, Operator.EQ, value);
                         }
                         sb.append(')');
                     }
@@ -644,7 +644,7 @@ public String toCQLString(TableMetadata metadata, RowFilter rowFilter)
                             operator = first.startInclusive ? Operator.GTE : Operator.GT;
                         sb.append(' ').append(operator).append(' ')
                           .append(column.type.toCQLString(first.startValue));
-                        rowFilter = rowFilter.without(column, operator, first.startValue);
+                        rowFilter = rowFilter.withoutFirstLevelExpression(column, operator, first.startValue);
                     }
                     if (first.endValue != null)
                     {
@@ -658,11 +658,11 @@ public String toCQLString(TableMetadata metadata, RowFilter rowFilter)
                             operator = first.endInclusive ? Operator.LTE : Operator.LT;
                         sb.append(' ').append(operator).append(' ')
                           .append(column.type.toCQLString(first.endValue));
-                        rowFilter = rowFilter.without(column, operator, first.endValue);
+                        rowFilter = rowFilter.withoutFirstLevelExpression(column, operator, first.endValue);
                     }
                 }
             }
-            rowFilter = rowFilter.without(metadata.clusteringColumns().get(0), Operator.IN);
+            rowFilter = rowFilter.withoutFirstLevelExpression(metadata.clusteringColumns().get(0), Operator.IN);
 
             // Append the row filter.
             if (!rowFilter.isEmpty())

src/java/org/apache/cassandra/db/filter/AbstractClusteringIndexFilter.java

@@ -19,10 +19,10 @@
 
 import java.io.IOException;
 
-import org.apache.cassandra.db.marshal.ReversedType;
 import org.apache.cassandra.schema.ColumnMetadata;
 import org.apache.cassandra.schema.TableMetadata;
 import org.apache.cassandra.db.*;
+import org.apache.cassandra.db.marshal.ReversedType;
 import org.apache.cassandra.io.util.DataInputPlus;
 import org.apache.cassandra.io.util.DataOutputPlus;
 

src/java/org/apache/cassandra/db/filter/ClusteringIndexNamesFilter.java

@@ -186,10 +186,10 @@ public String toCQLString(TableMetadata metadata, RowFilter rowFilter)
               .append(isSingleColumn ? "" : ')');
 
             for (int j = 0; j < clustering.size(); j++)
-                rowFilter = rowFilter.without(metadata.clusteringColumns().get(j), Operator.EQ, clustering.bufferAt(j));
+                rowFilter = rowFilter.withoutFirstLevelExpression(metadata.clusteringColumns().get(j), Operator.EQ, clustering.bufferAt(j));
         }
         sb.append(isSingleClustering ? "" : ")");
-        rowFilter = rowFilter.without(metadata.clusteringColumns().get(0), Operator.IN);
+        rowFilter = rowFilter.withoutFirstLevelExpression(metadata.clusteringColumns().get(0), Operator.IN);
 
         if (!rowFilter.isEmpty())
         {

src/java/org/apache/cassandra/db/filter/RowFilter.java

@@ -285,8 +285,8 @@ public boolean clusteringKeyRestrictionsAreSatisfiedBy(Clustering<?> clustering)
     }
 
     /**
-     * Returns this filter but without the provided expression. This method
-     * *assumes* that the filter contains the provided expression.
+     * Returns this filter but without the provided expression. This method *assumes* that the filter contains the
+     * provided expression, and it looks in all the levels of the filter tree.
      */
     public RowFilter without(Expression expression)
     {
@@ -298,19 +298,19 @@ public RowFilter without(Expression expression)
     }
 
     /**
-     * Returns a copy of this filter but without the provided first level expressions.
-     * If this filter doesn't contain the specified expression this method will just return an identical copy of this filter.
+     * Returns a copy of this filter but without first level expressions with the provided column, operator and value.
+     * If this filter doesn't contain the specified expressions this method will just return an identical copy of this filter.
      */
-    public RowFilter without(ColumnMetadata column, Operator op, ByteBuffer value)
+    public RowFilter withoutFirstLevelExpression(ColumnMetadata column, Operator op, ByteBuffer value)
     {
         return restrictFirstLevel(e -> !(e.column.equals(column) && e.operator == op && e.value.equals(value)));
     }
 
     /**
-     * Returns a copy of this filter but without the provided first level expressions.
-     * If this filter doesn't contain the specified expression this method will just return an identical copy of this filter.
+     * Returns a copy of this filter but without first level expressions with the provided column and operator.
+     * If this filter doesn't contain the specified expressions this method will just return an identical copy of this filter.
      */
-    public RowFilter without(ColumnMetadata column, Operator op)
+    public RowFilter withoutFirstLevelExpression(ColumnMetadata column, Operator op)
     {
         return restrictFirstLevel(e -> !(e.column.equals(column) && e.operator == op));
     }

src/java/org/apache/cassandra/schema/ColumnMetadata.java

@@ -594,7 +594,7 @@ public static String toCQLString(Iterator<ColumnMetadata> defs)
         StringBuilder sb = new StringBuilder();
         sb.append(defs.next().name.toCQLString());
         while (defs.hasNext())
-            sb.append(", ").append(defs.next().name);
+            sb.append(", ").append(defs.next().name.toCQLString());
         return sb.toString();
     }