Allows single-column slice restrictions to be merged with multi-columns slice restrictions

patch by Benjamin Lerer; reviewed by Sam Tunnicliffe for CASSANDRA-9606
This commit is contained in:
blerer 2015-08-09 21:48:04 +02:00
parent c9587cd2bd
commit de84a5c770
6 changed files with 167 additions and 32 deletions

View File

@ -24,8 +24,11 @@ import org.apache.cassandra.exceptions.InvalidRequestException;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.apache.cassandra.cql3.Term.Terminal;
public interface MultiColumnRestriction extends Restriction
{
public static class EQ extends SingleColumnRestriction.EQ implements MultiColumnRestriction
@ -128,8 +131,11 @@ public interface MultiColumnRestriction extends Restriction
*/
public List<ByteBuffer> componentBounds(Bound b, List<ByteBuffer> variables) throws InvalidRequestException
{
Tuples.Value value = (Tuples.Value)bounds[b.idx].bind(variables);
return value.getElements();
Terminal terminal = bounds[b.idx].bind(variables);
if (terminal instanceof Tuples.Value)
return ((Tuples.Value) terminal).getElements();
return Collections.singletonList(terminal.get());
}
}
}

View File

@ -54,6 +54,8 @@ public interface Restriction
/** Returns true if the start or end bound (depending on the argument) is set, false otherwise */
public boolean hasBound(Bound b);
public Term bound(Bound b);
public ByteBuffer bound(Bound b, List<ByteBuffer> variables) throws InvalidRequestException;
/** Returns true if the start or end bound (depending on the argument) is inclusive, false otherwise */
@ -64,5 +66,7 @@ public interface Restriction
public IndexOperator getIndexOperator(Bound b);
public void setBound(Relation.Type type, Term t) throws InvalidRequestException;
public void setBound(Slice restriction) throws InvalidRequestException;
}
}

View File

