You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ad...@apache.org on 2021/03/10 17:36:21 UTC
[cassandra] branch cassandra-3.0 updated: Fix
ColumnFilter::toString not returning a valid CQL fragment
This is an automated email from the ASF dual-hosted git repository.
adelapena pushed a commit to branch cassandra-3.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-3.0 by this push:
new 9c7b69e Fix ColumnFilter::toString not returning a valid CQL fragment
9c7b69e is described below
commit 9c7b69e4229b6817b248bcf5270a783eed1f9930
Author: Andrés de la Peña <a....@gmail.com>
AuthorDate: Wed Mar 10 17:19:17 2021 +0000
Fix ColumnFilter::toString not returning a valid CQL fragment
patch by Andrés de la Peña; reviewed by Benjamin Lerer for CASSANDRA-16483
---
CHANGES.txt | 1 +
src/java/org/apache/cassandra/db/ReadCommand.java | 4 +-
.../apache/cassandra/db/filter/ColumnFilter.java | 43 +++++++++++++++-------
.../cassandra/db/filter/ColumnFilterTest.java | 36 ++++++++++++++++++
4 files changed, 69 insertions(+), 15 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 567764e..25f938b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -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:
diff --git a/src/java/org/apache/cassandra/db/ReadCommand.java b/src/java/org/apache/cassandra/db/ReadCommand.java
index 925708e..4fd4b5f 100644
--- a/src/java/org/apache/cassandra/db/ReadCommand.java
+++ b/src/java/org/apache/cassandra/db/ReadCommand.java
@@ -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)
diff --git a/src/java/org/apache/cassandra/db/filter/ColumnFilter.java b/src/java/org/apache/cassandra/db/filter/ColumnFilter.java
index 520c43c..cbc8871 100644
--- a/src/java/org/apache/cassandra/db/filter/ColumnFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/ColumnFilter.java
@@ -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 "[]";
+
+ return toString(false);
+ }
+
+ public String toCQLString()
+ {
+ if (queried == null || queried.isEmpty())
+ return "*";
- StringJoiner joiner = new StringJoiner(", ", "[", "]");
- Iterator<ColumnDefinition> it = queried.selectOrderIterator();
- while (it.hasNext())
+ 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++)
{
diff --git a/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java b/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java
index 80c1e42..7977c6b 100644
--- a/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java
+++ b/test/unit/org/apache/cassandra/db/filter/ColumnFilterTest.java
@@ -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);
@@ -115,11 +121,29 @@ public class ColumnFilterTest
}
@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);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org