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