CNDB-15760: Fix AbstractReadQuery.toCQLString

src/java/org/apache/cassandra/cql3/CqlBuilder.java

@@ -27,6 +27,7 @@
 import com.google.common.annotations.VisibleForTesting;
 
 import org.apache.cassandra.cql3.functions.FunctionName;
+import org.apache.cassandra.db.filter.RowFilter;
 import org.apache.cassandra.db.marshal.AbstractType;
 
 /**
@@ -63,6 +64,11 @@ public CqlBuilder(int capacity)
         builder = new StringBuilder(capacity);
     }
 
+    public int length()
+    {
+        return builder.length();
+    }
+
     public CqlBuilder append(Object o)
     {
         return append(String.valueOf(o));
@@ -274,6 +280,40 @@ public CqlBuilder appendOptions(Consumer<OptionsBuilder> builder)
         return this;
     }
 
+    /**
+     * Appends a row filter to the CQL string we are building.
+     *
+     * @param filter the row filter to append
+     * @param hasKeyRestrictions whether the query has key restrictions that have already been appended
+     * @return this CQL builder after appending the row filter
+     */
+    public CqlBuilder append(RowFilter filter, boolean hasKeyRestrictions)
+    {
+        return filter.isEmpty() ? this : appendRestrictions(filter.toCQLString(), hasKeyRestrictions);
+    }
+
+    /**
+     * Appends the specified CQL restrictions to the CQL string we are building.
+     *
+     * @param filter the CQL restrictions to append
+     * @param hasMoreRestrictions whether the CQL query already has other restrictions appended
+     * @return this CQL builder after appending the row filter
+     */
+    public CqlBuilder appendRestrictions(String filter, boolean hasMoreRestrictions)
+    {
+        if (filter.isEmpty())
+            return this;
+
+        if (filter.startsWith("ORDER BY"))
+            append(' ');
+        else if (hasMoreRestrictions)
+            append(" AND ");
+        else
+            append(" WHERE ");
+
+        return append(filter);
+    }
+
     public static class OptionsBuilder
     {
         private final CqlBuilder builder;

src/java/org/apache/cassandra/cql3/restrictions/SingleColumnRestriction.java

@@ -221,6 +221,7 @@ protected boolean isSupportedBy(Index index)
 
     public static class INRestriction extends SingleColumnRestriction
     {
+        public static final String CANNOT_BE_MERGED_ERROR = "%s cannot be restricted by more than one relation if it includes a IN";
         protected final MarkerOrTerms terms;
 
         public INRestriction(ColumnMetadata columnDef, MarkerOrTerms terms)
@@ -238,7 +239,7 @@ public final boolean isIN()
         @Override
         public final SingleRestriction doMergeWith(SingleRestriction otherRestriction)
         {
-            throw invalidRequest("%s cannot be restricted by more than one relation if it includes a IN", columnDef.name);
+            throw invalidRequest(CANNOT_BE_MERGED_ERROR, columnDef.name);
         }
 
         @Override

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,8 @@ public default String toCQLString(TableMetadata metadata)
         for (int i = 0; i < size(); i++)
         {
             ColumnMetadata c = metadata.clusteringColumns().get(i);
-            sb.append(i == 0 ? "" : ", ").append(c.type.getString(get(i), accessor()));
+            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/DataRange.java

@@ -19,7 +19,6 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 
-import org.apache.cassandra.cql3.CqlBuilder;
 import org.apache.cassandra.schema.ColumnMetadata;
 import org.apache.cassandra.schema.TableMetadata;
 import org.apache.cassandra.db.filter.*;
@@ -277,54 +276,47 @@ public String toCQLString(TableMetadata metadata)
         if (isUnrestricted())
             return "";
 
-        CqlBuilder builder = new CqlBuilder();
-
-        boolean needAnd = false;
-
         if (isSinglePartition())
         {
             /*
              * Single partition queries using an index are internally mapped to range commands where the start and end
              * key are the same. If that is the case, we want to print the query as an equality on the partition key
              * rather than a token range, as if it was a partition query, for better readability.
              */
-            builder.append(((DecoratedKey) startKey()).toCQLString(metadata));
-            needAnd = true;
+            return ((DecoratedKey) startKey()).toCQLString(metadata);
         }
         else
         {
+            StringBuilder builder = new StringBuilder();
             if (!startKey().isMinimum())
             {
-                appendClause(startKey(), builder, metadata, true, keyRange.isStartInclusive());
-                needAnd = true;
+                appendCQLClause(startKey(), builder, metadata, true, keyRange.isStartInclusive());
             }
             if (!stopKey().isMinimum())
             {
-                if (needAnd)
+                if (builder.length() > 0)
                     builder.append(" AND ");
-                appendClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive());
-                needAnd = true;
+                appendCQLClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive());
             }
+            return builder.toString();
         }
-
-        String filterString = clusteringIndexFilter.toCQLString(metadata);
-        if (!filterString.isEmpty())
-            builder.append(needAnd ? " AND " : "").append(filterString);
-
-        return builder.toString();
     }
 
