Skip to content

Commit cb73a96

Browse files
adelapenaadelapena
committed
CNDB-15280: Fix AbstractReadQuery.toCQLString
Various fixes for toCQLString mainly coming from CASSANDRA-16510, and removal of code duplication.
1 parent b9c402b commit cb73a96

28 files changed

+716
-257
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public default String toCQLString(TableMetadata metadata)
8484
for (int i = 0; i < size(); i++)
8585
{
8686
ColumnMetadata c = metadata.clusteringColumns().get(i);
87-
sb.append(i == 0 ? "" : ", ").append(c.type.getString(get(i), accessor()));
87+
ByteBuffer value = accessor().toBuffer(get(i));
88+
sb.append(i == 0 ? "" : ", ").append(c.type.toCQLString(value));
8889
}
8990
return sb.toString();
9091
}

‎src/java/org/apache/cassandra/db/DataRange.java‎

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.io.IOException;
2020
import java.nio.ByteBuffer;
2121

22-
import org.apache.cassandra.cql3.CqlBuilder;
2322
import org.apache.cassandra.schema.ColumnMetadata;
2423
import org.apache.cassandra.schema.TableMetadata;
2524
import org.apache.cassandra.db.filter.*;
@@ -277,54 +276,47 @@ public String toCQLString(TableMetadata metadata)
277276
if (isUnrestricted())
278277
return "";
279278

280-
CqlBuilder builder = new CqlBuilder();
281-
282-
boolean needAnd = false;
283-
284279
if (isSinglePartition())
285280
{
286281
/*
287282
* Single partition queries using an index are internally mapped to range commands where the start and end
288283
* key are the same. If that is the case, we want to print the query as an equality on the partition key
289284
* rather than a token range, as if it was a partition query, for better readability.
290285
*/
291-
builder.append(((DecoratedKey) startKey()).toCQLString(metadata));
292-
needAnd = true;
286+
return ((DecoratedKey) startKey()).toCQLString(metadata);
293287
}
294288
else
295289
{
290+
StringBuilder builder = new StringBuilder();
296291
if (!startKey().isMinimum())
297292
{
298-
appendClause(startKey(), builder, metadata, true, keyRange.isStartInclusive());
299-
needAnd = true;
293+
appendCQLClause(startKey(), builder, metadata, true, keyRange.isStartInclusive());
300294
}
301295
if (!stopKey().isMinimum())
302296
{
303-
if (needAnd)
297+
if (builder.length() > 0)
304298
builder.append(" AND ");
305-
appendClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive());
306-
needAnd = true;
299+
appendCQLClause(stopKey(), builder, metadata, false, keyRange.isEndInclusive());
307300
}
301+
return builder.toString();
308302
}
309-
310-
String filterString = clusteringIndexFilter.toCQLString(metadata);
311-
if (!filterString.isEmpty())
312-
builder.append(needAnd ? " AND " : "").append(filterString);
313-
314-
return builder.toString();
315303
}
316304

