mirror of https://github.com/apache/cassandra
Merge branch 'cassandra-5.0' into trunk
* cassandra-5.0: Fix broken indexing tests when using SAI - This fixes a number of broken tests when the default index is set to SAI - Composite partition indexes were being filtered prior to row filtering in the index searcher resulting in incorrect results - Static and non-static index intersection was failing because static primary keys were not comparing correctly against non-static primary keys
This commit is contained in:
commit
85b94c561d
|
|
@ -4,6 +4,7 @@
|
|||
* Add the ability to disable bulk loading of SSTables (CASSANDRA-18781)
|
||||
* Clean up obsolete functions and simplify cql_version handling in cqlsh (CASSANDRA-18787)
|
||||
Merged from 5.0:
|
||||
* SAI fixes for composite partitions, and static and non-static rows intersections (CASSANDRA-19034)
|
||||
* Improve SAI IndexContext handling of indexed and non-indexed columns in queries (CASSANDRA-18166)
|
||||
* Fixed bug where UnifiedCompactionTask constructor was calling the wrong base constructor of CompactionTask (CASSANDRA-18757)
|
||||
* Fix SAI unindexed contexts not considering CONTAINS KEY (CASSANDRA-19040)
|
||||
|
|
|
|||
|
|
@ -388,14 +388,11 @@ public class StorageAttachedIndexSearcher implements Index.Searcher
|
|||
{
|
||||
Row staticRow = partition.staticRow();
|
||||
|
||||
// We want to short-circuit the filtering of the whole partition if the static row
|
||||
// satisfies the filter. If that is the case we just need to return the whole partition.
|
||||
queryContext.rowsFiltered++;
|
||||
if (tree.isSatisfiedBy(partition.partitionKey(), staticRow, staticRow))
|
||||
return partition;
|
||||
|
||||
List<Unfiltered> clusters = new ArrayList<>();
|
||||
|
||||
// We need to filter the partition rows before filtering on the static row. If this is done in the other
|
||||
// order then we get incorrect results if we are filtering on a partition key index on a table with a
|
||||
// composite partition key.
|
||||
while (partition.hasNext())
|
||||
{
|
||||
Unfiltered row = partition.next();
|
||||
|
|
@ -407,6 +404,15 @@ public class StorageAttachedIndexSearcher implements Index.Searcher
|
|||
}
|
||||
}
|
||||
|
||||
if (clusters.isEmpty())
|
||||
{
|
||||
queryContext.rowsFiltered++;
|
||||
if (tree.isSatisfiedBy(key.partitionKey(), staticRow, staticRow))
|
||||
{
|
||||
clusters.add(staticRow);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* If {@code clusters} is empty, which means either all clustering row and static row pairs failed,
|
||||
* or static row and static row pair failed. In both cases, we should not return any partition.
|
||||
|
|
|
|||
|
|
@ -309,9 +309,10 @@ public interface PrimaryKey extends Comparable<PrimaryKey>, ByteComparable
|
|||
int cmp = super.compareTo(o);
|
||||
if (cmp != 0 || o.kind() == Kind.TOKEN || o.kind() == Kind.SKINNY)
|
||||
return cmp;
|
||||
// The static clustering comes first in the sort order of if the other key has static clustering we
|
||||
// are equals otherwise we are less than the other
|
||||
return o.kind() == Kind.STATIC ? 0 : -1;
|
||||
// At this point the other key is in the same partition as this static key so is equal to it. This
|
||||
// has to be the case because otherwise, intersections between static column indexes and ordinary
|
||||
// indexes will fail.
|
||||
return 0;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -368,9 +369,10 @@ public interface PrimaryKey extends Comparable<PrimaryKey>, ByteComparable
|
|||
int cmp = super.compareTo(o);
|
||||
if (cmp != 0 || o.kind() == Kind.TOKEN || o.kind() == Kind.SKINNY)
|
||||
return cmp;
|
||||
// At this point we will be greater than other if it is static
|
||||
// At this point this key is in the same partition as the other key so if the other key is a static
|
||||
// key then it must be equal to it. See comment in the compareTo for static keys above.
|
||||
if (o.kind() == Kind.STATIC)
|
||||
return 1;
|
||||
return 0;
|
||||
return clusteringComparator.compare(clustering(), o.clustering());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,88 +17,92 @@
|
|||
*/
|
||||
package org.apache.cassandra.index.sai.cql;
|
||||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
import org.apache.cassandra.cql3.restrictions.StatementRestrictions;
|
||||
import org.apache.cassandra.index.sai.SAITester;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
|
||||
public class CompositePartitionKeyIndexTest extends SAITester
|
||||
{
|
||||
@Before
|
||||
public void createTableAndIndex()
|
||||
@Test
|
||||
public void testCompositePartitionIndex() throws Throwable
|
||||
{
|
||||
createTable("CREATE TABLE %s (pk1 int, pk2 text, val int, PRIMARY KEY((pk1, pk2)))");
|
||||
createIndex("CREATE INDEX ON %s(pk1) USING 'sai'");
|
||||
createIndex("CREATE INDEX ON %s(pk2) USING 'sai'");
|
||||
|
||||
disableCompaction();
|
||||
}
|
||||
|
||||
private void insertData1()
|
||||
{
|
||||
execute("INSERT INTO %s (pk1, pk2, val) VALUES (1, '1', 1)");
|
||||
execute("INSERT INTO %s (pk1, pk2, val) VALUES (2, '2', 2)");
|
||||
execute("INSERT INTO %s (pk1, pk2, val) VALUES (3, '3', 3)");
|
||||
}
|
||||
|
||||
private void insertData2()
|
||||
{
|
||||
execute("INSERT INTO %s (pk1, pk2, val) VALUES (4, '4', 4)");
|
||||
execute("INSERT INTO %s (pk1, pk2, val) VALUES (5, '5', 5)");
|
||||
execute("INSERT INTO %s (pk1, pk2, val) VALUES (6, '6', 6)");
|
||||
|
||||
beforeAndAfterFlush(() -> {
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 = 2"),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 > 1"),
|
||||
expectedRow(2),
|
||||
expectedRow(3),
|
||||
expectedRow(4),
|
||||
expectedRow(5),
|
||||
expectedRow(6));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 >= 3"),
|
||||
expectedRow(3),
|
||||
expectedRow(4),
|
||||
expectedRow(5),
|
||||
expectedRow(6));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 < 3"),
|
||||
expectedRow(1),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 <= 3"),
|
||||
expectedRow(1),
|
||||
expectedRow(2),
|
||||
expectedRow(3));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk2 = '2'"),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 > 1 AND pk2 = '2'"),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 = -1 AND pk2 = '2'"));
|
||||
|
||||
assertInvalidMessage(StatementRestrictions.REQUIRES_ALLOW_FILTERING_MESSAGE, "SELECT * FROM %s WHERE pk1 = -1 AND val = 2");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
public void queryFromMemtable()
|
||||
public void testFilterWithIndexForContains() throws Throwable
|
||||
{
|
||||
insertData1();
|
||||
insertData2();
|
||||
runQueries();
|
||||
createTable("CREATE TABLE %s (k1 int, k2 int, v set<int>, PRIMARY KEY ((k1, k2)))");
|
||||
createIndex("CREATE INDEX ON %s(k2) USING 'sai'");
|
||||
|
||||
execute("INSERT INTO %s (k1, k2, v) VALUES (?, ?, ?)", 0, 0, set(1, 2, 3));
|
||||
execute("INSERT INTO %s (k1, k2, v) VALUES (?, ?, ?)", 0, 1, set(2, 3, 4));
|
||||
execute("INSERT INTO %s (k1, k2, v) VALUES (?, ?, ?)", 1, 0, set(3, 4, 5));
|
||||
execute("INSERT INTO %s (k1, k2, v) VALUES (?, ?, ?)", 1, 1, set(4, 5, 6));
|
||||
|
||||
beforeAndAfterFlush(() -> {
|
||||
assertRows(execute("SELECT * FROM %s WHERE k2 = ?", 1),
|
||||
row(0, 1, set(2, 3, 4)),
|
||||
row(1, 1, set(4, 5, 6))
|
||||
);
|
||||
|
||||
assertRows(execute("SELECT * FROM %s WHERE k2 = ? AND v CONTAINS ? ALLOW FILTERING", 1, 6),
|
||||
row(1, 1, set(4, 5, 6))
|
||||
);
|
||||
|
||||
assertEmpty(execute("SELECT * FROM %s WHERE k2 = ? AND v CONTAINS ? ALLOW FILTERING", 1, 7));
|
||||
});
|
||||
}
|
||||
|
||||
private Object[] expectedRow(int index) {
|
||||
private Object[] expectedRow(int index)
|
||||
{
|
||||
return row(index, Integer.toString(index), index);
|
||||
}
|
||||
|
||||
private void runQueries()
|
||||
{
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 = 2"),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 > 1"),
|
||||
expectedRow(2),
|
||||
expectedRow(3),
|
||||
expectedRow(4),
|
||||
expectedRow(5),
|
||||
expectedRow(6));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 >= 3"),
|
||||
expectedRow(3),
|
||||
expectedRow(4),
|
||||
expectedRow(5),
|
||||
expectedRow(6));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 < 3"),
|
||||
expectedRow(1),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 <= 3"),
|
||||
expectedRow(1),
|
||||
expectedRow(2),
|
||||
expectedRow(3));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk2 = '2'"),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 > 1 AND pk2 = '2'"),
|
||||
expectedRow(2));
|
||||
|
||||
assertRowsIgnoringOrder(execute("SELECT * FROM %s WHERE pk1 = -1 AND pk2 = '2'"));
|
||||
|
||||
assertThatThrownBy(()->execute("SELECT * FROM %s WHERE pk1 = -1 AND val = 2"))
|
||||
.hasMessageContaining("use ALLOW FILTERING");
|
||||
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,55 @@
|
|||
/*
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.apache.cassandra.index.sai.cql;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import org.apache.cassandra.index.sai.SAITester;
|
||||
|
||||
public class StaticColumnIndexTest extends SAITester
|
||||
{
|
||||
@Test
|
||||
public void staticIndexReturnsAllRowsInPartition() throws Throwable
|
||||
{
|
||||
createTable("CREATE TABLE %s (pk int, ck int, val1 int static, val2 int, PRIMARY KEY(pk, ck))");
|
||||
createIndex("CREATE INDEX ON %s(val1) USING 'sai'");
|
||||
|
||||
execute("INSERT INTO %s(pk, ck, val1, val2) VALUES(?, ?, ?, ?)", 1, 1, 2, 1);
|
||||
execute("INSERT INTO %s(pk, ck, val2) VALUES(?, ?, ?)", 1, 2, 2);
|
||||
execute("INSERT INTO %s(pk, ck, val2) VALUES(?, ?, ?)", 1, 3, 3);
|
||||
|
||||
beforeAndAfterFlush(() -> assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 2"),
|
||||
row(1, 1, 2, 1), row(1, 2, 2, 2), row(1, 3, 2, 3)));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void staticIndexAndNonStaticIndex() throws Throwable
|
||||
{
|
||||
createTable("CREATE TABLE %s (pk int, ck int, val1 int static, val2 int, PRIMARY KEY(pk, ck))");
|
||||
createIndex("CREATE INDEX ON %s(val1) USING 'sai'");
|
||||
createIndex("CREATE INDEX ON %s(val2) USING 'sai'");
|
||||
|
||||
execute("INSERT INTO %s(pk, ck, val1, val2) VALUES(?, ?, ?, ?)", 1, 1, 20, 1000);
|
||||
execute("INSERT INTO %s(pk, ck, val2) VALUES(?, ?, ?)", 1, 2, 2000);
|
||||
execute("INSERT INTO %s(pk, ck, val1, val2) VALUES(?, ?, ?, ?)", 2, 1, 40, 2000);
|
||||
|
||||
beforeAndAfterFlush(() -> assertRows(execute("SELECT pk, ck, val1, val2 FROM %s WHERE val1 = 20 AND val2 = 2000 ALLOW FILTERING"),
|
||||
row(1, 2, 20, 2000)));
|
||||
}
|
||||
}
|
||||
|
|
@ -83,7 +83,8 @@ public class PrimaryKeyTest extends AbstractPrimaryKeyTester
|
|||
public void simplePartitonStaticAndSingleClusteringAscTest()
|
||||
{
|
||||
PrimaryKey.Factory factory = new PrimaryKey.Factory(Murmur3Partitioner.instance, simplePartitionStaticAndSingleClusteringAsc.comparator);
|
||||
int rows = nextInt(10, 100);
|
||||
// int rows = nextInt(10, 100);
|
||||
int rows = 10;
|
||||
PrimaryKey[] keys = new PrimaryKey[rows];
|
||||
int partition = 0;
|
||||
int clustering = 0;
|
||||
|
|
@ -372,9 +373,14 @@ public class PrimaryKeyTest extends AbstractPrimaryKeyTester
|
|||
assertCompareToAndEquals(key, key, 0);
|
||||
assertCompareToAndEquals(tokenOnlyKey, tokenOnlyKey, 0);
|
||||
|
||||
// StaticPrimaryKey is a special case. All other keys in the partition are equal to it
|
||||
boolean staticComparison = key.kind() == PrimaryKey.Kind.STATIC;
|
||||
boolean inPartition = staticComparison;
|
||||
for (int comparisonIndex = index + 1; comparisonIndex < keys.length; comparisonIndex++)
|
||||
{
|
||||
assertCompareToAndEquals(key, keys[comparisonIndex], -1);
|
||||
if (staticComparison && keys[comparisonIndex].kind() == PrimaryKey.Kind.STATIC)
|
||||
inPartition = false;
|
||||
assertCompareToAndEquals(key, keys[comparisonIndex], inPartition ? 0 : -1);
|
||||
assertCompareToAndEquals(tokenOnlyKey, keys[comparisonIndex], tokenOnlyKey.token().equals(keys[comparisonIndex].token()) ? 0 : -1);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue