Skip to content

Conversation

ygjia
Copy link

@ygjia ygjia commented Jul 30, 2025

Why are the changes needed?

Close #3697

Brief change log

  • change flink version to 1.18.1 in amoro-mixed-flink-common
  • add amoro-format-mixed-flink-1.18 and amoro-format-mixed-flink-runtime-1.18

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • 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? related mixed format docs need to be updated

@github-actions github-actions bot added module:mixed-flink Flink moduel for Mixed Format type:build labels Jul 30, 2025
@zhoujinsong
Copy link
Contributor

zhoujinsong commented Aug 19, 2025

@ygjia
Copy link
Author

ygjia commented Aug 20, 2025

Hi, you may need to change the access modifier of validate method in https://github.com/apache/amoro/blob/master/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common-iceberg-bridge/src/main/java/org/apache/iceberg/flink/source/ScanContext.java#L236 to package-private, according the change in Iceberg repo:https://github.com/apache/iceberg/blob/apache-iceberg-1.6.1/flink/v1.18/flink/src/main/java/org/apache/iceberg/flink/source/ScanContext.java#L134

Thanks for the review!
The Iceberg-ScanContext in Iceberg 1.6.1 has changed compared to Amoro-ScanContext, which caused several UTs to fail before.

org.apache.amoro.flink.table.TestUnkeyed
org.apache.amoro.flink.read.TestFlinkSource
org.apache.amoro.flink.table.TestLookupSecondary
org.apache.amoro.flink.table.TestUnkeyedOverwrite

I have aligned it with Iceberg 1.6.1, and above UTs passed locally.

@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.48%. Comparing base (e3c16af) to head (8650606).

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3700      +/-   ##
============================================
- Coverage     28.53%   28.48%   -0.06%     
+ Complexity     3759     3756       -3     
============================================
  Files           617      617              
  Lines         50056    49976      -80     
  Branches       6482     6440      -42     
============================================
- Hits          14283    14234      -49     
+ Misses        34760    34729      -31     
  Partials       1013     1013              
Flag Coverage Δ
core 28.48% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:mixed-flink Flink moduel for Mixed Format type:build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: mixed-format table support Flink 1.18
3 participants