You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "blerer (via GitHub)" <gi...@apache.org> on 2024/08/29 14:57:01 UTC
[PR] Fix BETWEEN filtering for reversed clustering columns [cassandra]
blerer opened a new pull request, #3507:
URL: https://github.com/apache/cassandra/pull/3507
Thanks for sending a pull request! Here are some tips if you're new here:
* Ensure you have added or run the [appropriate tests](https://cassandra.apache.org/_/development/testing.html) for your PR.
* Be sure to keep the PR description updated to reflect all changes.
* Write your PR title to summarize what this PR proposes.
* If possible, provide a concise example to reproduce the issue for a faster review.
* Read our [contributor guidelines](https://cassandra.apache.org/_/development/index.html)
* If you're making a documentation change, see our [guide to documentation contribution](https://cassandra.apache.org/_/development/documentation.html)
Commit messages should follow the following format:
```
<One sentence description, usually Jira title or CHANGES.txt summary>
<Optional lengthier description (context on patch)>
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
Co-authored-by: Name1 <email1>
Co-authored-by: Name2 <email2>
```
The [Cassandra Jira](https://issues.apache.org/jira/projects/CASSANDRA/issues/)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Fix BETWEEN filtering for reversed clustering columns [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3507:
URL: https://github.com/apache/cassandra/pull/3507#discussion_r1739457451
##########
src/java/org/apache/cassandra/cql3/restrictions/SimpleRestriction.java:
##########
@@ -327,8 +327,11 @@ public void addToRowFilter(RowFilter filter, IndexRegistry indexRegistry, QueryO
{
case SINGLE_COLUMN:
List<ByteBuffer> buffers = bindAndGet(options);
- if (operator == Operator.IN || operator == Operator.BETWEEN)
+ if (operator.kind() == Operator.Kind.TERNARY || operator.kind() == Operator.Kind.MULTI_VALUE)
{
+ // For BETWEEN we support like in SQL reversed bounds
+ if (operator.kind() == Operator.Kind.TERNARY)
Review Comment:
```suggestion
if (column.isClusteringColumn())
```
I was wondering if we can do something like this to avoid sorting when we don't have a clustering key column in the first place?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Fix BETWEEN filtering for reversed clustering columns [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz closed pull request #3507: Fix BETWEEN filtering for reversed clustering columns
URL: https://github.com/apache/cassandra/pull/3507
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Fix BETWEEN filtering for reversed clustering columns [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #3507:
URL: https://github.com/apache/cassandra/pull/3507#discussion_r1739457451
##########
src/java/org/apache/cassandra/cql3/restrictions/SimpleRestriction.java:
##########
@@ -327,8 +327,11 @@ public void addToRowFilter(RowFilter filter, IndexRegistry indexRegistry, QueryO
{
case SINGLE_COLUMN:
List<ByteBuffer> buffers = bindAndGet(options);
- if (operator == Operator.IN || operator == Operator.BETWEEN)
+ if (operator.kind() == Operator.Kind.TERNARY || operator.kind() == Operator.Kind.MULTI_VALUE)
{
+ // For BETWEEN we support like in SQL reversed bounds
+ if (operator.kind() == Operator.Kind.TERNARY)
Review Comment:
```suggestion
if (operator.kind() == Operator.Kind.TERNARY && column.isClusteringColumn())
```
I was wondering if we can do something like this to avoid sorting when we don't have a clustering key column in the first place?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Fix BETWEEN filtering for reversed clustering columns [cassandra]
Posted by "blerer (via GitHub)" <gi...@apache.org>.
blerer commented on code in PR #3507:
URL: https://github.com/apache/cassandra/pull/3507#discussion_r1741046462
##########
src/java/org/apache/cassandra/cql3/restrictions/SimpleRestriction.java:
##########
@@ -327,8 +327,11 @@ public void addToRowFilter(RowFilter filter, IndexRegistry indexRegistry, QueryO
{
case SINGLE_COLUMN:
List<ByteBuffer> buffers = bindAndGet(options);
- if (operator == Operator.IN || operator == Operator.BETWEEN)
+ if (operator.kind() == Operator.Kind.TERNARY || operator.kind() == Operator.Kind.MULTI_VALUE)
{
+ // For BETWEEN we support like in SQL reversed bounds
+ if (operator.kind() == Operator.Kind.TERNARY)
Review Comment:
The sort is always required because we support like in SQL between with reversed bounds. For example `BETWEEN 1 AND 2` and `BETWEEN 2 AND 1` are both valid and will return the same rows.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] Fix BETWEEN filtering for reversed clustering columns [cassandra]
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on PR #3507:
URL: https://github.com/apache/cassandra/pull/3507#issuecomment-2329868583
Committed as https://github.com/apache/cassandra/commit/c7e9048d4df9c0a5bb178c8388a13ad44f1cc0d2
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org