[GitHub] [spark] srowen commented on a change in pull request #26650: [SPARK-30067][CORE] Fix fragment offset comparison in getBlockHosts

[GitBox <gi...@apache.org>]

srowen commented on a change in pull request #26650: [SPARK-30067][CORE] Fix fragment offset comparison in getBlockHosts
URL: https://github.com/apache/spark/pull/26650#discussion_r353875112
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/PartitionedFileUtil.scala
 ##########
 @@ -64,13 +64,14 @@ object PartitionedFileUtil {
       offset: Long,
       length: Long): Array[String] = {
     val candidates = blockLocations.map {
-      // The fragment starts from a position within this block
+      // The fragment starts from a position within this block. It handles the case where the
+      // fragment is fully contained in the block.
       case b if b.getOffset <= offset && offset < b.getOffset + b.getLength =>
         b.getHosts -> (b.getOffset + b.getLength - offset).min(length)
 
       // The fragment ends at a position within this block
-      case b if offset <= b.getOffset && offset + length < b.getLength =>
-        b.getHosts -> (offset + length - b.getOffset).min(length)
+      case b if b.getOffset <= offset + length && offset + length < b.getOffset + b.getLength =>
 
 Review comment:
   It won't matter either way, but yeah if `b.getOffset == offset + length`, you could say the fragment doesn't actually end in the block; it ends just before it. Just for symmetry it might be OK to keep it this way, but I don't feel strongly about it.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org