-    private void appendClause(PartitionPosition pos, CqlBuilder builder, TableMetadata metadata, boolean isStart, boolean isInclusive)
+    private void appendCQLClause(PartitionPosition pos,
+                                 StringBuilder builder,
+                                 TableMetadata metadata,
+                                 boolean isStart,
+                                 boolean isInclusive)
     {
         builder.append("token(");
         builder.append(ColumnMetadata.toCQLString(metadata.partitionKeyColumns()));
         builder.append(") ");
         if (pos instanceof DecoratedKey)
         {
-            builder.append(getOperator(isStart, isInclusive)).append(" ");
+            builder.append(getOperator(isStart, isInclusive)).append(' ');
             builder.append("token(");
             appendKeyString(builder, metadata.partitionKeyType, ((DecoratedKey)pos).getKey());
-            builder.append(")");
+            builder.append(')');
         }
         else
         {
@@ -343,18 +335,18 @@ private static String getOperator(boolean isStart, boolean isInclusive)
 
     // TODO: this is reused in SinglePartitionReadCommand but this should not really be here. Ideally
     // we need a more "native" handling of composite partition keys.
-    public static void appendKeyString(CqlBuilder builder, AbstractType<?> type, ByteBuffer key)
+    public static void appendKeyString(StringBuilder builder, AbstractType<?> type, ByteBuffer key)
     {
         if (type instanceof CompositeType)
         {
             CompositeType ct = (CompositeType)type;
             ByteBuffer[] values = ct.split(key);
             for (int i = 0; i < ct.subTypes().size(); i++)
-                builder.append(i == 0 ? "" : ", ").append(ct.subTypes().get(i).getString(values[i]));
+                builder.append(i == 0 ? "" : ", ").append(ct.subTypes().get(i).toCQLString(values[i]));
         }
         else
         {
-            builder.append(type.getString(key));
+            builder.append(type.toCQLString(key));
         }
     }
 

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

@@ -170,6 +170,8 @@ public String toString()
      * Generate CQL representation of this partition key for the given table.
      * For single-column keys: "k = 0"
      * For multi-column keys: "k1 = 1 AND k2 = 2"
+     *
+     * @param metadata the table metadata
      */
     public String toCQLString(TableMetadata metadata)
     {
@@ -189,7 +191,7 @@ public String toCQLString(TableMetadata metadata)
 
     private static String toCQLString(ColumnMetadata metadata, ByteBuffer key)
     {
-        return String.format("%s = %s", metadata.name.toCQLString(), metadata.type.getString(key));
+        return String.format("%s = %s", metadata.name.toCQLString(), metadata.type.toCQLString(key));
     }
 
     public Token getToken()

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

@@ -0,0 +1,72 @@
+/*
+ * Copyright DataStax, Inc.
+ *
+ * Licensed 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.db;
+
+import java.util.List;
+
+import org.apache.cassandra.cql3.CqlBuilder;
+import org.apache.cassandra.schema.TableMetadata;
+
+/**
+ * A {@code ReadQuery} for multiple partitions, restricted by one of more data ranges.
+ */
+public interface MultiPartitionReadQuery extends ReadQuery
+{
+    List<DataRange> ranges();
+
+    default void appendCQLWhereClause(CqlBuilder builder)
+    {
+        // Append the data ranges.
+        TableMetadata metadata = metadata();
+        boolean hasRanges = appendRanges(builder);
+
+        // Append the clustering index filter and the row filter.
+        String filter = ranges().get(0).clusteringIndexFilter.toCQLString(metadata, rowFilter());
+        builder.appendRestrictions(filter, hasRanges);
+    }
+
+    private boolean appendRanges(CqlBuilder builder)
+    {
+        List<DataRange> ranges = ranges();
+        if (ranges.size() == 1)
+        {
+            DataRange range = ranges.get(0);
+            if (range.isUnrestricted())
+                return false;
+
+            String rangeString = range.toCQLString(metadata());
+            if (!rangeString.isEmpty())
+            {
+                builder.append(" WHERE ").append(rangeString);
+                return true;
+            }
+        }
+        else
+        {
+            builder.append(" WHERE ").append('(');
+            for (int i = 0; i < ranges.size(); i++)
+            {
+                if (i > 0)
+                    builder.append(" OR ");
+                builder.append(ranges.get(i).toCQLString(metadata()));
+            }
+            builder.append(')');
+            return true;
+        }
+        return false;
+    }
+}

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

@@ -56,7 +56,7 @@
  * Note: digest is not supported because each replica is responsible for different token ranges, there is no point on
  * sending digest.
  */
-public class MultiRangeReadCommand extends ReadCommand
+public class MultiRangeReadCommand extends ReadCommand implements MultiPartitionReadQuery
 {
     protected static final SelectionDeserializer selectionDeserializer = new Deserializer();
 
@@ -142,6 +142,7 @@ public boolean isSinglePartition()
     /**
      * @return all token ranges to be queried
      */
+    @Override
     public List<DataRange> ranges()
     {
         return dataRanges;
@@ -320,31 +321,9 @@ public Verb verb()
     }
 
     @Override
-    protected void appendCQLWhereClause(CqlBuilder builder)
+    public void appendCQLWhereClause(CqlBuilder builder)
     {
-        if (ranges().size() == 1 && ranges().get(0).isUnrestricted() && rowFilter().isEmpty())
-            return;
-
-        builder.append(" WHERE ");
-        // We put the row filter first because the data range can end by "ORDER BY"
-        if (!rowFilter().isEmpty())
-        {
-            builder.append(rowFilter());
-            builder.append(" AND ");
-        }
-
-        boolean isFirst = true;
-        for (int i = 0; i < ranges().size(); i++)
-        {
-            DataRange dataRange = ranges().get(i);
-            if (!dataRange.isUnrestricted())
-            {
-                if (!isFirst)
-                    builder.append(" AND ");
-                isFirst = false;
-                builder.append(dataRange.toCQLString(metadata()));
-            }
-        }
+        MultiPartitionReadQuery.super.appendCQLWhereClause(builder);
     }
 
     @Override

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

@@ -394,23 +394,9 @@ public Verb verb()
     }
 
     @Override
-    protected void appendCQLWhereClause(CqlBuilder builder)
+    public void appendCQLWhereClause(CqlBuilder builder)
     {
-        if (dataRange.isUnrestricted() && rowFilter().isEmpty())
-            return;
-
-        builder.append(" WHERE ");
-        // We put the row filter first because the data range can end by "ORDER BY"
-        if (!rowFilter().isEmpty())
-        {
-            builder.append(rowFilter());
-            if (!dataRange.isUnrestricted())
-                builder.append(" AND ");
-        }
-        if (!dataRange.isUnrestricted())
-        {
-            builder.append(dataRange.toCQLString(metadata()));
-        }
+        PartitionRangeReadQuery.super.appendCQLWhereClause(builder);
     }
 
     @Override