natas_hw
/
cassandra
mirror of https://github.com/apache/cassandra
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Treat IN values as a set instead of a list 

Patch by Tyler Hobbs; reviewed by Benjamin Lerer for CASSANDRA-12420
This commit is contained in:
Tyler Hobbs 2016-09-27 11:51:41 -05:00
parent 6fb89b905c
commit cdd535fcac
7 changed files with 41 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGES.txt
View File

@ -1,4 +1,7 @@
2.1.16
* Avoid infinitely looping result set when paging SELECT queries with
an IN clause with duplicate keys by treating the IN values as a set instead
of a list (CASSANDRA-12420)
* Add system property to set the max number of native transport requests in queue (CASSANDRA-11363)
* Include column family parameter when -st and -et are provided (CASSANDRA-11866)
* Fix queries with empty ByteBuffer values in clustering column restrictions (CASSANDRA-12127)

7
NEWS.txt
View File

@ -18,6 +18,13 @@ using the provided 'sstableupgrade' tool.
Upgrading
---------
- Duplicate partition keys in SELECT statement IN clauses will now be
filtered out, meaning that duplicate results will no longer be returned.
Futhermore, the partitions will be returned in the order of the sorted
partition keys instead of the order of the IN values; this matches the
behavior of Cassandra 2.2+. This was necessary to avoid an infinitely
looping result set when combined with paging under some circumstances.
See CASSANDRA-12420 for details.
- The ReversedType behaviour has been corrected for clustering columns of
BYTES type containing empty value. Scrub should be run on the existing
SSTables containing a descending clustering column of BYTES type to correct

45
src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
View File

@ -71,13 +71,6 @@ public class SelectStatement implements CQLStatement
private static final int DEFAULT_COUNT_PAGE_SIZE = 10000;
/**
* In the current version a query containing duplicate values in an IN restriction on the partition key will
* cause the same record to be returned multiple time. This behavior will be changed in 3.0 but until then
* we will log a warning the first time this problem occurs.
*/
private static volatile boolean HAS_LOGGED_WARNING_FOR_IN_RESTRICTION_WITH_DUPLICATES;
private final int boundTerms;
public final CFMetaData cfm;
public final Parameters parameters;
@ -682,9 +675,9 @@ public class SelectStatement implements CQLStatement
: limit;
}
private Collection<ByteBuffer> getKeys(final QueryOptions options) throws InvalidRequestException
private NavigableSet<ByteBuffer> getKeys(final QueryOptions options) throws InvalidRequestException
{
List<ByteBuffer> keys = new ArrayList<ByteBuffer>();
TreeSet<ByteBuffer> sortedKeys = new TreeSet<>(cfm.getKeyValidator());
CBuilder builder = cfm.getKeyValidatorAsCType().builder();
for (ColumnDefinition def : cfm.partitionKeyColumns())
{
@ -695,18 +688,14 @@ public class SelectStatement implements CQLStatement
if (builder.remainingCount() == 1)
{
if (values.size() > 1 && !HAS_LOGGED_WARNING_FOR_IN_RESTRICTION_WITH_DUPLICATES && containsDuplicates(values))
{
// This approach does not fully prevent race conditions but it is not a big deal.
HAS_LOGGED_WARNING_FOR_IN_RESTRICTION_WITH_DUPLICATES = true;
logger.warn("SELECT queries with IN restrictions on the partition key containing duplicate values will return duplicate rows.");
}
for (ByteBuffer val : values)
{
if (val == null)
throw new InvalidRequestException(String.format("Invalid null value for partition key part %s", def.name));
keys.add(builder.buildWith(val).toByteBuffer());
ByteBuffer keyBuffer = builder.buildWith(val).toByteBuffer();
validateKey(keyBuffer);
sortedKeys.add(keyBuffer);
}
}
else
@ -720,18 +709,22 @@ public class SelectStatement implements CQLStatement
builder.add(val);
}
}
return keys;
return sortedKeys;
}
/**
* Checks if the specified list contains duplicate values.
*
* @param values the values to check
* @return <code>true</code> if the specified list contains duplicate values, <code>false</code> otherwise.
*/
private static boolean containsDuplicates(List<ByteBuffer> values)
private void validateKey(ByteBuffer keyBuffer) throws InvalidRequestException
{
return new HashSet<>(values).size() < values.size();
if (keyBuffer == null || keyBuffer.remaining() == 0)
throw new InvalidRequestException("Key may not be empty");
try
{
cfm.getKeyValidator().validate(keyBuffer);
}
catch (MarshalException exc)
{
throw new InvalidRequestException("Partition key IN clause contained invalid value: " + exc);
}
}
private ByteBuffer getKeyBound(Bound b, QueryOptions options) throws InvalidRequestException

