[ 
https://issues.apache.org/jira/browse/CASSANDRA-11223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16086730#comment-16086730
 ] 

Stefania edited comment on CASSANDRA-11223 at 7/14/17 2:11 AM:
---------------------------------------------------------------

bq. So, in practice changing GroupByPrefixReversed.count() has no effect.

Fine to leave it as is then, but let's add a comment explaining this, so that 
people don't find it confusing later on.

bq. The limit is only used for limiting the number of rows stored in the cache 
for each partition and we will only count the static row if the partition does 
not have any row.

Agreed.

bq. ColumnFamily.liveCQL3RowCount() is only used by 
ColumnFamilyStore::isFilterFullyCoveredBy to check if the whole partition is 
cached. That we count the static row or not, the answer will be correct. We 
could argue about ColumnFamily.liveCQL3RowCount() independently of its current 
use,

You are right in that {{wholePartitionCached}} will be correct regardless and 
we don't need to consider any other use cases that don't exist for 2.2. So it's 
fine as is, I was getting mixed up with 3.0.

I've checked the 3.11 patch in depth, and the trunk patch with the GH diff:

* I think we can remove {{selectsFullPartitions}} from {{MultiPartitionPager}}.
* The javadoc of {{ReadQuery.selectsFullPartition()}} still looks wrong, I 
think the return is the opposite of what the doc says. This applies to 3.0 as 
well.

There is a conflict with CASSANDRA-13482 for 3.0+ so we need to rebase and 
rerun CI. Afterwards, if CI is still OK, we should be able to commit.


was (Author: stefania):
bq. So, in practice changing GroupByPrefixReversed.count() has no effect.

Fine to leave it as is then, but let's add a comment explaining this, so that 
people don't find it confusing later on.

bq. The limit is only used for limiting the number of rows stored in the cache 
for each partition and we will only count the static row if the partition does 
not have any row.

Agreed.

bq. ColumnFamily.liveCQL3RowCount() is only used by 
ColumnFamilyStore::isFilterFullyCoveredBy to check if the whole partition is 
cached. That we count the static row or not, the answer will be correct. We 
could argue about ColumnFamily.liveCQL3RowCount() independently of its current 
use,

You are right in that {{wholePartitionCached}} will be correct regardless and 
we don't need to consider any other use cases that don't exist for 2.2. So it's 
fine as is, I was getting mixed up with 3.0.

I've checked the 3.11 patch in depth, and the trunk patch with the GH diff:

* I think we can remove {{selectsFullPartitions}} from {{MultiPartitionPager}}.
* The javadoc of {{ReadQuery.selectsFullPartition()}} looks still wrong, I 
think the return is the opposite of what the doc says. This applies to 3.0 as 
well.

There is a conflict with CASSANDRA-13482 for 3.0+ so we need to rebase and 
rerun CI. Afterwards, if CI is still OK, we should be able to commit.

> Queries with LIMIT filtering on clustering columns can return less rows than 
> expected
> -------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-11223
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-11223
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Local Write-Read Paths
>            Reporter: Benjamin Lerer
>            Assignee: Benjamin Lerer
>
> A query like {{SELECT * FROM %s WHERE b = 1 LIMIT 2 ALLOW FILTERING}} can 
> return less row than expected if the table has some static columns and some 
> of the partition have no rows matching b = 1.
> The problem can be reproduced with the following unit test:
> {code}
>     public void testFilteringOnClusteringColumnsWithLimitAndStaticColumns() 
> throws Throwable
>     {
>         createTable("CREATE TABLE %s (a int, b int, s int static, c int, 
> primary key (a, b))");
>         for (int i = 0; i < 3; i++)
>         {
>             execute("INSERT INTO %s (a, s) VALUES (?, ?)", i, i);
>                 for (int j = 0; j < 3; j++)
>                     if (!(i == 0 && j == 1))
>                         execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 
> i, j, i + j);
>         }
>         assertRows(execute("SELECT * FROM %s"),
>                    row(1, 0, 1, 1),
>                    row(1, 1, 1, 2),
>                    row(1, 2, 1, 3),
>                    row(0, 0, 0, 0),
>                    row(0, 2, 0, 2),
>                    row(2, 0, 2, 2),
>                    row(2, 1, 2, 3),
>                    row(2, 2, 2, 4));
>         assertRows(execute("SELECT * FROM %s WHERE b = 1 ALLOW FILTERING"),
>                    row(1, 1, 1, 2),
>                    row(2, 1, 2, 3));
>         assertRows(execute("SELECT * FROM %s WHERE b = 1 LIMIT 2 ALLOW 
> FILTERING"),
>                    row(1, 1, 1, 2),
>                    row(2, 1, 2, 3)); // <-------- FAIL It returns only one 
> row because the static row of partition 0 is counted and filtered out in 
> SELECT statement
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to