@ -46,7 +46,6 @@ import org.apache.cassandra.service.StorageProxy;
import org.apache.cassandra.service.StorageService;
import org.apache.cassandra.service.pager.*;
import org.apache.cassandra.db.ConsistencyLevel;
import org.apache.cassandra.thrift.ColumnDef;
import org.apache.cassandra.thrift.IndexExpression;
import org.apache.cassandra.thrift.IndexOperator;
import org.apache.cassandra.thrift.ThriftValidation;
@ -1718,13 +1717,6 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache
}
else
{
if (!existing.isMultiColumn())
{
throw new InvalidRequestException(String.format(
"Column \"%s\" cannot have both tuple-notation inequalities and single-column inequalities: %s",
name, relation));
}
boolean existingRestrictionStartBefore =
(i == 0 && name.position != 0 && stmt.columnRestrictions[name.position - 1] == existing);
@ -1733,7 +1725,7 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache
if (existingRestrictionStartBefore || existingRestrictionStartAfter)
{
throw new InvalidRequestException(String.format(
"Column \"%s\" cannot be restricted by two tuple-notation inequalities not starting with the same column: %s",
"Column \"%s\" cannot be restricted by inequalities not starting with the same column: %s",
name, relation));
}
@ -1793,9 +1785,21 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache
Term t = relation.getValue().prepare(names);
t.collectMarkerSpecification(boundNames);
Restriction.Slice restriction = (Restriction.Slice) getExistingRestriction(stmt, names.get(0));
if (restriction == null)
Restriction.Slice existingRestriction = (Restriction.Slice) getExistingRestriction(stmt, names.get(0));
Restriction.Slice restriction;
if (existingRestriction == null)
{
restriction = new MultiColumnRestriction.Slice(false);
}
else if (!existingRestriction.isMultiColumn())
{
restriction = new MultiColumnRestriction.Slice(false);
restriction.setBound(existingRestriction);
}
else
{
restriction = existingRestriction;
}
restriction.setBound(relation.operator(), t);
for (CFDefinition.Name name : names)
@ -1847,17 +1851,25 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache
switch (name.kind)
{
case KEY_ALIAS:
stmt.keyRestrictions[name.position] = updateSingleColumnRestriction(name, stmt.keyRestrictions[name.position], relation, names);
{
Restriction existingRestriction = stmt.keyRestrictions[name.position];
Restriction previousRestriction = name.position == 0 ? null : stmt.keyRestrictions[name.position - 1];
stmt.keyRestrictions[name.position] = updateSingleColumnRestriction(name, existingRestriction, previousRestriction, relation, names);
break;
}
case COLUMN_ALIAS:
stmt.columnRestrictions[name.position] = updateSingleColumnRestriction(name, stmt.columnRestrictions[name.position], relation, names);
{
Restriction existingRestriction = stmt.columnRestrictions[name.position];
Restriction previousRestriction = name.position == 0 ? null : stmt.columnRestrictions[name.position - 1];
stmt.columnRestrictions[name.position] = updateSingleColumnRestriction(name, existingRestriction, previousRestriction, relation, names);
break;
}
case VALUE_ALIAS:
throw new InvalidRequestException(String.format("Predicates on the non-primary-key column (%s) of a COMPACT table are not yet supported", name.name));
case COLUMN_METADATA:
case STATIC:
// We only all IN on the row key and last clustering key so far, never on non-PK columns, and this even if there's an index
Restriction r = updateSingleColumnRestriction(name, stmt.metadataRestrictions.get(name), relation, names);
Restriction r = updateSingleColumnRestriction(name, stmt.metadataRestrictions.get(name), null, relation, names);
if (r.isIN() && !((Restriction.IN)r).canHaveOnlyOneValue())
// Note: for backward compatibility reason, we conside a IN of 1 value the same as a EQ, so we let that slide.
throw new InvalidRequestException(String.format("IN predicates on non-primary-key columns (%s) is not yet supported", name));
@ -1866,7 +1878,7 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache
}
}
Restriction updateSingleColumnRestriction(CFDefinition.Name name, Restriction existingRestriction, SingleColumnRelation newRel, VariableSpecifications boundNames) throws InvalidRequestException
Restriction updateSingleColumnRestriction(CFDefinition.Name name, Restriction existingRestriction, Restriction previousRestriction, SingleColumnRelation newRel, VariableSpecifications boundNames) throws InvalidRequestException
{
ColumnSpecification receiver = name;
if (newRel.onToken)
@ -1927,6 +1939,10 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache
case LT:
case LTE:
{
// A slice restriction can be merged with another one under some conditions:
// 1) both restrictions are on a token function or non of them are
// (e.g. token(partitionKey) > token(?) AND token(partitionKey) <= token(?) or clustering1 > 1 AND clustering1 <= 2).
// 2) both restrictions needs to start with the same column (e.g clustering1 > 0 AND (clustering1, clustering2) <= (2, 1)).
if (existingRestriction == null)
existingRestriction = new SingleColumnRestriction.Slice(newRel.onToken);
else if (!existingRestriction.isSlice())
@ -1936,8 +1952,12 @@ public class SelectStatement implements CQLStatement, MeasurableForPreparedCache
// processPartitionKeysRestrictions, we shouldn't update the existing restriction by the new one if the old one was using token()
// and the new one isn't since that would bypass that later test.
throw new InvalidRequestException("Only EQ and IN relation are supported on the partition key (unless you use the token() function)");
else if (existingRestriction.isMultiColumn())
throw new InvalidRequestException(String.format("Column \"%s\" cannot be restricted by both a tuple notation inequality and a single column inequality (%s)", name, newRel));
if (name.position != 0 && previousRestriction == existingRestriction)
throw new InvalidRequestException(String.format(
"Column \"%s\" cannot be restricted by two inequalities not starting with the same column: %s",
name, newRel));
Term t = newRel.getValue().prepare(receiver);
t.collectMarkerSpecification(boundNames);
((SingleColumnRestriction.Slice)existingRestriction).setBound(newRel.operator(), t);

View File

@ -218,6 +218,11 @@ public abstract class SingleColumnRestriction implements Restriction
return bounds[b.idx] != null;
}
public Term bound(Bound b)
{
return bounds[b.idx];
}
public ByteBuffer bound(Bound b, List<ByteBuffer> variables) throws InvalidRequestException
{
return bounds[b.idx].bindAndGet(variables);
@ -279,12 +284,24 @@ public abstract class SingleColumnRestriction implements Restriction
throw new AssertionError();
}
if (bounds[b.idx] != null)
throw new InvalidRequestException(String.format(
"More than one restriction was found for the %s bound", b.name().toLowerCase()));
setBound(b, inclusive, t);
}
bounds[b.idx] = t;
boundInclusive[b.idx] = inclusive;
public void setBound(Restriction.Slice slice) throws InvalidRequestException
{
for (Bound bound : Bound.values())
if (slice.hasBound(bound))
setBound(bound, slice.isInclusive(bound), slice.bound(bound));
}
private void setBound(Bound bound, boolean inclusive, Term term) throws InvalidRequestException {
if (bounds[bound.idx] != null)
throw new InvalidRequestException(String.format(
"More than one restriction was found for the %s bound", bound.name().toLowerCase()));
bounds[bound.idx] = term;
boundInclusive[bound.idx] = inclusive;
}
@Override