317-
private void appendClause(PartitionPosition pos, CqlBuilder builder, TableMetadata metadata, boolean isStart, boolean isInclusive)
305+
private void appendCQLClause(PartitionPosition pos,
306+
StringBuilder builder,
307+
TableMetadata metadata,
308+
boolean isStart,
309+
boolean isInclusive)
318310
{
319311
builder.append("token(");
320312
builder.append(ColumnMetadata.toCQLString(metadata.partitionKeyColumns()));
321313
builder.append(") ");
322314
if (pos instanceof DecoratedKey)
323315
{
324-
builder.append(getOperator(isStart, isInclusive)).append(" ");
316+
builder.append(getOperator(isStart, isInclusive)).append(' ');
325317
builder.append("token(");
326318
appendKeyString(builder, metadata.partitionKeyType, ((DecoratedKey)pos).getKey());
327-
builder.append(")");
319+
builder.append(')');
328320
}
329321
else
330322
{
@@ -343,18 +335,18 @@ private static String getOperator(boolean isStart, boolean isInclusive)
343335

344336
// TODO: this is reused in SinglePartitionReadCommand but this should not really be here. Ideally
345337
// we need a more "native" handling of composite partition keys.
346-
public static void appendKeyString(CqlBuilder builder, AbstractType<?> type, ByteBuffer key)
338+
public static void appendKeyString(StringBuilder builder, AbstractType<?> type, ByteBuffer key)
347339
{
348340
if (type instanceof CompositeType)
349341
{
350342
CompositeType ct = (CompositeType)type;
351343
ByteBuffer[] values = ct.split(key);
352344
for (int i = 0; i < ct.subTypes().size(); i++)
353-
builder.append(i == 0 ? "" : ", ").append(ct.subTypes().get(i).getString(values[i]));
345+
builder.append(i == 0 ? "" : ", ").append(ct.subTypes().get(i).toCQLString(values[i]));
354346
}
355347
else
356348
{
357-
builder.append(type.getString(key));
349+
builder.append(type.toCQLString(key));
358350
}
359351
}
360352

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,9 @@ public String toString()
170170
* Generate CQL representation of this partition key for the given table.
171171
* For single-column keys: "k = 0"
172172
* For multi-column keys: "k1 = 1 AND k2 = 2"
173+
*
174+
* @param metadata the table metadata
175+
* @param redact whether to redact the key value, as in "k1 = ? AND k2 = ?".
173176
*/
174177
public String toCQLString(TableMetadata metadata)
175178
{
@@ -189,7 +192,7 @@ public String toCQLString(TableMetadata metadata)
189192

190193
private static String toCQLString(ColumnMetadata metadata, ByteBuffer key)
191194
{
192-
return String.format("%s = %s", metadata.name.toCQLString(), metadata.type.getString(key));
195+
return String.format("%s = %s", metadata.name.toCQLString(), metadata.type.toCQLString(key));
193196
}
194197

195198
public Token getToken()

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

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright DataStax, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.apache.cassandra.db;
18+
19+
import java.util.List;
20+
21+
import org.apache.cassandra.cql3.CqlBuilder;
22+
import org.apache.cassandra.schema.TableMetadata;
23+
24+
/**
25+
* A {@code ReadQuery} for multiple partitions, restricted by one of more data ranges.
26+
*/
27+
public interface MultiPartitionReadQuery extends ReadQuery
28+
{
29+
List<DataRange> ranges();
30+
31+
default void appendCQLWhereClause(CqlBuilder builder)
32+
{
33+
List<DataRange> ranges = ranges();
34+
if (ranges.size() == 1 && ranges.get(0).isUnrestricted() && rowFilter().isEmpty())
35+
return;
36+
37+
// Append the data ranges.
38+
TableMetadata metadata = metadata();
39+
boolean hasRanges = appendRanges(builder);
40+
41+
// Append the clustering index filter and the row filter.
42+
String filter = ranges.get(0).clusteringIndexFilter.toCQLString(metadata, rowFilter());
43+
if (!filter.isEmpty())
44+
{
45+
if (filter.startsWith("ORDER BY"))
46+
builder.append(" ");
47+
else if (hasRanges)
48+
builder.append(" AND ");
49+
else
50+
builder.append(" WHERE ");
51+
builder.append(filter);
52+
}
53+
}
54+
55+
private boolean appendRanges(CqlBuilder builder)
56+
{
57+
List<DataRange> ranges = ranges();
58+
boolean hasRangeRestrictions = false;
59+
if (ranges().size() == 1)
60+
{
61+
String rangeString = ranges.get(0).toCQLString(metadata());
62+
if (!rangeString.isEmpty())
63+
{
64+
builder.append(" WHERE ").append(rangeString);
65+
hasRangeRestrictions = true;
66+
}
67+
}
68+
else
69+
{
70+
builder.append(" WHERE (");
71+
for (int i = 0; i < ranges.size(); i++)
72+
{
73+
if (i > 0)
74+
builder.append(" OR ");
75+
builder.append(ranges.get(i).toCQLString(metadata()));
76+
}
77+
builder.append(')');
78+
hasRangeRestrictions = true;
79+
}
80+
return hasRangeRestrictions;
81+
}
82+
}

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

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
* Note: digest is not supported because each replica is responsible for different token ranges, there is no point on
5757
* sending digest.
5858
*/
59-
public class MultiRangeReadCommand extends ReadCommand
59+
public class MultiRangeReadCommand extends ReadCommand implements MultiPartitionReadQuery
6060
{
6161
protected static final SelectionDeserializer selectionDeserializer = new Deserializer();
6262

@@ -142,6 +142,7 @@ public boolean isSinglePartition()
142142
/**
143143
* @return all token ranges to be queried
144144
*/
145+
@Override
145146
public List<DataRange> ranges()
146147
{
147148
return dataRanges;
@@ -320,31 +321,9 @@ public Verb verb()
320321
}
321322

322323
@Override
323-
protected void appendCQLWhereClause(CqlBuilder builder)
324+
public void appendCQLWhereClause(CqlBuilder builder)
324325
{
325-
if (ranges().size() == 1 && ranges().get(0).isUnrestricted() && rowFilter().isEmpty())
326-
return;
327-
328-
builder.append(" WHERE ");
329-
// We put the row filter first because the data range can end by "ORDER BY"
330-
if (!rowFilter().isEmpty())
331-
{
332-
builder.append(rowFilter());
333-
builder.append(" AND ");
334-
}
335-
336-
boolean isFirst = true;
337-
for (int i = 0; i < ranges().size(); i++)
338-
{
339-
DataRange dataRange = ranges().get(i);
340-
if (!dataRange.isUnrestricted())
341-
{
342-
if (!isFirst)
343-
builder.append(" AND ");
344-
isFirst = false;
345-
builder.append(dataRange.toCQLString(metadata()));
346-
}
347-
}
326+
MultiPartitionReadQuery.super.appendCQLWhereClause(builder);
348327
}
349328

350329
@Override

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

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -394,23 +394,9 @@ public Verb verb()
394394
}
395395

396396
@Override
397-
protected void appendCQLWhereClause(CqlBuilder builder)
397+
public void appendCQLWhereClause(CqlBuilder builder)
398398
{
399-
if (dataRange.isUnrestricted() && rowFilter().isEmpty())
400-
return;
401-
402-
builder.append(" WHERE ");
403-
// We put the row filter first because the data range can end by "ORDER BY"
404-
if (!rowFilter().isEmpty())
405-
{
406-
builder.append(rowFilter());
407-
if (!dataRange.isUnrestricted())
408-
builder.append(" AND ");
409-
}
410-
if (!dataRange.isUnrestricted())
411-
{
412-
builder.append(dataRange.toCQLString(metadata()));
413-
}
399+
PartitionRangeReadQuery.super.appendCQLWhereClause(builder);
414400
}
415401

416402
@Override

‎src/java/org/apache/cassandra/db/PartitionRangeReadQuery.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
*/
1818
package org.apache.cassandra.db;
1919

20+
import java.util.List;
21+
2022
import org.apache.cassandra.db.filter.ColumnFilter;
2123
import org.apache.cassandra.db.filter.DataLimits;
2224
import org.apache.cassandra.db.filter.RowFilter;
@@ -29,7 +31,7 @@
2931
/**
3032
* A {@code ReadQuery} for a range of partitions.
3133
*/
32-
public interface PartitionRangeReadQuery extends ReadQuery
34+
public interface PartitionRangeReadQuery extends MultiPartitionReadQuery
3335
{
3436
static ReadQuery create(TableMetadata table,
3537
int nowInSec,
@@ -46,6 +48,12 @@ static ReadQuery create(TableMetadata table,
4648

4749
DataRange dataRange();
4850

51+
@Override
52+
default List<DataRange> ranges()
53+
{
54+
return List.of(dataRange());
55+
}
56+
4957
/**
5058
* Creates a new {@code PartitionRangeReadQuery} with the updated limits.
5159
*

‎src/java/org/apache/cassandra/db/ReadCommand.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.slf4j.Logger;
3838
import org.slf4j.LoggerFactory;
3939

40-
import org.apache.cassandra.cql3.CqlBuilder;
4140
import org.apache.cassandra.db.filter.ClusteringIndexFilter;
4241
import org.apache.cassandra.db.filter.ColumnFilter;
4342
import org.apache.cassandra.db.filter.DataLimits;
@@ -762,8 +761,6 @@ public Message<ReadCommand> createMessage(boolean trackRepairedData)
762761

763762
public abstract Verb verb();
764763

765-
protected abstract void appendCQLWhereClause(CqlBuilder builder);
766-
767764
// Skip purgeable tombstones. We do this because it's safe to do (post-merge of the memtable and sstable at least), it
768765
// can save us some bandwith, and avoid making us throw a TombstoneOverwhelmingException for purgeable tombstones (which
769766
// are to some extend an artefact of compaction lagging behind and hence counting them is somewhat unintuitive).

‎src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import org.apache.cassandra.db.transform.Transformation;
4242
import org.apache.cassandra.exceptions.RequestExecutionException;
4343
import org.apache.cassandra.index.Index;
44-
import org.apache.cassandra.index.sai.utils.RowWithSourceTable;
4544
import org.apache.cassandra.io.sstable.format.RowIndexEntry;
4645
import org.apache.cassandra.io.sstable.format.SSTableReader;
4746
import org.apache.cassandra.io.sstable.format.SSTableReadsListener;
@@ -1153,19 +1152,9 @@ public Verb verb()
11531152
}
11541153

11551154
@Override
1156-
protected void appendCQLWhereClause(CqlBuilder builder)
1155+
public void appendCQLWhereClause(CqlBuilder builder)
11571156
{
1158-
builder.append(" WHERE ");
1159-
1160-
builder.append(partitionKey().toCQLString(metadata()));
1161-
1162-
// We put the row filter first because the clustering index filter can end by "ORDER BY"
1163-
if (!rowFilter().isEmpty())
1164-
builder.append(" AND ").append(rowFilter());
1165-
1166-
String filterString = clusteringIndexFilter().toCQLString(metadata());
1167-
if (!filterString.isEmpty())
1168-
builder.append(" AND ").append(filterString);
1157+
SinglePartitionReadQuery.super.appendCQLWhereClause(builder);
11691158
}
11701159

11711160
protected void serializeSelection(DataOutputPlus out, int version) throws IOException

‎src/java/org/apache/cassandra/db/SinglePartitionReadQuery.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.apache.commons.lang3.tuple.Pair;
2828

29+
import org.apache.cassandra.cql3.CqlBuilder;
2930
import org.apache.cassandra.db.filter.ClusteringIndexFilter;
3031
import org.apache.cassandra.db.filter.ColumnFilter;
3132
import org.apache.cassandra.db.filter.DataLimits;
@@ -158,6 +159,24 @@ default boolean selectsClustering(DecoratedKey key, Clustering<?> clustering)
158159
return rowFilter().clusteringKeyRestrictionsAreSatisfiedBy(clustering);
159160
}
160161

162+
default void appendCQLWhereClause(CqlBuilder builder)
163+
{
164+
builder.append(" WHERE ");
165+
166+
// Append the partition key restrictions.
167+
TableMetadata metadata = metadata();
168+
builder.append(partitionKey().toCQLString(metadata));
169+
170+
// Append the clustering index filter and the row filter.
171+
String filter = clusteringIndexFilter().toCQLString(metadata(), rowFilter());
172+
if (!filter.isEmpty())
173+
{
174+
if (!clusteringIndexFilter().selectsAllPartition() || !rowFilter().isEmpty())
175+
builder.append(" AND ");
176+
builder.append(filter);
177+
}
178+
}
179+
161180
/**
162181
* Groups multiple single partition read queries.
163182
*/

0 commit comments

Comments
 (0)