-
Notifications
You must be signed in to change notification settings - Fork 1k
PHOENIX-7524: Fix IndexOutOfBoundsException in queries with OFFSET when rows are exhausted #2307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -291,4 +291,33 @@ public static <T> Throwable getExceptionFromFailedFuture(Future<T> f) { | |
| } | ||
| return t; | ||
| } | ||
|
|
||
| /** | ||
| * Derives a safe row key for empty result sets based on scan or region boundaries. Used when | ||
| * constructing KeyValues for aggregate results or OFFSET responses when no actual data rows were | ||
| * scanned. | ||
| * @param scan The scan being executed | ||
| * @param region The region being scanned | ||
| * @return A valid row key derived from scan or region boundaries | ||
| */ | ||
| public static byte[] deriveRowKeyFromScanOrRegionBoundaries(Scan scan, Region region) { | ||
| byte[] startKey = | ||
| scan.getStartRow().length > 0 ? scan.getStartRow() : region.getRegionInfo().getStartKey(); | ||
| byte[] endKey = | ||
| scan.getStopRow().length > 0 ? scan.getStopRow() : region.getRegionInfo().getEndKey(); | ||
|
|
||
| byte[] rowKey = ByteUtil.getLargestPossibleRowKeyInRange(startKey, endKey); | ||
|
|
||
| if (rowKey == null) { | ||
| if (scan.includeStartRow()) { | ||
| rowKey = startKey; | ||
| } else if (scan.includeStopRow()) { | ||
| rowKey = endKey; | ||
|
Comment on lines
+314
to
+315
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this generates correct rowkey: if scan start rowkey is not inclusive, we need to find the shortest possible next rowkey?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @virajjasani , But actually you are right, we can use the nextkey before jumping to the endkey here. Something like this should be better? What do you think? |
||
| } else { | ||
| rowKey = HConstants.EMPTY_END_ROW; | ||
| } | ||
| } | ||
|
|
||
| return rowKey; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming convention for this method is similar to
getScanStartRowKeyFromScanOrRegionBoundariesmethod in the ServerUtil.javaThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use
getScanStartRowKeyFromScanOrRegionBoundaries()instead of this new method?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@virajjasani,
No, we cannot use
getScanStartRowKeyFromScanOrRegionBoundaries()because the two methods serve different purposes -getScanStartRowKeyFromScanOrRegionBoundaries()- Returns only the start row key of the scanwhile we need
getLargestPossibleRowKeyInRange()which analyzes the entire range. Which is implemented inderiveRowKeyFromScanOrRegionBoundaries(). It basically derives a representative row key within the scan range for the OFFSET queries.