czy006 commented on code in PR #4164:
URL: https://github.com/apache/amoro/pull/4164#discussion_r3091155243
##########
amoro-ams/src/main/java/org/apache/amoro/server/dashboard/MixedAndIcebergTableDescriptor.java:
##########
@@ -654,7 +655,46 @@ public List<ConsumerInfo>
getTableConsumerInfos(AmoroTable<?> amoroTable) {
}
@Override
- public Pair<List<OptimizingProcessInfo>, Integer> getOptimizingProcessesInfo(
+ public void createTag(
+ AmoroTable<?> amoroTable, String tagName, long snapshotId, Long
maxRefAgeMs) {
+ MixedTable mixedTable = getTable(amoroTable);
+ Preconditions.checkArgument(
+ !mixedTable.isKeyedTable(), "Creating tags on KeyedTable is not
supported yet");
+ Table icebergTable = mixedTable.asUnkeyedTable();
+ TableOperations ops = ((HasTableOperations) icebergTable).operations();
+ TableMetadata base = ops.refresh();
+ Preconditions.checkNotNull(base, "Table metadata is null");
+ Preconditions.checkNotNull(
+ base.snapshot(snapshotId), "Snapshot %s not found in table",
snapshotId);
+
+ TableMetadata.Builder builder = TableMetadata.buildFrom(base);
+ SnapshotRef.Builder refBuilder = SnapshotRef.tagBuilder(snapshotId);
+ if (maxRefAgeMs != null && maxRefAgeMs > 0) {
+ refBuilder.maxRefAgeMs(maxRefAgeMs);
+ }
+ builder.setRef(tagName, refBuilder.build());
+ TableMetadata updated = builder.build();
+ ops.commit(base, updated);
+ }
+
+ @Override
+ public void deleteTag(AmoroTable<?> amoroTable, String tagName) {
+ MixedTable mixedTable = getTable(amoroTable);
+ Preconditions.checkArgument(
+ !mixedTable.isKeyedTable(), "Deleting tags on KeyedTable is not
supported yet");
+ Table icebergTable = mixedTable.asUnkeyedTable();
+ TableOperations ops = ((HasTableOperations) icebergTable).operations();
+ TableMetadata base = ops.refresh();
+ Preconditions.checkNotNull(base, "Table metadata is null");
+ SnapshotRef existingRef = base.ref(tagName);
+ Preconditions.checkArgument(
+ existingRef != null && existingRef.isTag(), "Tag %s not found in
table", tagName);
+
+ TableMetadata.Builder builder = TableMetadata.buildFrom(base);
+ builder.removeRef(tagName);
+ TableMetadata updated = builder.build();
+ ops.commit(base, updated);
+ }
AmoroTable<?> amoroTable, String type, ProcessStatus status, int limit,
int offset) {
Review Comment:
[P0] Broken method signature causes compilation failure.\n\nReason summary:
right after `deleteTag`, the method declaration for
`getOptimizingProcessesInfo(...)` is missing, so this file becomes
syntactically invalid (`; expected`) and `amoro-ams` cannot build.\n\nSuggested
fix: restore the full method signature (`@Override`, return type, method name,
params) and rerun `./mvnw -pl amoro-ams -am -DskipTests -Pskip-dashboard-build
compile` to verify.
##########
amoro-ams/src/main/java/org/apache/amoro/server/dashboard/MixedAndIcebergTableDescriptor.java:
##########
@@ -654,7 +655,46 @@ public List<ConsumerInfo>
getTableConsumerInfos(AmoroTable<?> amoroTable) {
}
@Override
- public Pair<List<OptimizingProcessInfo>, Integer> getOptimizingProcessesInfo(
+ public void createTag(
+ AmoroTable<?> amoroTable, String tagName, long snapshotId, Long
maxRefAgeMs) {
+ MixedTable mixedTable = getTable(amoroTable);
+ Preconditions.checkArgument(
+ !mixedTable.isKeyedTable(), "Creating tags on KeyedTable is not
supported yet");
+ Table icebergTable = mixedTable.asUnkeyedTable();
+ TableOperations ops = ((HasTableOperations) icebergTable).operations();
+ TableMetadata base = ops.refresh();
+ Preconditions.checkNotNull(base, "Table metadata is null");
+ Preconditions.checkNotNull(
+ base.snapshot(snapshotId), "Snapshot %s not found in table",
snapshotId);
+
+ TableMetadata.Builder builder = TableMetadata.buildFrom(base);
Review Comment:
[P1] `createTag` currently uses `setRef` without guarding name
conflicts.\n\nReason summary: `builder.setRef(tagName, ...)` can overwrite an
existing ref with the same name, which is riskier than Iceberg `createTag`
semantics (create-only, fail on existing ref).\n\nSuggested fix: add a
pre-check such as `Preconditions.checkArgument(base.ref(tagName) == null, ...)`
before setting the ref, or switch to an Iceberg create-tag path that enforces
non-overwrite behavior.
--
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]
