Fix ColumnFilter::toString not returning a valid CQL fragment

patch by Andrés de la Peña; reviewed by Benjamin Lerer for CASSANDRA-16483
This commit is contained in:
Andrés de la Peña 2021-03-10 17:19:17 +00:00
parent 81e472571a
commit 9c7b69e422
4 changed files with 69 additions and 15 deletions

View File

@ -1,4 +1,5 @@
3.0.25:
* Fix ColumnFilter::toString not returning a valid CQL fragment (CASSANDRA-16483)
* Fix ColumnFilter behaviour to prevent digest mitmatches during upgrades (CASSANDRA-16415)
* Avoid pushing schema mutations when setting up distributed system keyspaces locally (CASSANDRA-16387)
Merged from 2.2:

View File

@ -595,8 +595,8 @@ public abstract class ReadCommand implements ReadQuery
public String toCQLString()
{
StringBuilder sb = new StringBuilder();
sb.append("SELECT ").append(columnFilter());
sb.append(" FROM ").append(metadata().ksName).append('.').append(metadata.cfName);
sb.append("SELECT ").append(columnFilter().toCQLString());
sb.append(" FROM ").append(metadata().ksName).append('.').append(metadata().cfName);
appendCQLWhereClause(sb);
if (limits() != DataLimits.NONE)

View File

@ -134,6 +134,7 @@ public class ColumnFilter
/**
* Whether we can skip the value for the provided selected column.
*/
@SuppressWarnings("unused")
public boolean canSkipValue(ColumnDefinition column)
{
// We don't use that currently, see #10655 for more details.
@ -275,7 +276,7 @@ public class ColumnFilter
SortedSetMultimap<ColumnIdentifier, ColumnSubselection> s = null;
if (subSelections != null)
{
s = TreeMultimap.create(Comparator.<ColumnIdentifier>naturalOrder(), Comparator.<ColumnSubselection>naturalOrder());
s = TreeMultimap.create(Comparator.naturalOrder(), Comparator.naturalOrder());
for (ColumnSubselection subSelection : subSelections)
s.put(subSelection.column().name, subSelection);
}
@ -305,27 +306,43 @@ public class ColumnFilter
@Override
public String toString()
{
String prefix = "";
if (isFetchAll)
return "*/*";
if (queried.isEmpty())
return prefix + "[]";
return "[]";
StringJoiner joiner = new StringJoiner(", ", "[", "]");
Iterator<ColumnDefinition> it = queried.selectOrderIterator();
while (it.hasNext())
return toString(false);
}
public String toCQLString()
{
if (queried == null || queried.isEmpty())
return "*";
return toString(true);
}
private String toString(boolean cql)
{
Iterator<ColumnDefinition> columns = queried.selectOrderIterator();
StringJoiner joiner = cql ? new StringJoiner(", ") : new StringJoiner(", ", "[", "]");
while (columns.hasNext())
{
ColumnDefinition column = it.next();
SortedSet<ColumnSubselection> s = subSelections != null ? subSelections.get(column.name) : Collections.emptySortedSet();
ColumnDefinition column = columns.next();
String columnName = cql ? column.name.toCQLString() : String.valueOf(column.name);
SortedSet<ColumnSubselection> s = subSelections != null
? subSelections.get(column.name)
: Collections.emptySortedSet();
if (s.isEmpty())
joiner.add(String.valueOf(column.name));
joiner.add(columnName);
else
s.forEach(subSel -> joiner.add(String.format("%s%s", column.name, subSel)));
s.forEach(subSel -> joiner.add(String.format("%s%s", columnName, subSel)));
}
return prefix + joiner.toString();
return joiner.toString();
}
public static class Serializer
@ -399,7 +416,7 @@ public class ColumnFilter
SortedSetMultimap<ColumnIdentifier, ColumnSubselection> subSelections = null;
if (hasSubSelections)
{
subSelections = TreeMultimap.create(Comparator.<ColumnIdentifier>naturalOrder(), Comparator.<ColumnSubselection>naturalOrder());
subSelections = TreeMultimap.create(Comparator.naturalOrder(), Comparator.naturalOrder());
int size = (int)in.readUnsignedVInt();
for (int i = 0; i < size; i++)
{

View File

@ -27,6 +27,7 @@ import org.junit.Test;
import org.apache.cassandra.config.CFMetaData;
import org.apache.cassandra.config.ColumnDefinition;
import org.apache.cassandra.cql3.ColumnIdentifier;
import org.apache.cassandra.db.PartitionColumns;
import org.apache.cassandra.db.marshal.Int32Type;
import org.apache.cassandra.db.marshal.SetType;
@ -52,12 +53,14 @@ public class ColumnFilterTest
.addStaticColumn("s2", SetType.getInstance(Int32Type.instance, true))
.addRegularColumn("v1", Int32Type.instance)
.addRegularColumn("v2", SetType.getInstance(Int32Type.instance, true))
.addRegularColumn(ColumnIdentifier.getInterned("Escaped Name", true), Int32Type.instance)
.build();
private final ColumnDefinition s1 = metadata.getColumnDefinition(ByteBufferUtil.bytes("s1"));
private final ColumnDefinition s2 = metadata.getColumnDefinition(ByteBufferUtil.bytes("s2"));
private final ColumnDefinition v1 = metadata.getColumnDefinition(ByteBufferUtil.bytes("v1"));
private final ColumnDefinition v2 = metadata.getColumnDefinition(ByteBufferUtil.bytes("v2"));
private final ColumnDefinition escaped = metadata.getColumnDefinition(ByteBufferUtil.bytes("Escaped Name"));
private final CellPath path0 = CellPath.create(ByteBufferUtil.bytes(0));
private final CellPath path1 = CellPath.create(ByteBufferUtil.bytes(1));
private final CellPath path2 = CellPath.create(ByteBufferUtil.bytes(2));
@ -72,6 +75,7 @@ public class ColumnFilterTest
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("*/*", filter.toString());
assertEquals("*", filter.toCQLString());
assertFetchedQueried(true, true, filter, v1, v2, s1, s2);
assertCellFetchedQueried(true, true, filter, v2, path0, path1, path2, path3, path4);
assertCellFetchedQueried(true, true, filter, s2, path0, path1, path2, path3, path4);
@ -89,6 +93,7 @@ public class ColumnFilterTest
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("[]", filter.toString());
assertEquals("*", filter.toCQLString());
assertFetchedQueried(false, false, filter, v1, v2, s1, s2);
assertCellFetchedQueried(false, false, filter, v2, path0, path1, path2, path3, path4);
assertCellFetchedQueried(false, false, filter, s2, path0, path1, path2, path3, path4);
@ -104,6 +109,7 @@ public class ColumnFilterTest
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("[v1]", filter.toString());
assertEquals("v1", filter.toCQLString());
assertFetchedQueried(true, true, filter, v1);
assertFetchedQueried(false, false, filter, v2, s1, s2);
assertCellFetchedQueried(false, false, filter, v2, path0, path1, path2, path3, path4);
@ -114,12 +120,30 @@ public class ColumnFilterTest
check.accept(ColumnFilter.selectionBuilder().add(v1).build());
}
@Test
public void testSelectEscapedColumn()
{
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("[Escaped Name]", filter.toString());
assertEquals("\"Escaped Name\"", filter.toCQLString());
assertFetchedQueried(true, true, filter, escaped);
assertFetchedQueried(false, false, filter, v2, s1, s2);
assertCellFetchedQueried(false, false, filter, v2, path0, path1, path2, path3, path4);
assertCellFetchedQueried(false, false, filter, s2, path0, path1, path2, path3, path4);
};
check.accept(ColumnFilter.selection(PartitionColumns.builder().add(escaped).build()));
check.accept(ColumnFilter.selectionBuilder().add(escaped).build());
}
@Test
public void testSelectComplexColumn()
{
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("[v2]", filter.toString());
assertEquals("v2", filter.toCQLString());
assertFetchedQueried(true, true, filter, v2);
assertFetchedQueried(false, false, filter, v1, s1, s2);
assertCellFetchedQueried(true, true, filter, v2, path0, path1, path2, path3, path4);
@ -136,6 +160,7 @@ public class ColumnFilterTest
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("[s1]", filter.toString());
assertEquals("s1", filter.toCQLString());
assertFetchedQueried(true, true, filter, s1);
assertFetchedQueried(false, false, filter, v1, v2, s2);
assertCellFetchedQueried(false, false, filter, v2, path0, path1, path2, path3, path4);
@ -152,6 +177,7 @@ public class ColumnFilterTest
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("[s2]", filter.toString());
assertEquals("s2", filter.toCQLString());
assertFetchedQueried(true, true, filter, s2);
assertFetchedQueried(false, false, filter, v1, v2, s1);
assertCellFetchedQueried(false, false, filter, v2, path0, path1, path2, path3, path4);
@ -168,6 +194,7 @@ public class ColumnFilterTest
Consumer<ColumnFilter> check = filter -> {
testRoundTrips(filter);
assertEquals("[s1, s2, v1, v2]", filter.toString());
assertEquals("s1, s2, v1, v2", filter.toCQLString());
assertFetchedQueried(true, true, filter, v1, v2, s1, s2);
assertCellFetchedQueried(true, true, filter, v2, path0, path1, path2, path3, path4);
assertCellFetchedQueried(true, true, filter, s2, path0, path1, path2, path3, path4);
@ -183,6 +210,7 @@ public class ColumnFilterTest
ColumnFilter filter = ColumnFilter.selectionBuilder().select(v2, path1).select(v2, path3).build();
testRoundTrips(filter);
assertEquals("[v2[1], v2[3]]", filter.toString());
assertEquals("v2[1], v2[3]", filter.toCQLString());
assertFetchedQueried(true, true, filter, v2);
assertFetchedQueried(false, false, filter, v1, s1, s2);
assertCellFetchedQueried(true, true, filter, v2, path1, path3);
@ -196,6 +224,7 @@ public class ColumnFilterTest
ColumnFilter filter = ColumnFilter.selectionBuilder().select(s2, path1).select(s2, path3).build();
testRoundTrips(filter);
assertEquals("[s2[1], s2[3]]", filter.toString());
assertEquals("s2[1], s2[3]", filter.toCQLString());
assertFetchedQueried(true, true, filter, s2);
assertFetchedQueried(false, false, filter, v1, v2, s1);
assertCellFetchedQueried(false, false, filter, v2, path0, path1, path2, path3, path4);
@ -209,6 +238,7 @@ public class ColumnFilterTest
ColumnFilter filter = ColumnFilter.selectionBuilder().slice(v2, path1, path3).build();
testRoundTrips(filter);
assertEquals("[v2[1:3]]", filter.toString());
assertEquals("v2[1:3]", filter.toCQLString());
assertFetchedQueried(true, true, filter, v2);
assertFetchedQueried(false, false, filter, v1, s1, s2);
assertCellFetchedQueried(true, true, filter, v2, path1, path2, path3);
@ -222,6 +252,7 @@ public class ColumnFilterTest
ColumnFilter filter = ColumnFilter.selectionBuilder().slice(s2, path1, path3).build();
testRoundTrips(filter);
assertEquals("[s2[1:3]]", filter.toString());
assertEquals("s2[1:3]", filter.toCQLString());
assertFetchedQueried(true, true, filter, s2);
assertFetchedQueried(false, false, filter, v1, v2, s1);
assertCellFetchedQueried(false, false, filter, v2, path0, path1, path2, path3, path4);
@ -242,6 +273,7 @@ public class ColumnFilterTest
.build();
testRoundTrips(filter);
assertEquals("[s1, s2[0], s2[2:4], v1, v2[0:2], v2[4]]", filter.toString());
assertEquals("s1, s2[0], s2[2:4], v1, v2[0:2], v2[4]", filter.toCQLString());
assertFetchedQueried(true, true, filter, v1, v2, s1, s2);
assertCellFetchedQueried(true, true, filter, v2, path0, path1, path2, path4);
assertCellFetchedQueried(false, false, filter, v2, path3);
@ -259,6 +291,7 @@ public class ColumnFilterTest
assertFetchedQueried(true, true, filter, v1);
assertEquals("*/*", filter.toString());
assertEquals("v1", filter.toCQLString());
assertFetchedQueried(true, true, filter, s1, s2, v2);
assertCellFetchedQueried(true, true, filter, v2, path0, path1, path2, path3, path4);
assertCellFetchedQueried(true, true, filter, s2, path0, path1, path2, path3, path4);
@ -276,6 +309,7 @@ public class ColumnFilterTest
assertFetchedQueried(true, true, filter, s1);
assertEquals("*/*", filter.toString());
assertEquals("s1", filter.toCQLString());
assertFetchedQueried(true, true, filter, v1, v2, s2);
assertCellFetchedQueried(true, true, filter, v2, path0, path1, path2, path3, path4);
assertCellFetchedQueried(true, true, filter, s2, path0, path1, path2, path3, path4);
@ -293,6 +327,7 @@ public class ColumnFilterTest
assertFetchedQueried(true, true, filter, v2);
assertEquals("*/*", filter.toString());
assertEquals("v2[1]", filter.toCQLString());
assertFetchedQueried(true, true, filter, s1, s2, v1);
assertCellFetchedQueried(true, true, filter, v2, path1);
assertCellFetchedQueried(true, false, filter, v2, path0, path2, path3, path4);
@ -307,6 +342,7 @@ public class ColumnFilterTest
assertFetchedQueried(true, true, filter, s2);
assertEquals("*/*", filter.toString());
assertEquals("s2[1]", filter.toCQLString());
assertFetchedQueried(true, true, filter, v1, v2, s1);
assertCellFetchedQueried(true, true, filter, v2, path0, path1, path2, path3, path4);
assertCellFetchedQueried(true, true, filter, s2, path1);