You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by cs...@apache.org on 2026/09/14 14:41:12 UTC

(impala) branch master updated: IMPALA-15291: Fix EC Policy column when block locations are not preloaded

This is an automated email from the ASF dual-hosted git repository.

asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git


The following commit(s) were added to refs/heads/master by this push:
     new a921854ac IMPALA-15291: Fix EC Policy column when block locations are not preloaded
a921854ac is described below

commit a921854ac4bd6c2e8a5b2ba896aa639af12f0a08
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Wed Aug 26 13:50:28 2026 +0200

    IMPALA-15291: Fix EC Policy column when block locations are not preloaded
    
    IMPALA-15229 made the "EC Policy" column (SHOW TABLE STATS, SHOW
    PARTITIONS, SHOW FILES) derive from FileDescriptor.getIsEc(). That flag
    was only set on the block-loading path (create()); createWithNoBlocks(),
    used when block-location preloading is disabled, hardcoded is_ec=false,
    so erasure-coded files loaded that way reported NONE.
    
    createWithNoBlocks now reads the erasure-coding and encryption flags from
    the FileStatus, like create(). Wherever the listing carries a real (HDFS)
    FileStatus, getIsEc()/getIsEncrypted() are now correct at no extra cost.
    
    Iceberg is the exception: on the no-preload path it builds the FileStatus
    synthetically (size and path only) to avoid a per-file stat RPC, so it
    carries no erasure-coding info. Rather than re-add a per-file NameNode RPC
    that could still be wrong (a directory's policy is not necessarily the
    file's), SHOW FILES reports "N/A" for such descriptors. Knowing the EC
    policy in such cases wouldn't have too much added value as we already
    schedule blockless splits as remote reads, so EC policy doesn't change
    much for query processing. SHOW TABLE STATS is unaffected: it reads the
    policy from one RPC on the table location.
    
    Testing
    * FeFsTableTest: descriptors from createWithNoBlocks keep the flags; a
      partition mixing an EC and a plain file reports MIXED.
    * iceberg-insert.test SHOW FILES rows assert $EXPECTED_ERASURECODE_POLICY,
      supplied per driver: the real policy in test_iceberg, "N/A" in
      TestDisabledBlockLocations.
    * test_iceberg_smoke/test_iceberg_v3_smoke in TestDisabledBlockLocations
      on an EC build.
    
    Change-Id: Ia7a7c97a20f6ec0efc2150185f4a9e26e3ac43cf
    Assisted-by: Claude Opus 4.8 <no...@anthropic.com>
    Reviewed-on: http://gerrit.cloudera.org:8080/24758
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../org/apache/impala/catalog/FeIcebergTable.java  | 19 ++++++++++++--
 .../org/apache/impala/catalog/FileDescriptor.java  | 11 +++++---
 .../org/apache/impala/common/FileSystemUtil.java   |  4 +++
 .../org/apache/impala/catalog/FeFsTableTest.java   | 29 ++++++++++++++++++++++
 .../queries/QueryTest/iceberg-insert.test          |  6 ++---
 .../test_disabled_block_locations.py               |  5 +++-
 tests/query_test/test_iceberg.py                   |  5 +++-
 7 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java b/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
index bf7857b20..0e4398a04 100644
--- a/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
@@ -556,7 +556,7 @@ public interface FeIcebergTable extends FeFsTable {
         rowBuilder.add(absPath);
         rowBuilder.add(PrintUtils.printBytes(fd.getFileLength()));
         rowBuilder.add("");
-        rowBuilder.add(FeFsTable.getErasureCodingPolicy(fd, new Path(absPath)));
+        rowBuilder.add(getShowFilesEcPolicy(fd, new Path(absPath)));
         result.addToRows(rowBuilder.get());
       }
       return result;
@@ -592,7 +592,7 @@ public interface FeIcebergTable extends FeFsTable {
 
         rowBuilder.add("");         // Partition - empty for now
         rowBuilder.add(fd != null
-            ? FeFsTable.getErasureCodingPolicy(fd, new Path(filePath))
+            ? getShowFilesEcPolicy(fd, new Path(filePath))
             : FileSystemUtil.getErasureCodingPolicy(new Path(filePath)));
         result.addToRows(rowBuilder.get());
       }
@@ -600,6 +600,21 @@ public interface FeIcebergTable extends FeFsTable {
       return result;
     }
 
+    /**
+     * Erasure-coding policy for a SHOW FILES row. is_ec is only ever set from a real
+     * FileStatus, so trust it when present (the policy resolves locally from the recorded
+     * id). Otherwise the file is either not erasure-coded, or came from the no-preload
+     * path where Iceberg builds a synthetic FileStatus with no erasure-coding info.
+     */
+    private static String getShowFilesEcPolicy(FileDescriptor fd, Path path) {
+      if (fd.getIsEc()) return FeFsTable.getErasureCodingPolicy(fd, path);
+      boolean syntheticNoBlocks = fd.getFileLength() > 0 && fd.getNumFileBlocks() == 0
+          && (FileSystemUtil.isDistributedFileSystem(path)
+              || FileSystemUtil.isOzoneFileSystem(path));
+      return syntheticNoBlocks ? FileSystemUtil.UNKNOWN_ERASURE_CODE_LABEL
+                               : FileSystemUtil.NO_ERASURE_CODE_LABEL;
+    }
+
     /**
      * Get partition stats for the given fe iceberg table.
      */
diff --git a/fe/src/main/java/org/apache/impala/catalog/FileDescriptor.java b/fe/src/main/java/org/apache/impala/catalog/FileDescriptor.java
index 92f465059..84db07401 100644
--- a/fe/src/main/java/org/apache/impala/catalog/FileDescriptor.java
+++ b/fe/src/main/java/org/apache/impala/catalog/FileDescriptor.java
@@ -159,14 +159,17 @@ public class FileDescriptor implements Comparable<FileDescriptor> {
   }
 
   /**
-   * Creates the file descriptor of a file represented by 'fileStatus' that
-   * resides in a filesystem that doesn't support the BlockLocation API (e.g. S3).
+   * Creates the descriptor of a file whose block locations are not loaded (the
+   * filesystem lacks the BlockLocation API, e.g. S3, or preloading is disabled for its
+   * scheme). No blocks are recorded, but the encryption and erasure-coding flags come
+   * from 'fileStatus', as in create(), so getIsEncrypted()/getIsEc() are reliable on
+   * every load path.
    */
   public static FileDescriptor createWithNoBlocks(
       FileStatus fileStatus, String relPath, @Nullable String absPath) {
     FlatBufferBuilder fbb = new FlatBufferBuilder(1);
-    return new FileDescriptor(
-        createFbFileDesc(fbb, fileStatus, relPath, null, false, false, absPath));
+    return new FileDescriptor(createFbFileDesc(fbb, fileStatus, relPath, null,
+        fileStatus.isEncrypted(), fileStatus.isErasureCoded(), absPath));
   }
 
   /**
diff --git a/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java b/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
index 8f995231e..bf04d3655 100644
--- a/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
+++ b/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
@@ -99,6 +99,10 @@ public class FileSystemUtil {
 
   public static final String NO_ERASURE_CODE_LABEL = "NONE";
 
+  // "EC Policy" shown when it cannot be determined from the file descriptor, e.g. an
+  // Iceberg file loaded without block locations (IMPALA-15291).
+  public static final String UNKNOWN_ERASURE_CODE_LABEL = "N/A";
+
   /**
    * Set containing all FileSystem scheme that known to supports storage UUIDs in
    * BlockLocation calls.
diff --git a/fe/src/test/java/org/apache/impala/catalog/FeFsTableTest.java b/fe/src/test/java/org/apache/impala/catalog/FeFsTableTest.java
index cea9b8e18..eeb447ef3 100644
--- a/fe/src/test/java/org/apache/impala/catalog/FeFsTableTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/FeFsTableTest.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertEquals;
 import com.google.flatbuffers.FlatBufferBuilder;
 import java.nio.ByteBuffer;
 import java.util.Arrays;
+import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdfs.protocol.SystemErasureCodingPolicies;
 import org.apache.impala.common.FileSystemUtil;
@@ -41,6 +42,15 @@ public class FeFsTableTest {
     return new FileDescriptor(FbFileDesc.getRootAsFbFileDesc((ByteBuffer) copy.flip()));
   }
 
+  /** Builds a descriptor through createWithNoBlocks (the no-block-locations path). */
+  private static FileDescriptor noBlockFd(boolean isEc, boolean isEncrypted) {
+    FileStatus status = Mockito.mock(FileStatus.class);
+    Mockito.when(status.isErasureCoded()).thenReturn(isEc);
+    Mockito.when(status.isEncrypted()).thenReturn(isEncrypted);
+    Mockito.when(status.getPath()).thenReturn(new Path("/dummy/f"));
+    return FileDescriptor.createWithNoBlocks(status, "f", null);
+  }
+
   private static FeFsPartition mockPartition(FileDescriptor... fds) {
     FeFsPartition p = Mockito.mock(FeFsPartition.class);
     Mockito.when(p.getFileDescriptors()).thenReturn(Arrays.asList(fds));
@@ -76,4 +86,23 @@ public class FeFsTableTest {
     assertEquals(FeFsTable.MIXED_ERASURE_CODE_LABEL, FeFsTable.getErasureCodingPolicy(
         mockPartition(createFd(true, (byte) 0), createFd(false, (byte) 0))));
   }
+
+  /**
+   * IMPALA-15291: createWithNoBlocks records the erasure-coding and encryption flags,
+   * so the EC Policy column reflects them instead of always reporting NONE.
+   */
+  @Test
+  public void testGetErasureCodingPolicyNoBlockLocations() {
+    FileDescriptor ecFd = noBlockFd(true, false);
+    FileDescriptor plainFd = noBlockFd(false, false);
+    // The flags survive with no block locations loaded.
+    assertEquals(true, ecFd.getIsEc());
+    assertEquals(false, plainFd.getIsEc());
+    assertEquals(true, noBlockFd(false, true).getIsEncrypted());
+    // All-plain is NONE; a plain+EC mix is MIXED (the EC file is not seen as plain).
+    assertEquals(FileSystemUtil.NO_ERASURE_CODE_LABEL,
+        FeFsTable.getErasureCodingPolicy(mockPartition(plainFd, plainFd)));
+    assertEquals(FeFsTable.MIXED_ERASURE_CODE_LABEL,
+        FeFsTable.getErasureCodingPolicy(mockPartition(ecFd, plainFd)));
+  }
 }
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-insert.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-insert.test
index 2d4c672a6..3617c11f7 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-insert.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-insert.test
@@ -119,7 +119,7 @@ INT
 ---- QUERY
 show files in iceberg_hadoop_cat;
 ---- RESULTS
-row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/hadoop_catalog_test/$DATABASE/iceberg_hadoop_cat/data/.*.0.parq','.*','','$ERASURECODE_POLICY'
+row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/hadoop_catalog_test/$DATABASE/iceberg_hadoop_cat/data/.*.0.parq','.*','','$EXPECTED_ERASURECODE_POLICY'
 ---- TYPES
 STRING, STRING, STRING, STRING
 ====
@@ -146,7 +146,7 @@ INT
 ---- QUERY
 show files in iceberg_hadoop_cat_ti;
 ---- RESULTS
-row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/hadoop_catalog_test/test/custom_db/int_table/data/.*.0.parq','.*','','$ERASURECODE_POLICY'
+row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/hadoop_catalog_test/test/custom_db/int_table/data/.*.0.parq','.*','','$EXPECTED_ERASURECODE_POLICY'
 ---- TYPES
 STRING, STRING, STRING, STRING
 ====
@@ -194,7 +194,7 @@ INT
 ---- QUERY
 show files in iceberg_hive_cat_custom_loc;
 ---- RESULTS
-row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/custom_hive_cat/data/.*.0.parq','.*','','$ERASURECODE_POLICY'
+row_regex:'$NAMENODE/test-warehouse/$DATABASE.db/custom_hive_cat/data/.*.0.parq','.*','','$EXPECTED_ERASURECODE_POLICY'
 ---- TYPES
 STRING, STRING, STRING, STRING
 ====
diff --git a/tests/custom_cluster/test_disabled_block_locations.py b/tests/custom_cluster/test_disabled_block_locations.py
index 76331f546..2649b1624 100644
--- a/tests/custom_cluster/test_disabled_block_locations.py
+++ b/tests/custom_cluster/test_disabled_block_locations.py
@@ -50,7 +50,10 @@ class TestDisabledBlockLocations(CustomClusterTestSuite):
   def test_iceberg_smoke(self, vector, unique_database):
     self.run_test_case('QueryTest/iceberg-create', vector, unique_database)
     self.run_test_case('QueryTest/iceberg-external', vector, unique_database)
-    self.run_test_case('QueryTest/iceberg-insert', vector, unique_database)
+    # IMPALA-15291: without preloaded block locations Iceberg descriptors carry no
+    # erasure-coding info, so SHOW FILES reports 'N/A' for the EC policy.
+    self.run_test_case('QueryTest/iceberg-insert', vector, unique_database,
+        test_file_vars={'$EXPECTED_ERASURECODE_POLICY': 'N/A'})
     self.run_test_case('QueryTest/iceberg-delete', vector, unique_database)
     self.run_test_case('QueryTest/iceberg-delete-partitioned', vector, unique_database)
 
diff --git a/tests/query_test/test_iceberg.py b/tests/query_test/test_iceberg.py
index de7bddee7..abf5c78be 100644
--- a/tests/query_test/test_iceberg.py
+++ b/tests/query_test/test_iceberg.py
@@ -201,7 +201,10 @@ class TestIcebergTable(IcebergTestSuite):
                                       format(full_table_name))
 
   def test_insert(self, vector, unique_database):
-    self.run_test_case('QueryTest/iceberg-insert', vector, use_db=unique_database)
+    # With block locations preloaded the SHOW FILES EC policy is $ERASURECODE_POLICY;
+    self.run_test_case('QueryTest/iceberg-insert', vector, use_db=unique_database,
+        test_file_vars={'$EXPECTED_ERASURECODE_POLICY':
+                        os.getenv('ERASURECODE_POLICY', 'NONE')})
 
   def test_partitioned_insert_v1(self, vector, unique_database):
     self.run_test_case('QueryTest/iceberg-partitioned-insert-v1', vector,