luoyuxia commented on code in PR #1485:
URL: https://github.com/apache/fluss/pull/1485#discussion_r2265773870
##########
fluss-client/src/main/java/com/alibaba/fluss/client/metadata/LakeSnapshot.java:
##########
@@ -36,9 +36,17 @@ public class LakeSnapshot {
// the specific log offset of the snapshot
private final Map<TableBucket, Long> tableBucketsOffset;
- public LakeSnapshot(long snapshotId, Map<TableBucket, Long>
tableBucketsOffset) {
+ // the partition name by partition id of this lake snapshot if
+ // is a partitioned table, empty if not a partitioned table
+ private final Map<Long, String> partitionNameById;
Review Comment:
If wrap `partition name` in `tableBucketsOffset`, the `tableBucketsOffset`
may look like `Map<Tuple2<TableBucket, String>, Long> tableBucketsOffset`, then
in method
`Map<TableBucket, Long> getTableBucketsOffset` and `Map<Long, String>
getPartitionNameById` we will need to convert it into `Map<TableBucket, Long>
`, `Map<Long, String> ` which I think a little of heavy.
So, I tend to keep the duplication, and I think the duplication is
acceptable, won't bring much memory consuming.
Maybe in the future, we can consider another refactor to make `TableBucket`
contains the partition name to simpify the caller logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
