[
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-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
