jerryshao commented on code in PR #7037:
URL: https://github.com/apache/gravitino/pull/7037#discussion_r2062825571
##########
catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java:
##########
@@ -429,6 +429,11 @@ private ModelVersionEntity updateModelVersionEntity(
ModelVersionChange.UpdateUri updateUriChange =
(ModelVersionChange.UpdateUri) change;
entityUri = updateUriChange.newUri();
+ } else if (change instanceof ModelVersionChange.UpdateAlias) {
+ ModelVersionChange.UpdateAlias updateAliasChange =
(ModelVersionChange.UpdateAlias) change;
+ doDeleteAlias(entityAliases, updateAliasChange);
+ doSetAlias(entityAliases, updateAliasChange);
Review Comment:
I think you should think of a scenario where the alias exists in both added
list and delete list, for example, `aliasesToAdd` is `{"alias1", "alias2"}`,
and `aliasesToDelete` is `{"alias2", "alias3"}`. This "alias2" should not be
added to the model version.
##########
api/src/main/java/org/apache/gravitino/model/ModelVersionChange.java:
##########
@@ -71,6 +80,19 @@ static ModelVersionChange updateUri(String newUri) {
return new ModelVersionChange.UpdateUri(newUri);
}
+ /**
+ * Create a ModelVersionChange for updating the alias of a model version.
+ *
+ * @param aliasesToAdd The new aliases to be added for the model version.
+ * @param aliasesToDelete The aliases to be removed from the model version.
+ * @return A new ModelVersionChange instance for updating the alias of a
model version.
+ */
+ static ModelVersionChange updateAlias(String[] aliasesToAdd, String[]
aliasesToDelete) {
Review Comment:
`updateAliases`, not `updateAlias`, it is a plural, can you please fix them
all.
##########
api/src/main/java/org/apache/gravitino/model/ModelVersionChange.java:
##########
@@ -327,4 +349,92 @@ public String toString() {
return "UpdateUri " + newUri;
}
}
+
+ /** A ModelVersionChange to update the alias of a model version. */
+ final class UpdateAlias implements ModelVersionChange {
+ private final List<String> aliasesToAdd;
+ private final List<String> aliasesToDelete;
+
+ /**
+ * Creates a new {@link UpdateAlias} instance with specified aliases to
add and delete. This
+ * constructor sorts both lists and converts them to immutable collections.
+ *
+ * @param aliasesToAdd List of aliases to add to the model version, can be
null
+ * @param aliasesToDelete List of aliases to remove from the model
version, can be null
+ */
+ public UpdateAlias(List<String> aliasesToAdd, List<String>
aliasesToDelete) {
+ List<String> tmpAliasesToAdd = aliasesToAdd == null ? new ArrayList<>()
: aliasesToAdd;
+ List<String> tmpAliasesToDelete =
+ aliasesToDelete == null ? new ArrayList<>() : aliasesToDelete;
+
+ Collections.sort(tmpAliasesToAdd);
+ Collections.sort(tmpAliasesToDelete);
Review Comment:
Why do we need to sort?
--
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]
