You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Aleksey Yeschenko (JIRA)" <ji...@apache.org> on 2018/10/29 14:06:03 UTC
[jira] [Comment Edited] (CASSANDRA-14849) some empty/invalid bounds
aren't caught by SelectStatement
[ https://issues.apache.org/jira/browse/CASSANDRA-14849?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16667251#comment-16667251 ]
Aleksey Yeschenko edited comment on CASSANDRA-14849 at 10/29/18 2:05 PM:
-------------------------------------------------------------------------
I think the patch is correct, but I have a few issues with it.
1. We are inlining a copy of {{ClusteringComparator.compare()}}, ish, into {{Slice.isEmpty()}}. If the former changes somehow, there is a risk of forgetting to apply the difference to the inlined version.
2. There is duplication of work. After making the regular {{compare()}} call, we are going through the motions again in the common case.
3. There are different returns with some nesting involved that makes it a bit trickier to follow than necessary.
I *think* essentially we are just lacking a {{cmp == 0 && one of the bounds is exclusive}} condition, and the whole method can be simplified quite a bit (relatively). Pushed an illustration/review branch with those issues handled [here|https://github.com/iamaleksey/cassandra/commits/14849-review].
was (Author: iamaleksey):
I think the patch is correct, but I have a few issues with it.
1. We are inlining a copy of {{ClusteringComparator.compare()}}, ish, into {{Slices.isEmpty()}}. If the former changes somehow, there is a risk of forgetting to apply the difference to the inlined version.
2. There is duplication of work. After making the regular {{compare()}} call, we are going through the motions again in the common case.
3. There are different returns with some nesting involved that makes it a bit trickier to follow than necessary.
I *think* essentially we are just lacking a {{cmp == 0 && one of the bounds is exclusive}} condition, and the whole method can be simplified quite a bit (relatively). Pushed an illustration/review branch with those issues handled [here|https://github.com/iamaleksey/cassandra/commits/14849-review].
> some empty/invalid bounds aren't caught by SelectStatement
> ----------------------------------------------------------
>
> Key: CASSANDRA-14849
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14849
> Project: Cassandra
> Issue Type: Bug
> Reporter: Blake Eggleston
> Assignee: Blake Eggleston
> Priority: Major
> Fix For: 4.0
>
>
> Nonsensical clustering bounds like "c >= 100 AND c < 100" aren't converted to Slices.NONE like they should be. Although this seems to be completely benign, it is technically incorrect and complicates some testing since it can cause memtables and sstables to return different results for the same data for these bounds in some cases.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org