sunchao commented on code in PR #6361:
URL: https://github.com/apache/datafusion-comet/pull/6361#discussion_r4315823720
##########
spark/src/test/scala/org/apache/spark/shuffle/sort/SpillSorterSuite.scala:
##########
@@ -267,6 +273,43 @@ class SpillSorterSuite extends AnyFunSuite with
BeforeAndAfterEach {
}
}
+ test("write sorted file across partitions with shuffle checksums disabled") {
+ // With spark.shuffle.checksum.enabled=false the sorter gets an empty
checksum array and no
+ // checksum algorithm.
+ val spills = new java.util.LinkedList[SpillInfo]()
+ val sorter = createSpillSorter(
+ spills = spills,
+ partitionChecksums = Array.emptyLongArray,
+ checksumAlgorithm = null)
+ val file = File.createTempFile("spill-sorter", ".data")
+ try {
+ val numPartitions = 3
+ val rowSize = UnsafeRow.calculateBitSetWidthInBytes(1) + 8
+ // Each record is a 4-byte key followed by an UnsafeRow with one int
column.
+ val record = new Array[Byte](4 + rowSize)
+ val row = new UnsafeRow(1)
+ row.pointTo(record, Platform.BYTE_ARRAY_OFFSET + 4, rowSize)
Review Comment:
[P2] Make this offset explicitly `Long`, for example
`Platform.BYTE_ARRAY_OFFSET + 4L`. Spark declares `BYTE_ARRAY_OFFSET` as `int`,
so `+ 4` produces an `Int` that is implicitly widened for
`UnsafeRow.pointTo(Object, long, int)`. The required strict Scala job enables
`-Ywarn-numeric-widen` and `-Xfatal-warnings`, causing test compilation to fail
here even after the separately reported `SpillInfo` collision is fixed. An
explicit long expression preserves the intended address and removes this
independent build blocker.
Evidence: Exact-head job
https://github.com/apache/datafusion-comet/actions/runs/39169798179/job/117800950319
reports `SpillSorterSuite.scala:291: implicit numeric widening`. A bounded JDK
17 / Scala 2.12.18 probe using Spark’s method signatures and the current
pom.xml strict test compiler arguments reproduced this error with `SpillInfo`
already aliased. The original offset exited 1 with the numeric-widening warning
promoted to an error. Changing only `+ 4` to `+ 4L` exited 0.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
