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


##########
core/src/main/java/org/apache/gravitino/storage/relational/service/OccWriteSupport.java:
##########
@@ -115,6 +115,20 @@ public static void deleteWithVersion(
     }
   }
 
+  /**
+   * Executes a single-row compare-and-set update for an entity guarded by 
version.
+   *
+   * @param updateOps an operation supplying the number of rows affected by 
the update
+   * @param onMissSupplier a supplier providing the RuntimeException when zero 
rows are updated
+   */
+  public static void updateWithVersion(

Review Comment:
   Agreed on the substance, deferred to 
https://github.com/apache/gravitino/issues/12969.
   
   Backfilling means touching the CAS path of `TableMetaService`, 
`TopicMetaService`, `CatalogMetaService` and `ModelVersionMetaService` and 
re-running their storage tests against H2, MySQL and PostgreSQL — on a PR that 
is already +1267/-399. This PR did the half that belongs to it: the shared 
helper now exists and has its own tests. The issue tracks migrating the four 
call sites one commit at a time.



##########
core/src/main/java/org/apache/gravitino/storage/relational/service/PolicyMetaService.java:
##########
@@ -93,36 +103,35 @@ public void insertPolicy(PolicyEntity policyEntity, 
boolean overwritten) throws
     String metalakeName = ns.level(0);
 
     try {
-      Long metalakeId =
-          EntityIdService.getEntityId(NameIdentifier.of(metalakeName), 
Entity.EntityType.METALAKE);
+      MetalakePO metalakePO =

Review Comment:
   Folded into https://github.com/apache/gravitino/issues/12969 as a secondary 
item, alongside the `updateWithVersion` backfill. A shared 
`requireMetalakeByName` has to live where several services can reach it, so it 
is a cross-service change rather than something to land inside the policy OCC 
PR.



##########
core/src/main/java/org/apache/gravitino/storage/relational/mapper/PolicyMetaMapper.java:
##########
@@ -146,6 +161,29 @@ Integer deletePolicyMetasByLegacyTimeline(
   PolicyPO selectPolicyMetaByMetalakeIdAndName(
       @Param("metalakeId") long metalakeId, @Param("policyName") String 
policyName);
 
+  /**
+   * Selects and exclusively locks an active policy by its natural key.
+   *
+   * @param metalakeId The metalake ID.
+   * @param policyName The policy name.
+   * @return The locked policy, or null if the natural key is not active.
+   */
+  @Results({

Review Comment:
   Filed as https://github.com/apache/gravitino/issues/12970.
   
   Seven of the ten blocks predate this PR, and a wrong result mapping fails 
silently rather than throwing, so converting them needs its own change with the 
policy storage tests run against all three dialects — not a ride-along in an 
OCC PR.



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