From 37ebd3856b6aa83b249fe0c7f07bb96d778e1399 Mon Sep 17 00:00:00 2001 From: "wangguangxin.cn" Date: Sat, 16 Nov 2019 00:33:21 +0800 Subject: [PATCH 1/6] handle endianness --- .../spark/sql/execution/RecordBinaryComparator.java | 10 ++++++++-- .../execution/sort/RecordBinaryComparatorSuite.java | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java b/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java index 40c2cc806e87a..7ced5b815cf5f 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java +++ b/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java @@ -20,6 +20,8 @@ import org.apache.spark.unsafe.Platform; import org.apache.spark.util.collection.unsafe.sort.RecordComparator; +import java.nio.ByteOrder; + public final class RecordBinaryComparator extends RecordComparator { @Override @@ -49,9 +51,13 @@ public int compare( // 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) { - final long v1 = Platform.getLong(leftObj, leftOff + i); - final long v2 = Platform.getLong(rightObj, rightOff + i); + long v1 = Platform.getLong(leftObj, leftOff + i); + long v2 = Platform.getLong(rightObj, rightOff + i); if (v1 != v2) { + if (ByteOrder.nativeOrder().equals(ByteOrder.LITTLE_ENDIAN)) { + v1 = Long.reverseBytes(v1); + v2 = Long.reverseBytes(v2); + } return v1 > v2 ? 1 : -1; } i += 8; diff --git a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java index 92dabc79d2bff..bbcc78eb7cedb 100644 --- a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java +++ b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java @@ -273,7 +273,7 @@ public void testBinaryComparatorWhenSubtractionIsDivisibleByMaxIntValue() throws insertRow(row1); insertRow(row2); - assert(compare(0, 1) < 0); + assert(compare(0, 1) > 0); } @Test From 3cb480d894dcacedcca0483f8c10dc316418731e Mon Sep 17 00:00:00 2001 From: "wangguangxin.cn" Date: Mon, 18 Nov 2019 14:39:27 +0800 Subject: [PATCH 2/6] use long.compareUnsigned & add ut --- .../sql/execution/RecordBinaryComparator.java | 27 ++++--- .../exchange/ShuffleExchangeExec.scala | 4 +- .../sort/RecordBinaryComparatorSuite.java | 72 +++++++++++++++++-- 3 files changed, 88 insertions(+), 15 deletions(-) diff --git a/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java b/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java index 7ced5b815cf5f..730fff77b33eb 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java +++ b/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java @@ -20,10 +20,14 @@ import org.apache.spark.unsafe.Platform; import org.apache.spark.util.collection.unsafe.sort.RecordComparator; -import java.nio.ByteOrder; - public final class RecordBinaryComparator extends RecordComparator { + boolean isLittlenEndian; + + public RecordBinaryComparator(boolean isLittlenEndian) { + this.isLittlenEndian = isLittlenEndian; + } + @Override public int compare( Object leftObj, long leftOff, int leftLen, Object rightObj, long rightOff, int rightLen) { @@ -40,10 +44,10 @@ 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) { - final int v1 = Platform.getByte(leftObj, leftOff + i) & 0xff; - final int v2 = Platform.getByte(rightObj, rightOff + i) & 0xff; + final int v1 = Platform.getByte(leftObj, leftOff + i); + final int v2 = Platform.getByte(rightObj, rightOff + i); if (v1 != v2) { - return v1 > v2 ? 1 : -1; + return (v1 & 0xff) > (v2 & 0xff) ? 1 : -1; } i += 1; } @@ -54,11 +58,14 @@ public int compare( long v1 = Platform.getLong(leftObj, leftOff + i); long v2 = Platform.getLong(rightObj, rightOff + i); if (v1 != v2) { - if (ByteOrder.nativeOrder().equals(ByteOrder.LITTLE_ENDIAN)) { + if (isLittlenEndian) { + // if read as little-endian, we have to reverse bytes so that the long comparison result + // is equivalent to byte-by-byte comparison result. + // See discussion in https://github.com/apache/spark/pull/26548#issuecomment-554645859 v1 = Long.reverseBytes(v1); v2 = Long.reverseBytes(v2); } - return v1 > v2 ? 1 : -1; + return Long.compareUnsigned(v1, v2); } i += 8; } @@ -66,10 +73,10 @@ public int compare( // this will finish off the unaligned comparisons, or do the entire aligned comparison // whichever is needed. while (i < leftLen) { - final int v1 = Platform.getByte(leftObj, leftOff + i) & 0xff; - final int v2 = Platform.getByte(rightObj, rightOff + i) & 0xff; + final int v1 = Platform.getByte(leftObj, leftOff + i); + final int v2 = Platform.getByte(rightObj, rightOff + i); if (v1 != v2) { - return v1 > v2 ? 1 : -1; + return (v1 & 0xff) > (v2 & 0xff) ? 1 : -1; } i += 1; } diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala index 2f94c522712b1..7bf9f3392ec4d 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala @@ -17,6 +17,7 @@ package org.apache.spark.sql.execution.exchange +import java.nio.ByteOrder import java.util.Random import java.util.function.Supplier @@ -243,7 +244,8 @@ object ShuffleExchangeExec { val newRdd = if (isRoundRobin && SQLConf.get.sortBeforeRepartition) { rdd.mapPartitionsInternal { iter => val recordComparatorSupplier = new Supplier[RecordComparator] { - override def get: RecordComparator = new RecordBinaryComparator() + val isLittlenEndian = ByteOrder.nativeOrder.equals(ByteOrder.LITTLE_ENDIAN) + override def get: RecordComparator = new RecordBinaryComparator(isLittlenEndian) } // The comparator for comparing row hashcode, which should always be Integer. val prefixComparator = PrefixComparators.LONG diff --git a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java index bbcc78eb7cedb..d4c7bbef6ff8c 100644 --- a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java +++ b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java @@ -56,8 +56,8 @@ public class RecordBinaryComparatorSuite { @Before public void beforeEach() { - // Only compare between two input rows. - array = consumer.allocateArray(2); + // At most three input rows + array = consumer.allocateArray(3); pos = 0; dataPage = memoryManager.allocatePage(4096, consumer); @@ -88,7 +88,7 @@ private void insertRow(UnsafeRow row) { Platform.copyMemory(recordBase, recordOffset, baseObject, pageCursor, recordLength); pageCursor += recordLength; - assert(pos < 2); + assert(pos < 3); array.set(pos, recordAddress); pos++; } @@ -108,7 +108,7 @@ private int compare(int index1, int index2) { baseOffset2, recordLength2); } - private final RecordComparator binaryComparator = new RecordBinaryComparator(); + private final RecordComparator binaryComparator = new RecordBinaryComparator(true); // Compute the most compact size for UnsafeRow's backing data. private int computeSizeInBytes(int originalSize) { @@ -321,4 +321,68 @@ public void testBinaryComparatorWhenOnlyTheLastColumnDiffers() throws Exception assert(compare(0, 1) < 0); } + + @Test + public void testBinaryComparatorGenerateSameResultBetweenComparedByteByByteAndComparedByLong() throws Exception { + int numFields = 1; + + UnsafeRow row1 = new UnsafeRow(numFields); + byte[] data1 = new byte[100]; + row1.pointTo(data1, computeSizeInBytes(numFields * 8)); + row1.setLong(0, 0x0800000000000000L); + + UnsafeRow row2 = new UnsafeRow(numFields); + byte[] data2 = new byte[100]; + row2.pointTo(data2, computeSizeInBytes(numFields * 8)); + row2.setLong(0, 0x0000008000000000L); + + UnsafeRow row3 = new UnsafeRow(numFields); + byte[] data3 = new byte[100]; + row3.pointTo(data3, computeSizeInBytes(numFields * 8)); + row3.setLong(0, 0x0000008000000000L); + + insertRow(row1); + insertRow(row2); + insertRow(row3); + + // the bytes in row2 and row3 are the same. + // the base offset of row1 is 20, row2 is 40, row3 is 60 + // so the RecordBinaryComparator will compare row1 and row2 with two long comparison directly, + // while the comparison between row1 and row3 is started with 4 bytes byte-by-byte comparison, + // followed by a long comparison, and lastly 4 bytes byte-by-byte comparison. + assert(compare(0, 1) < 0); + assert(compare(0, 2) < 0); + } + + @Test + public void testBinaryComparatorShouldComparedWithUnsignedLong() throws Exception { + int numFields = 1; + + UnsafeRow row1 = new UnsafeRow(numFields); + byte[] data1 = new byte[100]; + row1.pointTo(data1, computeSizeInBytes(numFields * 8)); + row1.setLong(0, 0xa000000000000000L); + + UnsafeRow row2 = new UnsafeRow(numFields); + byte[] data2 = new byte[100]; + row2.pointTo(data2, computeSizeInBytes(numFields * 8)); + row2.setLong(0, 0x0000000000000000L); + + UnsafeRow row3 = new UnsafeRow(numFields); + byte[] data3 = new byte[100]; + row3.pointTo(data3, computeSizeInBytes(numFields * 8)); + row3.setLong(0, 0x0000000000000000L); + + insertRow(row1); + insertRow(row2); + insertRow(row3); + + // the bytes in row2 and row3 are the same. + // the base offset of row1 is 20, row2 is 40, row3 is 60 + // so the RecordBinaryComparator will compare row1 and row2 with two long comparison directly, + // while the comparison between row1 and row3 is started with 4 bytes byte-by-byte comparison, + // followed by a long comparison, and lastly 4 bytes byte-by-byte comparison. + assert(compare(0, 1) > 0); + assert(compare(0, 2) > 0); + } } From a07cfc6816b49edfde216ae8694651d6c4f87565 Mon Sep 17 00:00:00 2001 From: "wangguangxin.cn" Date: Mon, 18 Nov 2019 21:59:37 +0800 Subject: [PATCH 3/6] fix style --- .../spark/sql/execution/sort/RecordBinaryComparatorSuite.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java index d4c7bbef6ff8c..72a26174b80dd 100644 --- a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java +++ b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java @@ -323,7 +323,8 @@ public void testBinaryComparatorWhenOnlyTheLastColumnDiffers() throws Exception } @Test - public void testBinaryComparatorGenerateSameResultBetweenComparedByteByByteAndComparedByLong() throws Exception { + public void testBinaryComparatorGiveSameResultWhenComparedByteByByteAndComparedByLong() + throws Exception { int numFields = 1; UnsafeRow row1 = new UnsafeRow(numFields); From fdbc871cd4119da970c8709451942c58b2ac6130 Mon Sep 17 00:00:00 2001 From: "wangguangxin.cn" Date: Mon, 18 Nov 2019 23:24:20 +0800 Subject: [PATCH 4/6] update --- .../sql/execution/RecordBinaryComparator.java | 11 +- .../exchange/ShuffleExchangeExec.scala | 4 +- .../sort/RecordBinaryComparatorSuite.java | 102 +++++++----------- 3 files changed, 45 insertions(+), 72 deletions(-) diff --git a/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java b/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java index 730fff77b33eb..1f243406c77e0 100644 --- a/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java +++ b/sql/core/src/main/java/org/apache/spark/sql/execution/RecordBinaryComparator.java @@ -20,13 +20,12 @@ import org.apache.spark.unsafe.Platform; import org.apache.spark.util.collection.unsafe.sort.RecordComparator; -public final class RecordBinaryComparator extends RecordComparator { +import java.nio.ByteOrder; - boolean isLittlenEndian; +public final class RecordBinaryComparator extends RecordComparator { - public RecordBinaryComparator(boolean isLittlenEndian) { - this.isLittlenEndian = isLittlenEndian; - } + private static final boolean LITTLE_ENDIAN = + ByteOrder.nativeOrder().equals(ByteOrder.LITTLE_ENDIAN); @Override public int compare( @@ -58,7 +57,7 @@ public int compare( long v1 = Platform.getLong(leftObj, leftOff + i); long v2 = Platform.getLong(rightObj, rightOff + i); if (v1 != v2) { - if (isLittlenEndian) { + if (LITTLE_ENDIAN) { // if read as little-endian, we have to reverse bytes so that the long comparison result // is equivalent to byte-by-byte comparison result. // See discussion in https://github.com/apache/spark/pull/26548#issuecomment-554645859 diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala index 7bf9f3392ec4d..2f94c522712b1 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/exchange/ShuffleExchangeExec.scala @@ -17,7 +17,6 @@ package org.apache.spark.sql.execution.exchange -import java.nio.ByteOrder import java.util.Random import java.util.function.Supplier @@ -244,8 +243,7 @@ object ShuffleExchangeExec { val newRdd = if (isRoundRobin && SQLConf.get.sortBeforeRepartition) { rdd.mapPartitionsInternal { iter => val recordComparatorSupplier = new Supplier[RecordComparator] { - val isLittlenEndian = ByteOrder.nativeOrder.equals(ByteOrder.LITTLE_ENDIAN) - override def get: RecordComparator = new RecordBinaryComparator(isLittlenEndian) + override def get: RecordComparator = new RecordBinaryComparator() } // The comparator for comparing row hashcode, which should always be Integer. val prefixComparator = PrefixComparators.LONG diff --git a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java index 72a26174b80dd..1f510fd439153 100644 --- a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java +++ b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java @@ -56,8 +56,8 @@ public class RecordBinaryComparatorSuite { @Before public void beforeEach() { - // At most three input rows - array = consumer.allocateArray(3); + // Only compare between two input rows. + array = consumer.allocateArray(2); pos = 0; dataPage = memoryManager.allocatePage(4096, consumer); @@ -88,7 +88,7 @@ private void insertRow(UnsafeRow row) { Platform.copyMemory(recordBase, recordOffset, baseObject, pageCursor, recordLength); pageCursor += recordLength; - assert(pos < 3); + assert(pos < 2); array.set(pos, recordAddress); pos++; } @@ -108,7 +108,7 @@ private int compare(int index1, int index2) { baseOffset2, recordLength2); } - private final RecordComparator binaryComparator = new RecordBinaryComparator(true); + private final RecordComparator binaryComparator = new RecordBinaryComparator(); // Compute the most compact size for UnsafeRow's backing data. private int computeSizeInBytes(int originalSize) { @@ -323,67 +323,43 @@ public void testBinaryComparatorWhenOnlyTheLastColumnDiffers() throws Exception } @Test - public void testBinaryComparatorGiveSameResultWhenComparedByteByByteAndComparedByLong() - throws Exception { - int numFields = 1; - - UnsafeRow row1 = new UnsafeRow(numFields); - byte[] data1 = new byte[100]; - row1.pointTo(data1, computeSizeInBytes(numFields * 8)); - row1.setLong(0, 0x0800000000000000L); - - UnsafeRow row2 = new UnsafeRow(numFields); - byte[] data2 = new byte[100]; - row2.pointTo(data2, computeSizeInBytes(numFields * 8)); - row2.setLong(0, 0x0000008000000000L); - - UnsafeRow row3 = new UnsafeRow(numFields); - byte[] data3 = new byte[100]; - row3.pointTo(data3, computeSizeInBytes(numFields * 8)); - row3.setLong(0, 0x0000008000000000L); - - insertRow(row1); - insertRow(row2); - insertRow(row3); - - // the bytes in row2 and row3 are the same. - // the base offset of row1 is 20, row2 is 40, row3 is 60 - // so the RecordBinaryComparator will compare row1 and row2 with two long comparison directly, - // while the comparison between row1 and row3 is started with 4 bytes byte-by-byte comparison, - // followed by a long comparison, and lastly 4 bytes byte-by-byte comparison. - assert(compare(0, 1) < 0); - assert(compare(0, 2) < 0); + public void testCompareLongsAsLittleEndian() { + long arrayOffset = 12; + + long[] arr1 = new long[2]; + Platform.putLong(arr1, arrayOffset, 0x0100000000000000L); + long[] arr2 = new long[2]; + Platform.putLong(arr2, arrayOffset + 4, 0x0000000000000001L); + int result1 = binaryComparator.compare(arr1, arrayOffset, 8, arr2, arrayOffset + 4, 8); + + long[] arr3 = new long[2]; + Platform.putLong(arr3, arrayOffset, 0x0100000000000000L); + long[] arr4 = new long[2]; + Platform.putLong(arr4, arrayOffset, 0x0000000000000001L); + int result2 = binaryComparator.compare(arr3, arrayOffset, 8, arr4, arrayOffset, 8); + + assert(result1 == result2); } @Test - public void testBinaryComparatorShouldComparedWithUnsignedLong() throws Exception { - int numFields = 1; - - UnsafeRow row1 = new UnsafeRow(numFields); - byte[] data1 = new byte[100]; - row1.pointTo(data1, computeSizeInBytes(numFields * 8)); - row1.setLong(0, 0xa000000000000000L); - - UnsafeRow row2 = new UnsafeRow(numFields); - byte[] data2 = new byte[100]; - row2.pointTo(data2, computeSizeInBytes(numFields * 8)); - row2.setLong(0, 0x0000000000000000L); - - UnsafeRow row3 = new UnsafeRow(numFields); - byte[] data3 = new byte[100]; - row3.pointTo(data3, computeSizeInBytes(numFields * 8)); - row3.setLong(0, 0x0000000000000000L); - - insertRow(row1); - insertRow(row2); - insertRow(row3); - - // the bytes in row2 and row3 are the same. - // the base offset of row1 is 20, row2 is 40, row3 is 60 - // so the RecordBinaryComparator will compare row1 and row2 with two long comparison directly, - // while the comparison between row1 and row3 is started with 4 bytes byte-by-byte comparison, - // followed by a long comparison, and lastly 4 bytes byte-by-byte comparison. - assert(compare(0, 1) > 0); - assert(compare(0, 2) > 0); + public void testCompareLongsAsUnsigned() { + long arrayOffset = 12; + + long[] arr1 = new long[2]; + Platform.putLong(arr1, arrayOffset + 4, 0xa000000000000000L); + long[] arr2 = new long[2]; + Platform.putLong(arr2, arrayOffset + 4, 0x0000000000000000L); + // both leftBaseOffset and rightBaseOffset are aligned, so it will start by comparing longs + int result1 = binaryComparator.compare(arr1, arrayOffset + 4, 8, arr2, arrayOffset + 4, 8); + + long[] arr3 = new long[2]; + Platform.putLong(arr3, arrayOffset, 0xa000000000000000L); + long[] arr4 = new long[2]; + Platform.putLong(arr4, arrayOffset, 0x0000000000000000L); + // both leftBaseOffset and rightBaseOffset are not aligned, so it will start with 4 bytes byte-by-byte comparison, + // followed by a long comparison, and at last 4 bytes byte-by-byte comparison + int result2 = binaryComparator.compare(arr3, arrayOffset, 8, arr4, arrayOffset, 8); + + assert(result1 == result2); } } From 5cc04fa81df6b2737942f2b98aa0764b41edd0e6 Mon Sep 17 00:00:00 2001 From: "wangguangxin.cn" Date: Mon, 18 Nov 2019 23:34:37 +0800 Subject: [PATCH 5/6] style --- .../spark/sql/execution/sort/RecordBinaryComparatorSuite.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java index 1f510fd439153..f4f0b052b2136 100644 --- a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java +++ b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java @@ -356,7 +356,8 @@ public void testCompareLongsAsUnsigned() { Platform.putLong(arr3, arrayOffset, 0xa000000000000000L); long[] arr4 = new long[2]; Platform.putLong(arr4, arrayOffset, 0x0000000000000000L); - // both leftBaseOffset and rightBaseOffset are not aligned, so it will start with 4 bytes byte-by-byte comparison, + // both leftBaseOffset and rightBaseOffset are not aligned, + // so it will start with 4 bytes byte-by-byte comparison, // followed by a long comparison, and at last 4 bytes byte-by-byte comparison int result2 = binaryComparator.compare(arr3, arrayOffset, 8, arr4, arrayOffset, 8); From 753883d7f84552c7392295aa42f5023c0909c615 Mon Sep 17 00:00:00 2001 From: "wangguangxin.cn" Date: Tue, 19 Nov 2019 11:00:57 +0800 Subject: [PATCH 6/6] update comments & ut --- .../execution/sort/RecordBinaryComparatorSuite.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java index f4f0b052b2136..68f984ae0c1e3 100644 --- a/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java +++ b/sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java @@ -33,6 +33,7 @@ import org.apache.spark.util.collection.unsafe.sort.*; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -330,15 +331,18 @@ public void testCompareLongsAsLittleEndian() { Platform.putLong(arr1, arrayOffset, 0x0100000000000000L); long[] arr2 = new long[2]; Platform.putLong(arr2, arrayOffset + 4, 0x0000000000000001L); + // leftBaseOffset is not aligned while rightBaseOffset is aligned, + // it will start by comparing long int result1 = binaryComparator.compare(arr1, arrayOffset, 8, arr2, arrayOffset + 4, 8); long[] arr3 = new long[2]; Platform.putLong(arr3, arrayOffset, 0x0100000000000000L); long[] arr4 = new long[2]; Platform.putLong(arr4, arrayOffset, 0x0000000000000001L); + // both left and right offset is not aligned, it will start with byte-by-byte comparison int result2 = binaryComparator.compare(arr3, arrayOffset, 8, arr4, arrayOffset, 8); - assert(result1 == result2); + Assert.assertEquals(result1, result2); } @Test @@ -349,7 +353,7 @@ public void testCompareLongsAsUnsigned() { Platform.putLong(arr1, arrayOffset + 4, 0xa000000000000000L); long[] arr2 = new long[2]; Platform.putLong(arr2, arrayOffset + 4, 0x0000000000000000L); - // both leftBaseOffset and rightBaseOffset are aligned, so it will start by comparing longs + // both leftBaseOffset and rightBaseOffset are aligned, so it will start by comparing long int result1 = binaryComparator.compare(arr1, arrayOffset + 4, 8, arr2, arrayOffset + 4, 8); long[] arr3 = new long[2]; @@ -357,10 +361,9 @@ public void testCompareLongsAsUnsigned() { long[] arr4 = new long[2]; Platform.putLong(arr4, arrayOffset, 0x0000000000000000L); // both leftBaseOffset and rightBaseOffset are not aligned, - // so it will start with 4 bytes byte-by-byte comparison, - // followed by a long comparison, and at last 4 bytes byte-by-byte comparison + // so it will start with byte-by-byte comparison int result2 = binaryComparator.compare(arr3, arrayOffset, 8, arr4, arrayOffset, 8); - assert(result1 == result2); + Assert.assertEquals(result1, result2); } }