From 30bea0b476f7b439b7b1f6c5ca5d015d171dd2f5 Mon Sep 17 00:00:00 2001 From: Eunbin Son Date: Thu, 10 Sep 2026 08:57:06 +0900 Subject: [PATCH] MR: Keep computed block locations in IcebergSplit.getLocations The transient locations field and the locations == null check make getLocations() a one-time computation that is cached on the split, but the else branch reassigned locations to ANYWHERE on every later call. Calling getLocations() twice on the same split therefore returned the computed block locations first and "*" afterwards, which breaks the Hadoop InputSplit contract that repeated calls return the same value. The else branch was added in #1582 to avoid an NPE when getLocations() runs on a worker node, where the deserialized split has a null conf. Folding the conf null check into localityPreferred keeps that guard: with a null conf the split still resolves to ANYWHERE and never calls Util.blockLocations. Extend testLocality to assert that a second call returns the same locations. Generated-by: Claude Code --- .../java/org/apache/iceberg/mr/mapreduce/IcebergSplit.java | 7 +++---- .../org/apache/iceberg/mr/TestIcebergInputFormats.java | 2 ++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/mr/src/main/java/org/apache/iceberg/mr/mapreduce/IcebergSplit.java b/mr/src/main/java/org/apache/iceberg/mr/mapreduce/IcebergSplit.java index e4aeeaee6d3f..7295ff79ea94 100644 --- a/mr/src/main/java/org/apache/iceberg/mr/mapreduce/IcebergSplit.java +++ b/mr/src/main/java/org/apache/iceberg/mr/mapreduce/IcebergSplit.java @@ -71,11 +71,10 @@ public long getLength() { public String[] getLocations() { // The implementation of getLocations() is only meant to be used during split computation // getLocations() won't be accurate when called on worker nodes and will always return "*" - if (locations == null && conf != null) { - boolean localityPreferred = conf.getBoolean(InputFormatConfig.LOCALITY, false); + if (locations == null) { + boolean localityPreferred = + conf != null && conf.getBoolean(InputFormatConfig.LOCALITY, false); locations = localityPreferred ? Util.blockLocations(task, conf) : ANYWHERE.clone(); - } else { - locations = ANYWHERE.clone(); } return locations; diff --git a/mr/src/test/java/org/apache/iceberg/mr/TestIcebergInputFormats.java b/mr/src/test/java/org/apache/iceberg/mr/TestIcebergInputFormats.java index ce588a7e83e2..ad63b0b58c35 100644 --- a/mr/src/test/java/org/apache/iceberg/mr/TestIcebergInputFormats.java +++ b/mr/src/test/java/org/apache/iceberg/mr/TestIcebergInputFormats.java @@ -326,6 +326,8 @@ public void testLocality() throws Exception { for (InputSplit split : testInputFormat.create(builder.conf()).getSplits()) { assertThat(split.getLocations()).containsExactly("localhost"); + // repeated calls must return the same locations that were computed and cached above + assertThat(split.getLocations()).containsExactly("localhost"); } }