yuqi1129 commented on code in PR #7502:
URL: https://github.com/apache/gravitino/pull/7502#discussion_r2176251935
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/ModelVersionMetaService.java:
##########
@@ -321,26 +329,53 @@ public <E extends Entity & HasIdentifier>
ModelVersionEntity updateModelVersion(
try {
SessionUtils.doMultipleWithCommit(
() ->
- updateResult.set(
+ updateResult.addAndGet(
SessionUtils.doWithoutCommitAndFetchResult(
ModelVersionMetaMapper.class,
mapper ->
mapper.updateModelVersionMeta(
POConverters.updateModelVersionPO(
- oldModelVersionPO, newModelVersionEntity),
- oldModelVersionPO))),
+ oldModelVersionPOs.get(0),
newModelVersionEntity),
+ oldModelVersionPOs.get(0)))),
+ // TODO Only modifying the unknown URI is supported currently.
+ () ->
+ oldModelVersionPOs.stream()
+ .filter(
+ v ->
+
ModelVersion.URI_NAME_UNKNOWN.equals(v.getModelVersionUriName())
Review Comment:
The condition is quite complicated. Could you please provide a method with a
meaningful name to replace it?
##########
catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java:
##########
@@ -477,7 +478,7 @@ private ModelVersionEntity updateModelVersionEntity(
.withModelIdentifier(entityModelIdentifier)
.withAliases(entityAliases)
.withComment(entityComment)
- .withUri(entityUri)
+ .withUris(ImmutableMap.of(ModelVersion.URI_NAME_UNKNOWN, entityUri))
Review Comment:
What's the difference between `URI_NAME_UNKNOWN` and
`PROPERTY_DEFAULT_URI_NAME` in interface `ModelVersion`
##########
core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java:
##########
@@ -1474,21 +1484,31 @@ public static List<ModelVersionAliasRelPO>
updateModelVersionAliasRelPO(
}
}
- public static ModelVersionPO initializeModelVersionPO(
- ModelVersionEntity modelVersionEntity, ModelVersionPO.Builder builder) {
+ public static List<ModelVersionPO> initializeModelVersionPO(
+ ModelVersionEntity modelVersionEntity, Long modelId) {
try {
- return builder
- // Note that version set here will not be used when inserting into
database, it will
- // directly use the version from the query to avoid concurrent
version conflict.
- .withModelVersion(modelVersionEntity.version())
- .withModelVersionComment(modelVersionEntity.comment())
- .withModelVersionUri(modelVersionEntity.uri())
- .withModelVersionProperties(
-
JsonUtils.anyFieldMapper().writeValueAsString(modelVersionEntity.properties()))
- .withAuditInfo(
-
JsonUtils.anyFieldMapper().writeValueAsString(modelVersionEntity.auditInfo()))
- .withDeletedAt(DEFAULT_DELETED_AT)
- .build();
+ String modelVersionProperties =
+
JsonUtils.anyFieldMapper().writeValueAsString(modelVersionEntity.properties());
+ String modelVersionAuditInfo =
+
JsonUtils.anyFieldMapper().writeValueAsString(modelVersionEntity.auditInfo());
+ return modelVersionEntity.uris().entrySet().stream()
+ .map(
+ entry ->
+ ModelVersionPO.builder()
+ .withModelId(modelId)
+ // Note that version set here will not be used when
inserting into database,
+ // it will
+ // directly use the version from the query to avoid
concurrent version
Review Comment:
please format the comment
##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/h2/ModelVersionMetaH2Provider.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.storage.relational.mapper.provider.h2;
+
+import java.util.List;
+import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper;
+import
org.apache.gravitino.storage.relational.mapper.provider.base.ModelVersionMetaBaseSQLProvider;
+import org.apache.gravitino.storage.relational.po.ModelVersionPO;
+import org.apache.ibatis.annotations.Param;
+
+public class ModelVersionMetaH2Provider extends
ModelVersionMetaBaseSQLProvider {
+
+ public String insertModelVersionMetas(
+ @Param("modelVersionMetas") List<ModelVersionPO> modelVersionPOs) {
+ return "<script>"
+ + "INSERT INTO "
+ + ModelVersionMetaMapper.TABLE_NAME
+ + "(metalake_id, catalog_id, schema_id, model_id, version,
model_version_comment,"
+ + " model_version_properties, model_version_uri_name,
model_version_uri, audit_info, deleted_at)"
+ + " SELECT m.metalake_id, m.catalog_id, m.schema_id, m.model_id,
m.model_latest_version, v.model_version_comment,"
+ + " v.model_version_properties, v.model_version_uri_name,
v.model_version_uri, v.audit_info, v.deleted_at"
+ + " FROM ("
+ + "<foreach collection='modelVersionMetas' item='version'
separator='UNION ALL'>"
+ + " SELECT"
+ + " CAST(#{version.modelId} AS BIGINT) AS model_id,
CAST(#{version.modelVersionComment} AS TEXT) AS model_version_comment,"
Review Comment:
Why do we need to `cast` here?
##########
core/src/main/java/org/apache/gravitino/meta/ModelVersionEntity.java:
##########
@@ -133,6 +134,13 @@ public Long id() {
throw new UnsupportedOperationException("Model version entity does not
have an ID.");
}
+ @Override
+ public void validate() throws IllegalArgumentException {
+ Entity.super.validate();
+ Preconditions.checkArgument(
+ !uris.isEmpty(), "The uri of the model version entity must not be
empty.");
Review Comment:
Will the `url` can't be null?
--
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]