8
test/unit/org/apache/cassandra/cql3/validation/entities/FrozenCollectionsTest.java
View File

@ -85,8 +85,8 @@ public class FrozenCollectionsTest extends CQLTester
);
assertRows(execute("SELECT * FROM %s WHERE k IN ?", list(set(4, 5, 6), set())),
row(set(4, 5, 6), 0),
row(set(), 0)
row(set(), 0),
row(set(4, 5, 6), 0)
);
assertRows(execute("SELECT * FROM %s WHERE token(k) >= token(?)", set(4, 5, 6)),
@ -154,9 +154,9 @@ public class FrozenCollectionsTest extends CQLTester
);
assertRows(execute("SELECT * FROM %s WHERE k IN ?", list(map(set(4, 5, 6), list(1, 2, 3)), map(), map(set(), list(1, 2, 3)))),
row(map(set(4, 5, 6), list(1, 2, 3)), 0),
row(map(), 0),
row(map(set(), list(1, 2, 3)), 0)
row(map(set(), list(1, 2, 3)), 0),
row(map(set(4, 5, 6), list(1, 2, 3)), 0)
);
assertRows(execute("SELECT * FROM %s WHERE token(k) >= token(?)", map(set(4, 5, 6), list(1, 2, 3))),

2
test/unit/org/apache/cassandra/cql3/validation/operations/SelectLimitTest.java
View File

@ -72,7 +72,7 @@ public class SelectLimitTest extends CQLTester
// Check that we do limit the output to 1 *and* that we respect query
// order of keys (even though 48 is after 2)
assertRows(execute("SELECT * FROM %s WHERE userid IN (48, 2) LIMIT 1"),
row(48, "http://foo.com", 42L));
row(2, "http://foo.com", 42L));
}
/**

6
test/unit/org/apache/cassandra/cql3/validation/operations/SelectMultiColumnRelationTest.java
View File

@ -629,10 +629,10 @@ public class SelectMultiColumnRelationTest extends CQLTester
// same query, but reversed order for the IN values
assertRows(execute("SELECT * FROM %s WHERE a IN (?, ?) AND (b, c, d) IN (?, ?)", 1, 0, tuple(0, 1, 1), tuple(0, 1, 0)),
row(1, 0, 1, 0),
row(1, 0, 1, 1),
row(0, 0, 1, 0),
row(0, 0, 1, 1)
row(0, 0, 1, 1),
row(1, 0, 1, 0),
row(1, 0, 1, 1)
);
assertRows(execute("SELECT * FROM %s WHERE a IN (?, ?) and (b, c) IN ((?, ?))", 0, 1, 0, 1),

8
test/unit/org/apache/cassandra/cql3/validation/operations/SelectOrderByTest.java
View File

@ -517,12 +517,12 @@ public class SelectOrderByTest extends CQLTester
row(2),
row(0));
assertRows(execute("SELECT v FROM %s WHERE k IN (1, 0)"),
row(3),
row(4),
row(5),
row(0),
row(1),
row(2));
row(2),
row(3),
row(4),
row(5));
assertRows(execute("SELECT v FROM %s WHERE k IN (1, 0) ORDER BY c1 ASC"),
row(0),