[AMORO-1940] Support list partitions via flink catalog #1994
Conversation
Codecov ReportAll modified lines are covered by tests ✅ see 50 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
huyuanfeng2018
force-pushed
the
list_partition
branch
from
3a1068e to
4f32289
Compare
|
@huyuanfeng2018 Thanks for your contribution. The CI failed due to the checkstyle. Could you take a look? |
done. |
| List<Object[]> data = new LinkedList<>(); | ||
| data.add(new Object[]{1, "mark", "2023-10-01"}); | ||
| data.add(new Object[]{2, "Gerry", "2023-10-02"}); | ||
| data.add(new Object[]{RowKind.DELETE, 2, "Gerry", "2023-10-02"}); |
There was a problem hiding this comment.
Like this, first insert a piece of data with ID 2 and then delete it. ListPartition cannot determine whether the partition 2023-10-02 really exists.
@YesOrNo828
WDYT?
There was a problem hiding this comment.
@huyuanfeng2018 I don't think there's an excellent way to return the data's partitions. Additionally, the issue is present in the iceberg v2 table.
Perhaps we can add a comment to the method to clarify that the partitions returned in this scenario encompass all the partitions related to writing the data, including -D and -U.
cc @zhoujinsong WDYT?
There was a problem hiding this comment.
I think it's okay to return this partition in this situation.
| tables.add(arcticTable.asUnkeyedTable()); | ||
| } | ||
| for (Table table : tables) { | ||
| try (CloseableIterable<FileScanTask> tasks = table.newScan().planFiles()) { |
There was a problem hiding this comment.
Mixed table Files is not equal change files + base files. you can use mixed plan
There was a problem hiding this comment.
Mixed table Files is not equal change files + base files. you can use mixed plan
I see that the logic in ams should be calculated using change files + base files.
https://sourcegraph.com/github.com/NetEase/amoro/-/blob/ams/server/src/main/java/com/netease/arctic/server/dashboard/ServerTableDescriptor.java?L273:37&popover=pinned
There was a problem hiding this comment.
I had talked with the auth of ServerTableDescriptor. The logic is error.
There was a problem hiding this comment.
You can use KeyedTable#newScan to plan files
There was a problem hiding this comment.
You can use KeyedTable#newScan to plan files
It seems cannot get partition or file information through this.
There is no planfiles method, and plantask cannot get file and partition information.
There was a problem hiding this comment.
planTasks() -> KeyedTableScanTask -> ArcticFileScanTask. ArcticFileScanTask is a FileScanTask.
or
planTasks() -> KeyedTableScanTask -> ArcticFileScanTask -> PrimaryKeyedFile. PrimaryKeyedFile is a ContentFile
There was a problem hiding this comment.
planTasks() -> KeyedTableScanTask -> ArcticFileScanTask. ArcticFileScanTask is a FileScanTask. or planTasks() -> KeyedTableScanTask -> ArcticFileScanTask -> PrimaryKeyedFile. PrimaryKeyedFile is a ContentFile
done
huyuanfeng2018
force-pushed
the
list_partition
branch
from
03c5da9 to
41887c0
Compare
huyuanfeng2018
force-pushed
the
list_partition
branch
from
2c96039 to
63eced9
Compare
|
@huyuanfeng2018 Thanks for your contribution. I find that Would you like to open new issues to complete? |
Okay, I will open a new issue to complete it |
* ArcticCatalog support listPartitions * add test case * checkstyle * resolve conflicts for flink1.12 * fix * spotless apply --------- Co-authored-by: huyuanfeng <[email protected]>
Why are the changes needed?
Close #1940
Brief change log
listpartitionsmethod inArcticCatalogHow 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