XN137 commented on code in PR #2518:
URL: https://github.com/apache/polaris/pull/2518#discussion_r2330616309


##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java:
##########
@@ -577,28 +575,22 @@ private void revokeGrantRecord(
     // role. The principal role will be granted to that root principal and the 
root catalog admin
     // of the root catalog will be granted to that principal role.
     long rootPrincipalId = ms.generateNewId(callCtx);
-    PolarisBaseEntity rootPrincipal =
-        new PolarisBaseEntity(
-            PolarisEntityConstants.getNullId(),
-            rootPrincipalId,
-            PolarisEntityType.PRINCIPAL,
-            PolarisEntitySubType.NULL_SUBTYPE,
-            PolarisEntityConstants.getRootEntityId(),
-            PolarisEntityConstants.getRootPrincipalName());
-
-    // create this principal
+    PrincipalEntity rootPrincipal =
+        new PrincipalEntity.Builder()
+            .setId(rootPrincipalId)
+            .setName(PolarisEntityConstants.getRootPrincipalName())
+            .setCreateTimestamp(System.currentTimeMillis())

Review Comment:
   this looks odd as we are not using the `clock` field but using the clock 
field causes some test to fail, so i think we need to followup where we 
overhaul things to prefer `Clock` everywhere.
   
   for now this is simply keeping the existing logic from the 
`PolarisBaseEntity` ctor:
   
https://github.com/apache/polaris/blob/7af85be7f45c933a377314a669e2a16633c93532/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisBaseEntity.java#L244-L257



-- 
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: issues-unsubscr...@polaris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to