Repository: apache/amoro
Pull request #3071 (2024-07-30 05:43:57)

[AMORO-3084] Upgrade to iceberg 1.6.1
<!--
Thanks for sending a pull request!

Here are some tips for you:
  1. If this is your first time, please read our contributor guidelines: https://amoro.apache.org/how-to-contribute/
  2. If the PR is related to an issue in https://github.com/apache/amoro/issues, add '[AMORO-XXXX]' in your PR title, e.g., '[AMORO-XXXX] Your PR title ...'.
  3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][AMORO-XXXX] Your PR title ...'.
-->

## Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
  1. If you add a feature, you can talk about its use case.
  2. If you fix a bug, you can clarify why it is a bug.
  3. Use Fix/Resolve/Close #{ISSUE_NUMBER} to link this PR to its related issue
-->

#3084.

## Brief change log
<!--
Clearly describe the changes made in modules, classes, methods, etc.
-->

- Upgrade iceberg to 1.6.X
- Mixed Format iceberg version keep to 1.4.3


## How was this patch tested?

- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [x] Add screenshots for manual tests if appropriate
- [x] Run test locally before making a pull request

## Documentation

- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (not documented)


CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/NetEase/amoro?pullRequest=3071) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/NetEase/amoro?pullRequest=3071) before we can accept your contribution.<br/><hr/>**ConradJam** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/NetEase/amoro?pullRequest=3071) it.</sub>
czy006: - https://github.com/apache/iceberg/blob/main/flink/v1.18/flink/src/main/java/org/apache/iceberg/flink/source/ScanContext.java#L161
- https://github.com/apache/iceberg/blob/apache-iceberg-1.4.3/flink/v1.15/flink/src/main/java/org/apache/iceberg/flink/source/ScanContext.java#L149