View File

@ -296,9 +296,7 @@ public class MultiColumnRelationTest
for (String tableSuffix : new String[]{"", "_compact"})
{
String[] queries = new String[]{
"SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a = 0 AND (b, c, d) > (0, 1, 0) AND b < 1",
"SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a = 0 AND (b, c, d) > (0, 1, 0) AND c < 1",
"SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a = 0 AND b > 1 AND (b, c, d) < (1, 1, 0)",
"SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a = 0 AND c > 1 AND (b, c, d) < (1, 1, 0)",
"SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE (a, b, c, d) IN ((0, 1, 2, 3))",
"SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE (c, d) IN ((0, 1))",
@ -379,6 +377,12 @@ public class MultiColumnRelationTest
checkRow(0, results, 0, 1, 1, 0);
checkRow(1, results, 0, 1, 1, 1);
results = execute("SELECT * FROM %s.multiple_clustering" + tableSuffix
+ " WHERE a = 0 and b = 1 and (c, d) > (0, 0) and c <= 1");
assertEquals(2, results.size());
checkRow(0, results, 0, 1, 1, 0);
checkRow(1, results, 0, 1, 1, 1);
results = execute("SELECT * FROM %s.multiple_clustering" + tableSuffix
+ " WHERE a = 0 and b = 1 and (c, d) >= (0, 0) and (c, d) < (1, 1)");
assertEquals(2, results.size());
@ -476,6 +480,12 @@ public class MultiColumnRelationTest
checkRow(0, results, 0, 1, 1, 0);
checkRow(1, results, 0, 1, 1, 1);
results = execute("SELECT * FROM %s.multiple_clustering" + tableSuffix
+ " WHERE a = 0 and (b) = (1) and (c, d) > (0, 0) and c <= 1");
assertEquals(2, results.size());
checkRow(0, results, 0, 1, 1, 0);
checkRow(1, results, 0, 1, 1, 1);
results = execute("SELECT * FROM %s.multiple_clustering" + tableSuffix
+ " WHERE a = 0 and (b) = (1) and (c, d) >= (0, 0) and (c, d) < (1, 1)");
assertEquals(2, results.size());
@ -516,6 +526,14 @@ public class MultiColumnRelationTest
results = execute("SELECT * FROM %s.single_clustering" + tableSuffix + " WHERE a=0 AND (b) > (0) AND (b) < (2)");
assertEquals(1, results.size());
checkRow(0, results, 0, 1, 0);
results = execute("SELECT * FROM %s.single_clustering" + tableSuffix + " WHERE a=0 AND b > 0 AND (b) < (2)");
assertEquals(1, results.size());
checkRow(0, results, 0, 1, 0);
results = execute("SELECT * FROM %s.single_clustering" + tableSuffix + " WHERE a=0 AND (b) > (0) AND b < 2");
assertEquals(1, results.size());
checkRow(0, results, 0, 1, 0);
}
}
@ -606,6 +624,10 @@ public class MultiColumnRelationTest
assertEquals(1, results.size());
checkRow(0, results, 0, 0, 1, 1);
results = execute("SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a=0 AND (b, c, d) > (0, 1, 0) AND b < 1");
assertEquals(1, results.size());
checkRow(0, results, 0, 0, 1, 1);
results = execute("SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a=0 AND (b, c, d) > (0, 1, 1) AND (b, c) < (1, 1)");
assertEquals(1, results.size());
checkRow(0, results, 0, 1, 0, 0);
@ -972,6 +994,12 @@ public class MultiColumnRelationTest
prepare("SELECT * FROM %s.single_clustering WHERE a=0 AND (b) > (?) AND (b) > (?)");
}
@Test(expected=InvalidRequestException.class)
public void testPrepareMixMultipleInequalitiesOnSameBoundWithSingleColumnRestriction() throws Throwable
{
prepare("SELECT * FROM %s.single_clustering WHERE a=0 AND (b) > (?) AND b > ?");
}
@Test(expected=InvalidRequestException.class)
public void testPrepareClusteringColumnsOutOfOrderInInequality() throws Throwable
{
@ -1051,6 +1079,14 @@ public class MultiColumnRelationTest
results = executePrepared(prepare("SELECT * FROM %s.single_clustering" + tableSuffix + " WHERE a=0 AND (b) > (?) AND (b) < (?)"), makeIntOptions(0, 2));
assertEquals(1, results.size());
checkRow(0, results, 0, 1, 0);
results = executePrepared(prepare("SELECT * FROM %s.single_clustering" + tableSuffix + " WHERE a=0 AND (b) > (?) AND b < ?"), makeIntOptions(0, 2));
assertEquals(1, results.size());
checkRow(0, results, 0, 1, 0);
results = executePrepared(prepare("SELECT * FROM %s.single_clustering" + tableSuffix + " WHERE a=0 AND b > ? AND (b) < (?)"), makeIntOptions(0, 2));
assertEquals(1, results.size());
checkRow(0, results, 0, 1, 0);
}
}
@ -1129,6 +1165,10 @@ public class MultiColumnRelationTest
assertEquals(1, results.size());
checkRow(0, results, 0, 0, 1, 1);
results = executePrepared(prepare("SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a=0 AND (b, c, d) > (?, ?, ?) AND b < ?"), makeIntOptions(0, 1, 0, 1));
assertEquals(1, results.size());
checkRow(0, results, 0, 0, 1, 1);
results = executePrepared(prepare
("SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a=0 AND (b, c, d) > (?, ?, ?) AND (b, c) < (?, ?)"),
makeIntOptions(0, 1, 1, 1, 1));
@ -1195,6 +1235,11 @@ public class MultiColumnRelationTest
assertEquals(1, results.size());
checkRow(0, results, 0, 0, 1, 1);
results = executePrepared(prepare("SELECT * FROM %s.multiple_clustering" + tableSuffix
+ " WHERE a=0 AND (b, c, d) > ? AND b < ?"), options(tuple(0, 1, 0), ByteBufferUtil.bytes(1)));
assertEquals(1, results.size());
checkRow(0, results, 0, 0, 1, 1);
results = executePrepared(prepare
("SELECT * FROM %s.multiple_clustering" + tableSuffix + " WHERE a=0 AND (b, c, d) > ? AND (b, c) < ?"),
options(tuple(0, 1, 1), tuple(1, 1)));

