adelapena commented on code in PR #2673:
URL: https://github.com/apache/cassandra/pull/2673#discussion_r1331686334
##########
src/java/org/apache/cassandra/cql3/statements/SelectStatement.java:
##########
@@ -483,7 +486,7 @@ else if (restrictions.keyIsInRelation())
// Please note that the isExhausted state of the pager only gets
updated when we've closed the page, so this
// shouldn't be moved inside the 'try' above.
- if (!pager.isExhausted())
+ if (!pager.isExhausted() && !pager.pager.isTopK())
Review Comment:
While testing how paging works, I have randomly found that a range filter on
the partition key can throw an exception:
```java
@Test
public void testKeyRestrictions()
{
createTable("CREATE TABLE %s (k int PRIMARY KEY, v vector<float, 1>)");
createIndex("CREATE CUSTOM INDEX ON %s(v) USING 'StorageAttachedIndex'
WITH OPTIONS = {'similarity_function' : 'euclidean'}");
execute("INSERT INTO %s (k, v) VALUES (1, [1])");
assertRows(execute("SELECT k, v FROM %s WHERE k > 0 LIMIT 4 ALLOW
FILTERING"), row(1, vector(1f)));
assertRows(execute("SELECT k, v FROM %s WHERE k = 1 ORDER BY v ANN OF
[0] LIMIT 4 ALLOW FILTERING"), row(1, vector(1f)));
assertRows(execute("SELECT k, v FROM %s WHERE k > 0 ORDER BY v ANN OF
[0] LIMIT 4 ALLOW FILTERING"), row(1, vector(1f))); // exception!!
}
```
The error for in-memory is:
```
java.lang.AssertionError: Attempt to use memtable index manager on
non-indexed context
at
org.apache.cassandra.index.sai.IndexContext.getMemtableIndexManager(IndexContext.java:231)
at
org.apache.cassandra.index.sai.plan.vector.VectorQueryController.lambda$getIndexQueryResults$1(VectorQueryController.java:89)
at
java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:271)
at
java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
at
java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
at
java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
at
java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913)
at
java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at
java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
at
org.apache.cassandra.index.sai.plan.vector.VectorQueryController.getIndexQueryResults(VectorQueryController.java:90)
at
org.apache.cassandra.index.sai.plan.Operation$AndNode.rangeIterator(Operation.java:305)
at
org.apache.cassandra.index.sai.plan.Operation.buildIterator(Operation.java:178)
at
org.apache.cassandra.index.sai.plan.StorageAttachedIndexSearcher$ResultRetriever.<init>(StorageAttachedIndexSearcher.java:152)
at
org.apache.cassandra.index.sai.plan.StorageAttachedIndexSearcher.lambda$search$0(StorageAttachedIndexSearcher.java:111)
at
org.apache.cassandra.index.sai.plan.StorageAttachedIndexSearcher.search(StorageAttachedIndexSearcher.java:118)
at
org.apache.cassandra.db.ReadCommand.executeLocally(ReadCommand.java:431)
at
org.apache.cassandra.db.AbstractReadQuery.executeInternal(AbstractReadQuery.java:65)
at
org.apache.cassandra.db.ReadCommand.executeInternal(ReadCommand.java:88)
at
org.apache.cassandra.cql3.statements.SelectStatement.executeInternal(SelectStatement.java:543)
at
org.apache.cassandra.cql3.statements.SelectStatement.executeLocally(SelectStatement.java:515)
at
org.apache.cassandra.cql3.statements.SelectStatement.executeLocally(SelectStatement.java:108)
at
org.apache.cassandra.cql3.QueryProcessor.executeInternal(QueryProcessor.java:445)
at
org.apache.cassandra.cql3.CQLTester.executeFormattedQuery(CQLTester.java:1595)
at org.apache.cassandra.cql3.CQLTester.execute(CQLTester.java:1574)
at
org.apache.cassandra.cql3.validation.operations.CQLVectorTest.testKeyRestrictions(CQLVectorTest.java:69)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at
org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
at
com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
at
com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
at
com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
at
com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:232)
at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:55)
```
Whereas if we flush the table it looks like this:
```
java.lang.UnsupportedOperationException
at
org.apache.cassandra.io.filesystem.ListenableFileSystem$ListenablePath.toFile(ListenableFileSystem.java:661)
at org.apache.cassandra.io.util.File.toJavaIOFile(File.java:724)
at
org.apache.cassandra.index.sai.disk.format.IndexDescriptor.createComponentOnDisk(IndexDescriptor.java:185)
at
org.apache.cassandra.index.sai.disk.v1.SSTableComponentsWriter.complete(SSTableComponentsWriter.java:106)
at
org.apache.cassandra.index.sai.disk.StorageAttachedIndexWriter.complete(StorageAttachedIndexWriter.java:171)
at
com.google.common.collect.ImmutableList.forEach(ImmutableList.java:422)
at
org.apache.cassandra.io.sstable.format.SSTableWriter.commit(SSTableWriter.java:235)
at
org.apache.cassandra.io.sstable.SimpleSSTableMultiWriter.commit(SimpleSSTableMultiWriter.java:90)
at
org.apache.cassandra.db.ColumnFamilyStore$Flush.flushMemtable(ColumnFamilyStore.java:1321)
at
org.apache.cassandra.db.ColumnFamilyStore$Flush.run(ColumnFamilyStore.java:1219)
at
org.apache.cassandra.concurrent.ExecutionFailure$1.run(ExecutionFailure.java:133)
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at
io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.base/java.lang.Thread.run(Thread.java:834)
```
##########
src/java/org/apache/cassandra/cql3/statements/SelectStatement.java:
##########
@@ -288,7 +291,7 @@ public ResultMessage.Rows execute(QueryState state,
QueryOptions options, long q
query.trackWarnings();
ResultMessage.Rows rows;
- if (aggregationSpec == null && (pageSize <= 0 ||
(query.limits().count() <= pageSize)))
+ if (aggregationSpec == null && (pageSize <= 0 ||
(query.limits().count() <= pageSize) || query.isTopK()))
Review Comment:
Also, maybe we should throw a client warning if the query's page size is
lower than the limit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]