yuqi1129 commented on code in PR #5426:
URL: https://github.com/apache/gravitino/pull/5426#discussion_r1827161958


##########
core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java:
##########
@@ -229,6 +231,31 @@ public boolean deleteTable(NameIdentifier identifier) {
           if (deleteResult.get() > 0) {
             
TableColumnMetaService.getInstance().deleteColumnsByTableId(tableId);
           }
+        },
+        () -> {
+          if (deleteResult.get() > 0) {

Review Comment:
   This is a problem unrelated to this PR. please see L218, Since it uses 
`doWithCommitAndFetchResult`, so it's will use two transactions in the 
`deleteTable` if I'm not wrong. 



##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SecurableObjectBaseSQLProvider.java:
##########
@@ -84,6 +89,78 @@ public String 
softDeleteRoleMetasByMetalakeId(@Param("metalakeId") Long metalake
         + " AND ro.deleted_at = 0) AND ob.deleted_at = 0";
   }
 
+  public String softDeleteObjectRelsByMetadataObject(
+      @Param("metadataObjectId") Long metadataObjectId,
+      @Param("metadataObjectType") String metadataObjectType) {
+    return "UPDATE "
+        + SECURABLE_OBJECT_TABLE_NAME
+        + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)"
+        + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000"
+        + " WHERE metadata_object_id = #{metadataObjectId} AND deleted_at = 0"
+        + " AND type = #{metadataObjectType}";
+  }
+
+  public String softDeleteObjectRelsByCatalogId(@Param("catalogId") Long 
catalogId) {
+    return "UPDATE "
+        + SECURABLE_OBJECT_TABLE_NAME
+        + " sect SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)"

Review Comment:
   Can we simply use this SQL as " 
   
   ```SQL
   update SECURABLE_OBJECT_TABLE_NAME c set xxxxx
   
   where sect.deleted_at = 0 and sec.type in ('catalog', 'schema'....) and 
exist (
   
   
   
   .....
   )
    
   ```



##########
core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java:
##########
@@ -242,16 +251,28 @@ public boolean deleteCatalog(NameIdentifier identifier, 
boolean cascade) {
             "Entity %s has sub-entities, you should remove sub-entities 
first", identifier);
       }
       SessionUtils.doMultipleWithCommit(
-          () ->
-              SessionUtils.doWithoutCommit(
-                  CatalogMetaMapper.class,
-                  mapper -> 
mapper.softDeleteCatalogMetasByCatalogId(catalogId)),
           () ->
               SessionUtils.doWithoutCommit(
                   OwnerMetaMapper.class,
                   mapper ->
                       mapper.softDeleteOwnerRelByMetadataObjectIdAndType(
-                          catalogId, MetadataObject.Type.CATALOG.name())));
+                          catalogId, MetadataObject.Type.CATALOG.name())),
+          () ->
+              SessionUtils.doWithoutCommit(
+                  SecurableObjectMapper.class,
+                  mapper ->
+                      mapper.softDeleteObjectRelsByMetadataObject(
+                          catalogId, MetadataObject.Type.CATALOG.name())),
+          () ->
+              SessionUtils.doWithoutCommit(
+                  TagMetadataObjectRelMapper.class,
+                  mapper ->
+                      mapper.softDeleteTagMetadataObjectRelsByMetadataObject(
+                          catalogId, MetadataObject.Type.CATALOG.name())),
+          () ->
+              SessionUtils.doWithoutCommit(
+                  CatalogMetaMapper.class,
+                  mapper -> 
mapper.softDeleteCatalogMetasByCatalogId(catalogId)));

Review Comment:
   Does the deletion of `securableobject`, `tag` and `catalog` matters in this 
transaction? why do you change the sequence of them?



-- 
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]

Reply via email to