View File

@ -21,6 +21,7 @@ import java.util.Iterator;
import java.util.List;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@ -45,7 +46,8 @@ public class SingleColumnRelationTest
{
SchemaLoader.loadSchema();
executeSchemaChange("CREATE KEYSPACE IF NOT EXISTS %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'}");
executeSchemaChange("CREATE TABLE IF NOT EXISTS %s.single_partition (a int PRIMARY KEY, b int, c text)");
executeSchemaChange("CREATE TABLE IF NOT EXISTS %s.compound_partition (a int, b int, c text, PRIMARY KEY ((a, b)))");
executeSchemaChange("CREATE TABLE IF NOT EXISTS %s.partition_with_indices (a int, b int, c int, d int, e int, f int, PRIMARY KEY ((a, b), c, d, e))");
executeSchemaChange("CREATE INDEX ON %s.partition_with_indices (c)");
executeSchemaChange("CREATE INDEX ON %s.partition_with_indices (f)");
@ -133,16 +135,44 @@ public class SingleColumnRelationTest
checkRow(0, results, 0, 0, 1, 1, 1, 5);
}
@Test(expected=InvalidRequestException.class)
public void testMissingPartitionComponentAndFileringOnTheSecondClusteringColumnWithoutAllowFiltering() throws Throwable
@Test
public void testSliceRestrictionOnPartitionKey() throws Throwable
{
execute("SELECT * FROM %s.partition_with_indices WHERE d >= 1 AND f = 5");
assertInvalidMessage("Only EQ and IN relation are supported on the partition key (unless you use the token() function)",
"SELECT * FROM %s.single_partition WHERE a >= 1 and a < 4");
}
@Test(expected=InvalidRequestException.class)
@Test
public void testMulticolumnSliceRestrictionOnPartitionKey() throws Throwable
{
assertInvalidMessage("Multi-column relations can only be applied to clustering columns: a",
"SELECT * FROM %s.single_partition WHERE (a) >= (1) and (a) < (4)");
assertInvalidMessage("Multi-column relations can only be applied to clustering columns: a",
"SELECT * FROM %s.compound_partition WHERE (a, b) >= (1, 1) and (a, b) < (4, 1)");
assertInvalidMessage("Multi-column relations can only be applied to clustering columns: a",
"SELECT * FROM %s.compound_partition WHERE a >= 1 and (a, b) < (4, 1)");
assertInvalidMessage("Multi-column relations can only be applied to clustering columns: a",
"SELECT * FROM %s.compound_partition WHERE b >= 1 and (a, b) < (4, 1)");
assertInvalidMessage("Multi-column relations can only be applied to clustering columns: a",
"SELECT * FROM %s.compound_partition WHERE (a, b) >= (1, 1) and (b) < (4)");
assertInvalidMessage("Multi-column relations can only be applied to clustering columns: b",
"SELECT * FROM %s.compound_partition WHERE (b) < (4) and (a, b) >= (1, 1)");
assertInvalidMessage("Multi-column relations can only be applied to clustering columns: a",
"SELECT * FROM %s.compound_partition WHERE (a, b) >= (1, 1) and a = 1");
}
@Test
public void testMissingPartitionComponentAndFileringOnTheSecondClusteringColumnWithoutAllowFiltering() throws Throwable
{
assertInvalidMessage("Cannot execute this query as it might involve data filtering and thus may have unpredictable performance. If you want to execute this query despite the performance unpredictability, use ALLOW FILTERING",
"SELECT * FROM %s.partition_with_indices WHERE d >= 1 AND f = 5");
}
@Test
public void testMissingPartitionComponentWithSliceRestrictionOnIndexedColumn() throws Throwable
{
execute("SELECT * FROM %s.partition_with_indices WHERE a = 0 AND c >= 1 ALLOW FILTERING");
assertInvalidMessage("Partition key part b must be restricted since preceding part is",
"SELECT * FROM %s.partition_with_indices WHERE a = 0 AND c >= 1 ALLOW FILTERING");
}
private static void checkRow(int rowIndex, UntypedResultSet results, Integer... expectedValues)
@ -158,4 +188,17 @@ public class SingleColumnRelationTest
(long) expected, actual);
}
}
private static void assertInvalidMessage(String expectedMsg, String query) throws Throwable
{
try
{
execute(query);
Assert.fail("The statement should trigger an InvalidRequestException but did not");
}
catch (InvalidRequestException e)
{
assertEquals("The error message is not the expected one.",expectedMsg, e.getMessage());
}
}
}