srowen commented on a change in pull request #26548: [SPARK-29918][SQL]
RecordBinaryComparator should check endianness when compared by long
URL: https://github.com/apache/spark/pull/26548#discussion_r347508931
##########
File path:
sql/core/src/test/java/test/org/apache/spark/sql/execution/sort/RecordBinaryComparatorSuite.java
##########
@@ -321,4 +321,46 @@ public void
testBinaryComparatorWhenOnlyTheLastColumnDiffers() throws Exception
assert(compare(0, 1) < 0);
}
+
+ @Test
+ 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);
Review comment:
BTW I don't think we have to fix it here necessarily, but this whole suite
is using JVM asserts, not JUnit asserts. It's still reasonable as assertions
are enabled in tests, but, isn't the best practice - error messages are not as
informative, and if someone doesn't turn on asserts, doesn't actually test. I
don't mind fixing this separately ; there are just a few Java suites with this
problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
