Skip to content
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.

Sign up for GitHub

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

[SPARK-25114][Core] Fix RecordBinaryComparator when subtraction between two words is divisible by Integer.MAX_VALUE. #22101

Closed
wants to merge 4 commits into from
Closed

[SPARK-25114][Core] Fix RecordBinaryComparator when subtraction between two words is divisible by Integer.MAX_VALUE. #22101

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f6b259
fix
jiangxb1987 Aug 14, 2018
ddcfea3
update
jiangxb1987 Aug 14, 2018
9c1f486
add test cases
jiangxb1987 Aug 15, 2018
ecb26fc
update
jiangxb1987 Aug 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions sql/catalyst/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public final class RecordBinaryComparator extends RecordComparator {
public int compare(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let us add a test suite for this function as you mentioned in the line 25. : )

Object leftObj, long leftOff, int leftLen, Object rightObj, long rightOff, int rightLen) {
int i = 0;
int res = 0;

// If the arrays have different length, the longer one is larger.
if (leftLen != rightLen) {
Expand All @@ -40,27 +39,33 @@ public int compare(
// check if stars align and we can get both offsets to be aligned
if ((leftOff % 8) == (rightOff % 8)) {
while ((leftOff + i) % 8 != 0 && i < leftLen) {
res = (Platform.getByte(leftObj, leftOff + i) & 0xff) -
(Platform.getByte(rightObj, rightOff + i) & 0xff);
if (res != 0) return res;
final int v1 = Platform.getByte(leftObj, leftOff + i) & 0xff;
final int v2 = Platform.getByte(rightObj, rightOff + i) & 0xff;
if (v1 != v2) {
return v1 > v2 ? 1 : -1;
}
i += 1;
}
}
// for architectures that support unaligned accesses, chew it up 8 bytes at a time
if (Platform.unaligned() || (((leftOff + i) % 8 == 0) && ((rightOff + i) % 8 == 0))) {
while (i <= leftLen - 8) {
res = (int) ((Platform.getLong(leftObj, leftOff + i) -
Platform.getLong(rightObj, rightOff + i)) % Integer.MAX_VALUE);
if (res != 0) return res;
final long v1 = Platform.getLong(leftObj, leftOff + i);
final long v2 = Platform.getLong(rightObj, rightOff + i);
if (v1 != v2) {
return v1 > v2 ? 1 : -1;
}
i += 8;
}
}
// this will finish off the unaligned comparisons, or do the entire aligned comparison
// whichever is needed.
while (i < leftLen) {
res = (Platform.getByte(leftObj, leftOff + i) & 0xff) -
(Platform.getByte(rightObj, rightOff + i) & 0xff);
if (res != 0) return res;
final int v1 = Platform.getByte(leftObj, leftOff + i) & 0xff;
final int v2 = Platform.getByte(rightObj, rightOff + i) & 0xff;
if (v1 != v2) {
return v1 > v2 ? 1 : -1;
}
i += 1;
}

Expand Down