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

Tyler Hobbs commented on CASSANDRA-12127:
-----------------------------------------

The patches look good, overall.  Thank you for putting a lot of effort into 
improving the tests.

I just have a few review comments:
* In the 2.1/2.2 patches in {{SelectStatement.makeExclusiveBound()}}, why are 
we using {{.get(0}} for the bound? It seems like we need to use the full set of 
values to make a proper exclusive bound.  (If this actually is a problem and 
our tests didn't catch it, we should add a test case for this.)
* In {{Scubber}}, it would be good to add a few comments around {{tryAppend()}} 
and {{saveOutOfOrderRow()}} that clarify which cells are kept and which are 
saved elsewhere.  (It looks like the behavior is to keep everything before the 
first out of order cell, and then save the remaining cells elsewhere.)  The 
logged message could also be improved to provide more details about this.
* In {{collectOutOfOrderCells()}}, there are extra curly braces for single-line 
while-loop.
* In the 2.1/2.2 tests that loop through {{flush = {true, false})), shouldn't 
{{flush}} be set to {{false}} in the first loop? Right now it's flushing and 
then reading from sstables twice.

> Queries with empty ByteBuffer values in clustering column restrictions fail 
> for non-composite compact tables
> ------------------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-12127
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12127
>             Project: Cassandra
>          Issue Type: Bug
>          Components: CQL
>            Reporter: Benjamin Lerer
>            Assignee: Benjamin Lerer
>             Fix For: 2.1.x, 2.2.x, 3.0.x, 3.x
>
>         Attachments: 12127.txt
>
>
> For the following table:
> {code}
> CREATE TABLE myTable (pk int,
>                       c blob,
>                       value int,
>                       PRIMARY KEY (pk, c)) WITH COMPACT STORAGE;
> INSERT INTO myTable (pk, c, value) VALUES (1, textAsBlob('1'), 1);
> INSERT INTO myTable (pk, c, value) VALUES (1, textAsBlob('2'), 2);
> {code}
> The query: {{SELECT * FROM myTable WHERE pk = 1 AND c > textAsBlob('');}}
> Will result in the following Exception:
> {code}
> java.lang.ClassCastException: 
> org.apache.cassandra.db.composites.Composites$EmptyComposite cannot be cast 
> to org.apache.cassandra.db.composites.CellName
>       at 
> org.apache.cassandra.db.composites.AbstractCellNameType.cellFromByteBuffer(AbstractCellNameType.java:188)
>       at 
> org.apache.cassandra.db.composites.AbstractSimpleCellNameType.makeCellName(AbstractSimpleCellNameType.java:125)
>       at 
> org.apache.cassandra.db.composites.AbstractCellNameType.makeCellName(AbstractCellNameType.java:254)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.makeExclusiveSliceBound(SelectStatement.java:1206)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.applySliceRestriction(SelectStatement.java:1214)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.processColumnFamily(SelectStatement.java:1292)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.process(SelectStatement.java:1259)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.processResults(SelectStatement.java:299)
>       [...]
> {code}
> The query: {{SELECT * FROM myTable WHERE pk = 1 AND c < textAsBlob('');}}
> Will return 2 rows instead of 0.
> The query: {{SELECT * FROM myTable WHERE pk = 1 AND c = textAsBlob('');}}
> {code}
> java.lang.AssertionError
>       at 
> org.apache.cassandra.db.composites.SimpleDenseCellNameType.create(SimpleDenseCellNameType.java:60)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.addSelectedColumns(SelectStatement.java:853)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.getRequestedColumns(SelectStatement.java:846)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.makeFilter(SelectStatement.java:583)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.getSliceCommands(SelectStatement.java:383)
>       at 
> org.apache.cassandra.cql3.statements.SelectStatement.getPageableCommand(SelectStatement.java:253)
>       [...]
> {code}
> I checked 2.0 and {{SELECT * FROM myTable  WHERE pk = 1 AND c > 
> textAsBlob('');}} works properly but {{SELECT * FROM myTable WHERE pk = 1 AND 
> c < textAsBlob('');}} return the same wrong results than in 2.1.
> The {{SELECT * FROM myTable WHERE pk = 1 AND c = textAsBlob('');}} is 
> rejected if a clear error message: {{Invalid empty value for clustering 
> column of COMPACT TABLE}}.
> As it is not possible to insert an empty ByteBuffer value within the 
> clustering column of a non-composite compact tables those queries do not
> have a lot of meaning. {{SELECT * FROM myTable WHERE pk = 1 AND c < 
> textAsBlob('');}} and {{SELECT * FROM myTable WHERE pk = 1 AND c = 
> textAsBlob('');}} will return nothing
> and {{SELECT * FROM myTable WHERE pk = 1 AND c > textAsBlob('');}} will 
> return the entire partition (pk = 1).
> In my opinion those queries should probably all be rejected as it seems that 
> the fact that {{SELECT * FROM myTable  WHERE pk = 1 AND c > textAsBlob('');}} 
> was accepted in {{2.0}} was due to a bug.
> I am of course open to discussion.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to