Diff,Run TestFlinkSource.testUnkeyedSnapshotRead Have Exception
![image](https://github.com/user-attachments/assets/09c144d4-817b-45dc-b013-df90a5de8dc2)


codecov-commenter: ## [Codecov](https://app.codecov.io/gh/apache/amoro/pull/3071?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 23.17%. Comparing base [(`9def89f`)](https://app.codecov.io/gh/apache/amoro/commit/9def89f0b005d59850bce17b5d255f1bb246d02e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`47781b0`)](https://app.codecov.io/gh/apache/amoro/commit/47781b060229589d246ddd3367faf9240d9df695?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 8 commits behind head on master.

| [Files](https://app.codecov.io/gh/apache/amoro/pull/3071?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...java/org/apache/amoro/table/BasicUnkeyedTable.java](https://app.codecov.io/gh/apache/amoro/pull/3071?src=pr&el=tree&filepath=amoro-iceberg-format%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Famoro%2Ftable%2FBasicUnkeyedTable.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YW1vcm8taWNlYmVyZy1mb3JtYXQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Ftb3JvL3RhYmxlL0Jhc2ljVW5rZXllZFRhYmxlLmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/amoro/pull/3071?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff              @@
##             master    #3071      +/-   ##
============================================
- Coverage     27.64%   23.17%   -4.47%     
+ Complexity     2496     2330     -166     
============================================
  Files           360      368       +8     
  Lines         37133    37914     +781     
  Branches       5372     5456      +84     
============================================
- Hits          10265     8787    -1478     
- Misses        25908    28353    +2445     
+ Partials        960      774     -186     
```

| [Flag](https://app.codecov.io/gh/apache/amoro/pull/3071/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [trino](https://app.codecov.io/gh/apache/amoro/pull/3071/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `23.17% <0.00%> (-4.47%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/amoro/pull/3071?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).

czy006: ops,some time ci was blocker have long time.Maybe we should check
baiyangtx: It looks like there will be many engine versions that will no longer be supported.

@zhoujinsong PTAL
zhoujinsong: Maybe we should follow Iceberg 1.6, support:

- Flink 1.17/1.18/1.19
- Spark 3.3/3.4/3.5

What do you think?
czy006: > Details

This will be supported in the future PR, Flink 1.20 as the LTS long-term support version, I recommend as the default version option
czy006: > > Details
> 
> This will be supported in the future PR, Flink 1.20 as the LTS long-term support version, I recommend as the default version option



> Maybe we should follow Iceberg 1.6, support:
> 
> * Flink 1.17/1.18/1.19
> * Spark 3.3/3.4/3.5
> 
> What do you think?

This will be support in the future PR, Flink 1.20 as the LTS long-term support version, I recommend as the default version option
czy006: ![image](https://github.com/user-attachments/assets/8e693d2b-adff-42f2-9989-a3a79f65aa66)

Optimizing Task Run Normal
zhongqishang: I think it is not a good idea to remove so many versions support. 
What is the reason for deleting the support? Is it just that the official does not support it?

Maybe we can keep the combination of old flink and old iceberg versions. @zhoujinsong @baiyangtx @czy006 


czy006: > I think it is not a good idea to remove so many versions support. What is the reason for deleting the support? Is it just that the official does not support it?
> 
> Maybe we can keep the combination of old flink and old iceberg versions. @zhoujinsong @baiyangtx @czy006

mixed format relies on the spark and flink versions, and currently the iceberg community only supports the latest 3 versions. For example, they have to give up [spark 3.2](https://mvnrepository.com/artifact/org.apache.iceberg/iceberg-spark-runtime-3.2) their support.the last support in 1.4.3, iceberg if we upgrade version, we must give up its support it.flink version seems(I will add flink1.19 1.20 in the next pr),Spark now just support 3.3
czy006: I have initiated relevant discussions on the email list:
https://lists.apache.org/thread/1crbstg7prjxz8xokd7dtj1o5mgv1034
github-actions[bot]: This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@amoro.apache.org list. Thank you for your contributions.
github-actions[bot]: This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
czy006: cc @zhoujinsong 

Patch:
diff --git a/amoro-ams/pom.xml b/amoro-ams/pom.xml
index b46859a22..ebc862e40 100644
--- a/amoro-ams/pom.xml
+++ b/amoro-ams/pom.xml
@@ -200,6 +200,12 @@
             <artifactId>derby</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>mysql</groupId>
+            <artifactId>mysql-connector-java</artifactId>
+            <scope>provided</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-dbcp2</artifactId>
@@ -397,13 +403,6 @@
             <version>${pagehelper.version}</version>
         </dependency>
 
-        <dependency>
-            <groupId>com.mysql</groupId>
-            <artifactId>mysql-connector-j</artifactId>
-            <version>${mysql-connector-j.version}</version>
-            <scope>${mysql-j-dependency-scope}</scope>
-        </dependency>
-
         <dependency>
             <groupId>org.apache.iceberg</groupId>
             <artifactId>iceberg-data</artifactId>
@@ -452,7 +451,8 @@
             <version>1.19.6</version>
             <scope>test</scope>
         </dependency>
-        
+
+
     </dependencies>
 
     <build>
diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/optimizing/IcebergRewriteExecutor.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/optimizing/IcebergRewriteExecutor.java
index fa355a8bf..d17222172 100644
--- a/amoro-format-iceberg/src/main/java/org/apache/amoro/optimizing/IcebergRewriteExecutor.java
+++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/optimizing/IcebergRewriteExecutor.java
@@ -24,7 +24,6 @@ import org.apache.amoro.io.writer.IcebergFanoutPosDeleteWriter;
 import org.apache.amoro.table.MixedTable;
 import org.apache.amoro.utils.map.StructLikeCollections;
 import org.apache.iceberg.PartitionSpec;
-import org.apache.iceberg.StructLike;
 import org.apache.iceberg.TableProperties;
 import org.apache.iceberg.data.GenericAppenderFactory;
 import org.apache.iceberg.data.IdentityPartitionConverters;
@@ -46,18 +45,6 @@ public class IcebergRewriteExecutor extends AbstractRewriteFilesExecutor {
     super(input, table, structLikeCollections);
   }
 
-  // TODO We can remove this override method after upgrading Iceberg version to 1.5+.
-  @Override
-  protected StructLike partition() {
-    StructLike partitionData = super.partition();
-    if (partitionData != null && partitionData.size() == 0) {
-      // Cast empty partition data to NULL to avoid creating empty partition directory.
-      return null;
-    } else {
-      return partitionData;
-    }
-  }
-
   @Override
   protected OptimizingDataReader dataReader() {
     return new GenericCombinedIcebergDataReader(
diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/table/BasicUnkeyedTable.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/table/BasicUnkeyedTable.java
index d5f6f878d..6c5cceee6 100644
--- a/amoro-format-iceberg/src/main/java/org/apache/amoro/table/BasicUnkeyedTable.java
+++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/table/BasicUnkeyedTable.java
@@ -65,6 +65,7 @@ import org.apache.iceberg.util.StructLikeMap;
 
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 
 /** Basic implementation of {@link UnkeyedTable}, wrapping a {@link Table}. */
 public class BasicUnkeyedTable implements UnkeyedTable, HasTableOperations {
@@ -318,4 +319,9 @@ public class BasicUnkeyedTable implements UnkeyedTable, HasTableOperations {
   public UpdatePartitionProperties updatePartitionProperties(Transaction transaction) {
     return new PartitionPropertiesUpdate(this, transaction);
   }
+
+  @Override
+  public UUID uuid() {
+    return UUID.fromString(this.operations().current().uuid());
+  }
 }
diff --git a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/pom.xml b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/pom.xml
index 41a51b9d8..b173af469 100644
--- a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/pom.xml
+++ b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/pom.xml
@@ -34,10 +34,11 @@
     <packaging>jar</packaging>
 
     <properties>
+        <iceberg.version>1.4.3</iceberg.version>
         <kafka.version>3.2.3</kafka.version>
         <assertj.version>3.21.0</assertj.version>
         <testcontainers.version>1.17.2</testcontainers.version>
-        <flink.version>1.17.1</flink.version>
+        <flink.version>1.17.2</flink.version>
     </properties>
 
     <dependencies>
diff --git a/amoro-format-mixed/amoro-mixed-flink/v1.15/amoro-mixed-flink-1.15/pom.xml b/amoro-format-mixed/amoro-mixed-flink/v1.15/amoro-mixed-flink-1.15/pom.xml
index 0f9202e68..223f8e795 100644
--- a/amoro-format-mixed/amoro-mixed-flink/v1.15/amoro-mixed-flink-1.15/pom.xml
+++ b/amoro-format-mixed/amoro-mixed-flink/v1.15/amoro-mixed-flink-1.15/pom.xml
@@ -34,6 +34,7 @@
     <packaging>jar</packaging>
 
     <properties>
+        <iceberg.version>1.4.3</iceberg.version>
         <kafka.version>2.8.1</kafka.version>
         <assertj.version>3.21.0</assertj.version>
         <testcontainers.version>1.17.2</testcontainers.version>
diff --git a/amoro-format-mixed/amoro-mixed-flink/v1.16/amoro-mixed-flink-1.16/pom.xml b/amoro-format-mixed/amoro-mixed-flink/v1.16/amoro-mixed-flink-1.16/pom.xml
index cee742267..9ccfd0a6b 100644
--- a/amoro-format-mixed/amoro-mixed-flink/v1.16/amoro-mixed-flink-1.16/pom.xml
+++ b/amoro-format-mixed/amoro-mixed-flink/v1.16/amoro-mixed-flink-1.16/pom.xml
@@ -34,10 +34,11 @@
     <packaging>jar</packaging>
 
     <properties>
+        <iceberg.version>1.4.3</iceberg.version>
         <kafka.version>3.2.3</kafka.version>
         <assertj.version>3.21.0</assertj.version>
         <testcontainers.version>1.17.2</testcontainers.version>
-        <flink.version>1.16.2</flink.version>
+        <flink.version>1.16.3</flink.version>
     </properties>
 
     <dependencies>
diff --git a/amoro-format-mixed/amoro-mixed-flink/v1.17/amoro-mixed-flink-1.17/pom.xml b/amoro-format-mixed/amoro-mixed-flink/v1.17/amoro-mixed-flink-1.17/pom.xml
index 5fd713cca..669b76b5b 100644
--- a/amoro-format-mixed/amoro-mixed-flink/v1.17/amoro-mixed-flink-1.17/pom.xml
+++ b/amoro-format-mixed/amoro-mixed-flink/v1.17/amoro-mixed-flink-1.17/pom.xml
@@ -34,10 +34,11 @@
     <packaging>jar</packaging>
 
     <properties>
+        <iceberg.version>1.4.3</iceberg.version>
         <kafka.version>3.2.3</kafka.version>
         <assertj.version>3.21.0</assertj.version>
         <testcontainers.version>1.17.2</testcontainers.version>
-        <flink.version>1.17.1</flink.version>
+        <flink.version>1.17.2</flink.version>
     </properties>
 
     <dependencies>
diff --git a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/pom.xml b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/pom.xml
index dd4fa0995..e3ca63138 100644
--- a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/pom.xml
+++ b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/pom.xml
@@ -33,6 +33,7 @@
     <url>https://amoro.apache.org</url>
 
     <properties>
+        <iceberg.version>1.4.3</iceberg.version>
         <spark.version>3.2.4</spark.version>
         <scala.version>2.12.15</scala.version>
     </properties>
@@ -144,7 +145,7 @@
 
         <dependency>
             <groupId>org.apache.iceberg</groupId>
-            <artifactId>iceberg-spark-3.2_2.12</artifactId>
+            <artifactId>iceberg-spark-3.3_2.12</artifactId>
             <version>${iceberg.version}</version>
             <scope>provided</scope>
             <exclusions>
diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/pom.xml b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/pom.xml
index cc4e7a8d3..e5d5cf667 100644
--- a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/pom.xml
+++ b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/pom.xml
@@ -34,6 +34,7 @@
     <url>https://amoro.apache.org</url>
 
     <properties>
+        <iceberg.version>1.4.3</iceberg.version>
         <hive.version>2.3.9</hive.version>
         <spark.version>3.2.4</spark.version>
         <scala.version>2.12.15</scala.version>
diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/pom.xml b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/pom.xml
index 8d6cbff8b..2dd91e9ba 100644
--- a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/pom.xml
+++ b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/pom.xml
@@ -34,6 +34,7 @@
     <url>https://amoro.apache.org</url>
 
     <properties>
+        <iceberg.version>1.4.3</iceberg.version>
         <hive.version>2.3.9</hive.version>
         <spark.version>3.3.2</spark.version>
         <scala.version>2.12.15</scala.version>
diff --git a/pom.xml b/pom.xml
index 130552a4a..458481f38 100644
--- a/pom.xml
+++ b/pom.xml
@@ -101,7 +101,7 @@
         <maven-clean-plugin.version>3.3.2</maven-clean-plugin.version>
         <maven-checkstyle-plugin.version>3.3.1</maven-checkstyle-plugin.version>
 
-        <iceberg.version>1.4.3</iceberg.version>
+        <iceberg.version>1.6.1</iceberg.version>
         <paimon.version>0.9.0</paimon.version>
         <hive.version>3.1.1</hive.version>
         <hadoop.version>3.4.0</hadoop.version>
@@ -151,12 +151,10 @@
         <guava.version>32.1.1-jre</guava.version>
         <hudi.version>0.14.1</hudi.version>
         <pagehelper.version>6.1.0</pagehelper.version>
-        <mysql-connector-j.version>9.1.0</mysql-connector-j.version>
 
         <rocksdb-dependency-scope>compile</rocksdb-dependency-scope>
         <lucene-dependency-scope>compile</lucene-dependency-scope>
         <aliyun-sdk-dependency-scope>provided</aliyun-sdk-dependency-scope>
-        <mysql-j-dependency-scope>provided</mysql-j-dependency-scope>
     </properties>
 
     <dependencies>
@@ -628,6 +626,12 @@
                 <version>${derby-jdbc.version}</version>
             </dependency>
 
+            <dependency>
+                <groupId>mysql</groupId>
+                <artifactId>mysql-connector-java</artifactId>
+                <version>${mysql-jdbc.version}</version>
+            </dependency>
+
             <dependency>
                 <groupId>org.apache.commons</groupId>
                 <artifactId>commons-dbcp2</artifactId>
@@ -1356,8 +1360,8 @@
                 <guava.version>14.0.1</guava.version>
                 <hive.version>2.3.9</hive.version>
                 <hadoop.version>2.10.2</hadoop.version>
-                <terminal.spark.version>3.2.4</terminal.spark.version>
-                <terminal.spark.major.version>3.2</terminal.spark.major.version>
+                <terminal.spark.version>3.3.3</terminal.spark.version>
+                <terminal.spark.major.version>3.3</terminal.spark.major.version>
             </properties>
         </profile>
         <profile>
@@ -1373,11 +1377,5 @@
                 <aliyun-sdk-dependency-scope>compile</aliyun-sdk-dependency-scope>
             </properties>
         </profile>
-        <profile>
-            <id>mysql-j</id>
-            <properties>
-                <mysql-j-dependency-scope>compile</mysql-j-dependency-scope>
-            </properties>
-        </profile>
     </profiles>
 </project>


Test patch:
diff --git a/amoro-format-iceberg/src/test/java/org/apache/iceberg/TestIcebergFindFiles.java b/amoro-format-iceberg/src/test/java/org/apache/iceberg/TestIcebergFindFiles.java
index bd674c7d3..e7fa4a37e 100644
--- a/amoro-format-iceberg/src/test/java/org/apache/iceberg/TestIcebergFindFiles.java
+++ b/amoro-format-iceberg/src/test/java/org/apache/iceberg/TestIcebergFindFiles.java
@@ -25,24 +25,44 @@ import org.apache.amoro.shade.guava32.com.google.common.collect.Sets;
 import org.apache.iceberg.expressions.Expressions;
 import org.apache.iceberg.types.Conversions;
 import org.apache.iceberg.types.Types;
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
 import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
 
 @RunWith(Parameterized.class)
-public class TestIcebergFindFiles extends TableTestBase {
+public class TestIcebergFindFiles extends TestBase {
+
   @Parameterized.Parameters(name = "formatVersion = {0}")
-  public static Object[] parameters() {
-    return new Object[] {1, 2};
+  public static List<Object> parameters() {
+    return Arrays.asList(1, 2);
   }
 
+  @Rule public TemporaryFolder tempDir = new TemporaryFolder();
+
   public TestIcebergFindFiles(int formatVersion) {
-    super(formatVersion);
+    this.formatVersion = formatVersion;
+  }
+
+  @Before
+  public void setUp() throws Exception {
+    this.tableDir = tempDir.newFolder();
+    this.tableDir.delete();
+    super.setupTable();
+  }
+
+  @After
+  public void cleanUp() throws Exception {
+    super.cleanupTables();
   }
 
   @Test