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

blue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-iceberg.git


The following commit(s) were added to refs/heads/master by this push:
     new 80e97a5  Fix classloader used for metadata files (#898)
80e97a5 is described below

commit 80e97a5367a7ad2f0fc0ea57fe9828f771427845
Author: Ryan Blue <[email protected]>
AuthorDate: Tue Apr 7 10:36:51 2020 -0700

    Fix classloader used for metadata files (#898)
    
    By default, the Avro read helper uses the current thread's classloader. In 
some cases, this may not be able to load Iceberg classes so internal Avro reads 
should explicitly use the right classloader to avoid ClassCastException.
---
 core/src/main/java/org/apache/iceberg/AllManifestsTable.java     | 1 +
 core/src/main/java/org/apache/iceberg/BaseSnapshot.java          | 1 +
 core/src/main/java/org/apache/iceberg/ManifestReader.java        | 1 +
 core/src/main/java/org/apache/iceberg/RemoveSnapshots.java       | 1 +
 core/src/main/java/org/apache/iceberg/avro/Avro.java             | 9 +++++++--
 .../test/java/org/apache/iceberg/TestGenericManifestFile.java    | 1 +
 6 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/iceberg/AllManifestsTable.java 
b/core/src/main/java/org/apache/iceberg/AllManifestsTable.java
index 93b4396..1a600a6 100644
--- a/core/src/main/java/org/apache/iceberg/AllManifestsTable.java
+++ b/core/src/main/java/org/apache/iceberg/AllManifestsTable.java
@@ -167,6 +167,7 @@ public class AllManifestsTable extends BaseMetadataTable {
           .rename("partitions", GenericPartitionFieldSummary.class.getName())
           .rename("r508", GenericPartitionFieldSummary.class.getName())
           .project(ManifestFile.schema())
+          .classLoader(GenericManifestFile.class.getClassLoader())
           .reuseContainers(false)
           .build()) {
 
diff --git a/core/src/main/java/org/apache/iceberg/BaseSnapshot.java 
b/core/src/main/java/org/apache/iceberg/BaseSnapshot.java
index 6f28961..2c82d8b 100644
--- a/core/src/main/java/org/apache/iceberg/BaseSnapshot.java
+++ b/core/src/main/java/org/apache/iceberg/BaseSnapshot.java
@@ -119,6 +119,7 @@ class BaseSnapshot implements Snapshot {
           .rename("manifest_file", GenericManifestFile.class.getName())
           .rename("partitions", GenericPartitionFieldSummary.class.getName())
           .rename("r508", GenericPartitionFieldSummary.class.getName())
+          .classLoader(GenericManifestFile.class.getClassLoader())
           .project(ManifestFile.schema())
           .reuseContainers(false)
           .build()) {
diff --git a/core/src/main/java/org/apache/iceberg/ManifestReader.java 
b/core/src/main/java/org/apache/iceberg/ManifestReader.java
index 1ae8e3b..886eb77 100644
--- a/core/src/main/java/org/apache/iceberg/ManifestReader.java
+++ b/core/src/main/java/org/apache/iceberg/ManifestReader.java
@@ -252,6 +252,7 @@ public class ManifestReader extends CloseableGroup 
implements Filterable<Filtere
             .rename("r102", PartitionData.class.getName())
             .rename("data_file", GenericDataFile.class.getName())
             .rename("r2", GenericDataFile.class.getName())
+            .classLoader(ManifestEntry.class.getClassLoader())
             .reuseContainers()
             .build();
 
diff --git a/core/src/main/java/org/apache/iceberg/RemoveSnapshots.java 
b/core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
index 76ee31d..6ba2850 100644
--- a/core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
+++ b/core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
@@ -380,6 +380,7 @@ class RemoveSnapshots implements ExpireSnapshots {
     if (snapshot.manifestListLocation() != null) {
       return Avro.read(ops.io().newInputFile(snapshot.manifestListLocation()))
           .rename("manifest_file", GenericManifestFile.class.getName())
+          .classLoader(GenericManifestFile.class.getClassLoader())
           .project(MANIFEST_PROJECTION)
           .reuseContainers(true)
           .build();
diff --git a/core/src/main/java/org/apache/iceberg/avro/Avro.java 
b/core/src/main/java/org/apache/iceberg/avro/Avro.java
index 3fbf6ed..40c43cb 100644
--- a/core/src/main/java/org/apache/iceberg/avro/Avro.java
+++ b/core/src/main/java/org/apache/iceberg/avro/Avro.java
@@ -169,9 +169,9 @@ public class Avro {
   }
 
   public static class ReadBuilder {
-    private final ClassLoader defaultLoader = 
Thread.currentThread().getContextClassLoader();
     private final InputFile file;
     private final Map<String, String> renames = Maps.newLinkedHashMap();
+    private ClassLoader loader = 
Thread.currentThread().getContextClassLoader();
     private NameMapping nameMapping;
     private boolean reuseContainers = false;
     private org.apache.iceberg.Schema schema = null;
@@ -179,7 +179,7 @@ public class Avro {
     private BiFunction<org.apache.iceberg.Schema, Schema, DatumReader<?>> 
createReaderBiFunc = null;
     private final Function<Schema, DatumReader<?>> defaultCreateReaderFunc = 
readSchema -> {
       GenericAvroReader<?> reader = new GenericAvroReader<>(readSchema);
-      reader.setClassLoader(defaultLoader);
+      reader.setClassLoader(loader);
       return reader;
     };
     private Long start = null;
@@ -240,6 +240,11 @@ public class Avro {
       return this;
     }
 
+    public ReadBuilder classLoader(ClassLoader classLoader) {
+      this.loader = classLoader;
+      return this;
+    }
+
     public <D> AvroIterable<D> build() {
       Preconditions.checkNotNull(schema, "Schema is required");
       Function<Schema, DatumReader<?>> readerFunc;
diff --git a/core/src/test/java/org/apache/iceberg/TestGenericManifestFile.java 
b/core/src/test/java/org/apache/iceberg/TestGenericManifestFile.java
index f50b850..eaceedf 100644
--- a/core/src/test/java/org/apache/iceberg/TestGenericManifestFile.java
+++ b/core/src/test/java/org/apache/iceberg/TestGenericManifestFile.java
@@ -68,6 +68,7 @@ public class TestGenericManifestFile {
         .rename("manifest_file", GenericManifestFile.class.getName())
         .rename("partitions", GenericPartitionFieldSummary.class.getName())
         .rename("r508", GenericPartitionFieldSummary.class.getName())
+        .classLoader(GenericManifestFile.class.getClassLoader())
         .project(ManifestFile.schema())
         .reuseContainers(false)
         .build()) {

Reply via email to