You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "jasonstack (via GitHub)" <gi...@apache.org> on 2023/06/14 01:01:03 UTC
[GitHub] [cassandra] jasonstack opened a new pull request, #2416: fix Segment#intersects to compare bound instead of token
jasonstack opened a new pull request, #2416:
URL: https://github.com/apache/cassandra/pull/2416
(no comment)
--
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
[GitHub] [cassandra] maedhroz commented on a diff in pull request #2416: fix Segment#intersects to compare bound instead of token
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #2416:
URL: https://github.com/apache/cassandra/pull/2416#discussion_r1229939427
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/Segment.java:
##########
@@ -87,14 +87,15 @@ public boolean intersects(AbstractBounds<PartitionPosition> keyRange)
if (keyRange instanceof Range && ((Range<?>)keyRange).isWrapAround())
return keyRange.contains(minKeyBound) || keyRange.contains(maxKeyBound);
- int cmp = keyRange.right.getToken().compareTo(minKey);
+ int cmp = keyRange.right.compareTo(minKeyBound);
// if right is minimum, it means right is the max token and bigger than maxKey.
// if right bound is less than minKey, no intersection
if (!keyRange.right.isMinimum() && (!keyRange.inclusiveRight() && cmp == 0 || cmp < 0))
return false;
+ cmp = keyRange.left.compareTo(maxKeyBound);
// if left bound is bigger than maxKey, no intersection
- return keyRange.isStartInclusive() || keyRange.left.getToken().compareTo(maxKey) < 0;
+ return (keyRange.isStartInclusive() || cmp != 0) && cmp <= 0;
Review Comment:
With the changes in this method, we no longer need the `minKey` or `maxKey` fields in this class, right?
CC @mike-tr-adamson
--
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
[GitHub] [cassandra] maedhroz commented on a diff in pull request #2416: fix Segment#intersects to compare bound instead of token
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #2416:
URL: https://github.com/apache/cassandra/pull/2416#discussion_r1229939803
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/Segment.java:
##########
@@ -87,14 +87,15 @@ public boolean intersects(AbstractBounds<PartitionPosition> keyRange)
if (keyRange instanceof Range && ((Range<?>)keyRange).isWrapAround())
return keyRange.contains(minKeyBound) || keyRange.contains(maxKeyBound);
- int cmp = keyRange.right.getToken().compareTo(minKey);
+ int cmp = keyRange.right.compareTo(minKeyBound);
// if right is minimum, it means right is the max token and bigger than maxKey.
// if right bound is less than minKey, no intersection
if (!keyRange.right.isMinimum() && (!keyRange.inclusiveRight() && cmp == 0 || cmp < 0))
return false;
+ cmp = keyRange.left.compareTo(maxKeyBound);
// if left bound is bigger than maxKey, no intersection
Review Comment:
```suggestion
// if left bound is bigger than maxKeyBound, no intersection
```
--
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
[GitHub] [cassandra] jasonstack commented on a diff in pull request #2416: fix Segment#intersects to compare bound instead of token
Posted by "jasonstack (via GitHub)" <gi...@apache.org>.
jasonstack commented on code in PR #2416:
URL: https://github.com/apache/cassandra/pull/2416#discussion_r1230294758
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/Segment.java:
##########
@@ -87,14 +87,15 @@ public boolean intersects(AbstractBounds<PartitionPosition> keyRange)
if (keyRange instanceof Range && ((Range<?>)keyRange).isWrapAround())
return keyRange.contains(minKeyBound) || keyRange.contains(maxKeyBound);
- int cmp = keyRange.right.getToken().compareTo(minKey);
+ int cmp = keyRange.right.compareTo(minKeyBound);
// if right is minimum, it means right is the max token and bigger than maxKey.
// if right bound is less than minKey, no intersection
if (!keyRange.right.isMinimum() && (!keyRange.inclusiveRight() && cmp == 0 || cmp < 0))
return false;
+ cmp = keyRange.left.compareTo(maxKeyBound);
// if left bound is bigger than maxKey, no intersection
- return keyRange.isStartInclusive() || keyRange.left.getToken().compareTo(maxKey) < 0;
+ return (keyRange.isStartInclusive() || cmp != 0) && cmp <= 0;
Review Comment:
removed `minKey` and `maxKey`
--
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
[GitHub] [cassandra] maedhroz commented on a diff in pull request #2416: fix Segment#intersects to compare bound instead of token
Posted by "maedhroz (via GitHub)" <gi...@apache.org>.
maedhroz commented on code in PR #2416:
URL: https://github.com/apache/cassandra/pull/2416#discussion_r1229938530
##########
src/java/org/apache/cassandra/index/sai/disk/v1/segment/Segment.java:
##########
@@ -87,14 +87,15 @@ public boolean intersects(AbstractBounds<PartitionPosition> keyRange)
if (keyRange instanceof Range && ((Range<?>)keyRange).isWrapAround())
return keyRange.contains(minKeyBound) || keyRange.contains(maxKeyBound);
- int cmp = keyRange.right.getToken().compareTo(minKey);
+ int cmp = keyRange.right.compareTo(minKeyBound);
// if right is minimum, it means right is the max token and bigger than maxKey.
// if right bound is less than minKey, no intersection
Review Comment:
```suggestion
// if right bound is less than minKeyBound, no intersection
```
--
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
[GitHub] [cassandra] jasonstack merged pull request #2416: fix Segment#intersects to compare bound instead of token
Posted by "jasonstack (via GitHub)" <gi...@apache.org>.
jasonstack merged PR #2416:
URL: https://github.com/apache/cassandra/pull/2416
--
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