mchades commented on code in PR #10879:
URL: https://github.com/apache/gravitino/pull/10879#discussion_r3164860198
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/ViewMetaService.java:
##########
@@ -74,188 +80,118 @@ public Long getViewIdBySchemaIdAndName(Long schemaId,
String viewName) {
return viewId;
}
- @Monitored(metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
baseMetricName = "insertView")
- public void insertView(ViewPO viewPO, boolean overwrite) throws IOException {
- try {
- SessionUtils.doWithCommit(
- ViewMetaMapper.class,
- mapper -> {
- if (overwrite) {
- mapper.insertViewMetaOnDuplicateKeyUpdate(viewPO);
- } else {
- mapper.insertViewMeta(viewPO);
- }
- });
- } catch (RuntimeException re) {
- ExceptionUtils.checkSQLException(re, Entity.EntityType.VIEW,
viewPO.getViewName());
- throw re;
- }
- }
-
- @Monitored(
- metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
- baseMetricName = "deleteViewMetasByLegacyTimeline")
- public int deleteViewMetasByLegacyTimeline(Long legacyTimeline, int limit) {
- return SessionUtils.doWithCommitAndFetchResult(
- ViewMetaMapper.class,
- mapper -> mapper.deleteViewMetasByLegacyTimeline(legacyTimeline,
limit));
- }
-
- /**
- * List views as GenericEntity by namespace.
- *
- * @param namespace The namespace to list views from.
- * @return A list of GenericEntity representing views.
- */
@Monitored(
metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
baseMetricName = "listViewsByNamespace")
- public List<GenericEntity> listViewsByNamespace(Namespace namespace) {
+ public List<ViewEntity> listViewsByNamespace(Namespace namespace) {
NamespaceUtil.checkView(namespace);
List<ViewPO> viewPOs = listViewPOsByNamespace(namespace);
- return viewPOs.stream()
- .map(
- viewPO ->
- GenericEntity.builder()
- .withId(viewPO.getViewId())
- .withName(viewPO.getViewName())
- .withNamespace(namespace)
- .withEntityType(Entity.EntityType.VIEW)
- .build())
- .collect(Collectors.toList());
+ return viewPOs.stream().map(po -> fromViewPO(po,
namespace)).collect(Collectors.toList());
}
- /**
- * Get a view as GenericEntity by identifier.
- *
- * @param identifier The identifier of the view.
- * @return The GenericEntity representing the view.
- */
@Monitored(
metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
baseMetricName = "getViewByIdentifier")
- public GenericEntity getViewByIdentifier(NameIdentifier identifier) {
+ public ViewEntity getViewByIdentifier(NameIdentifier identifier) {
NameIdentifierUtil.checkView(identifier);
-
ViewPO viewPO = viewPOFetcher().apply(identifier);
-
- return GenericEntity.builder()
- .withId(viewPO.getViewId())
- .withName(viewPO.getViewName())
- .withNamespace(identifier.namespace())
- .withEntityType(Entity.EntityType.VIEW)
- .build();
+ return fromViewPO(viewPO, identifier.namespace());
}
- /**
- * Insert a view from GenericEntity.
- *
- * @param entity The GenericEntity representing the view.
- * @param overwrite Whether to overwrite an existing view.
- * @throws IOException If an I/O error occurs.
- */
- @Monitored(
- metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
- baseMetricName = "insertViewFromEntity")
- public void insertView(GenericEntity entity, boolean overwrite) throws
IOException {
- Namespace namespace = entity.namespace();
- NamespaceUtil.checkView(namespace);
+ @Monitored(metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
baseMetricName = "insertView")
+ public void insertView(ViewEntity viewEntity, boolean overwrite) throws
IOException {
+ NameIdentifierUtil.checkView(viewEntity.nameIdentifier());
- ViewPO.Builder builder = createViewPOBuilder(namespace);
- ViewPO viewPO =
- builder
- .withViewId(entity.id())
- .withViewName(entity.name())
- .withCurrentVersion(1L)
- .withLastVersion(1L)
- .withDeletedAt(0L)
- .build();
-
- insertView(viewPO, overwrite);
+ ViewPO.Builder builder = ViewPO.builder();
+ try {
+ ViewPO po = initializeViewPO(viewEntity, builder);
+
+ SessionUtils.doMultipleWithCommit(
+ () ->
+ SessionUtils.doWithoutCommit(
+ ViewMetaMapper.class,
+ mapper -> {
+ if (overwrite) {
+ mapper.insertViewMetaOnDuplicateKeyUpdate(po);
+ } else {
+ mapper.insertViewMeta(po);
+ }
+ }),
+ () ->
+ SessionUtils.doWithoutCommit(
+ ViewVersionInfoMapper.class,
+ mapper -> {
+ if (overwrite) {
+
mapper.insertViewVersionInfoOnDuplicateKeyUpdate(po.getViewVersionInfoPO());
+ } else {
+ mapper.insertViewVersionInfo(po.getViewVersionInfoPO());
+ }
+ }));
+ } catch (RuntimeException re) {
+ ExceptionUtils.checkSQLException(
+ re, Entity.EntityType.VIEW, viewEntity.nameIdentifier().toString());
+ throw re;
+ }
}
- /**
- * Update a view.
- *
- * @param ident The identifier of the view.
- * @param updater The function to update the view entity.
- * @return The updated GenericEntity.
- * @throws IOException If an I/O error occurs.
- */
@Monitored(
metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME,
baseMetricName = "updateViewByIdentifier")
- public <E extends HasIdentifier> GenericEntity updateView(
+ public <E extends Entity & HasIdentifier> ViewEntity updateView(
NameIdentifier ident, Function<E, E> updater) throws IOException {
NameIdentifierUtil.checkView(ident);
ViewPO oldViewPO = viewPOFetcher().apply(ident);
+ ViewEntity oldViewEntity = fromViewPO(oldViewPO, ident.namespace());
+ ViewEntity newEntity = (ViewEntity) updater.apply((E) oldViewEntity);
+ Preconditions.checkArgument(
+ Objects.equals(oldViewEntity.id(), newEntity.id()),
+ "The updated view entity id: %s should be same with the entity id
before: %s",
+ newEntity.id(),
+ oldViewEntity.id());
- GenericEntity oldEntity =
- GenericEntity.builder()
- .withId(oldViewPO.getViewId())
- .withName(oldViewPO.getViewName())
- .withNamespace(ident.namespace())
- .withEntityType(Entity.EntityType.VIEW)
- .build();
-
- GenericEntity newEntity = (GenericEntity) updater.apply((E) oldEntity);
-
- // Check if the namespace (schema) has changed and resolve new schema ID
if needed
- boolean isSchemaChanged =
- newEntity.namespace() != null &&
!newEntity.namespace().equals(ident.namespace());
- Long schemaId =
- isSchemaChanged
- ? EntityIdService.getEntityId(
- NameIdentifier.of(newEntity.namespace().levels()),
Entity.EntityType.SCHEMA)
- : oldViewPO.getSchemaId();
-
- ViewPO newViewPO =
- ViewPO.builder()
- .withViewId(oldViewPO.getViewId())
- .withViewName(newEntity.name())
- .withMetalakeId(oldViewPO.getMetalakeId())
- .withCatalogId(oldViewPO.getCatalogId())
- .withSchemaId(schemaId)
- .withDeletedAt(oldViewPO.getDeletedAt())
- .withLastVersion(oldViewPO.getLastVersion())
- .withCurrentVersion(oldViewPO.getCurrentVersion())
- .build();
-
- updateView(oldViewPO, newViewPO);
-
- return newEntity;
+ try {
+ ViewPO newViewPO = updateViewPO(oldViewPO, newEntity);
+ SessionUtils.doMultipleWithCommit(
+ () ->
+ SessionUtils.doWithoutCommit(
+ ViewVersionInfoMapper.class,
+ mapper ->
mapper.insertViewVersionInfo(newViewPO.getViewVersionInfoPO())),
+ () ->
+ SessionUtils.doWithoutCommit(
+ ViewMetaMapper.class, mapper ->
mapper.updateViewMeta(newViewPO, oldViewPO)));
Review Comment:
fixed
--
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]
