This is an automated email from the ASF dual-hosted git repository.

honahx pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git


The following commit(s) were added to refs/heads/main by this push:
     new 0d4e0ff0f [Core, Bug] CreateEntitiesIfNotExist/CreatePrincipal not 
return the same entity persisted. (#3219)
0d4e0ff0f is described below

commit 0d4e0ff0f3dbe261c76fd826925ad7f9166b43f7
Author: Honah (Jonas) J. <[email protected]>
AuthorDate: Mon Dec 8 17:52:46 2025 -0600

    [Core, Bug] CreateEntitiesIfNotExist/CreatePrincipal not return the same 
entity persisted. (#3219)
    
    The PR fixes the issue, "CreateEntitiesIfNotExist/CreatePrincipal not 
return the same entity persisted", by letting persistEntity return the entity 
persisted and include that in the EntityResult. The PR also include new unit 
tests to verify the behavior
---
 .../AtomicOperationMetaStoreManager.java           |  2 +-
 .../TransactionalMetaStoreManagerImpl.java         | 18 ++++------
 .../BasePolarisMetaStoreManagerTest.java           | 39 ++++++++++++++++++++++
 3 files changed, 47 insertions(+), 12 deletions(-)

diff --git 
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
 
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
index d90223752..c31886d56 100644
--- 
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
+++ 
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
@@ -807,7 +807,7 @@ public class AtomicOperationMetaStoreManager extends 
BaseMetaStoreManager {
     }
 
     // success, return the two entities
-    return new CreatePrincipalResult(updatedPrincipal, principalSecrets);
+    return new CreatePrincipalResult(lowLevelResult.getEntity(), 
principalSecrets);
   }
 
   /** {@inheritDoc} */
diff --git 
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
 
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
index 753c58ab6..658b7cf94 100644
--- 
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
+++ 
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
@@ -113,7 +113,7 @@ public class TransactionalMetaStoreManagerImpl extends 
BaseMetaStoreManager {
    * @param ms meta store in read/write mode
    * @param entity entity we need a new persisted record for
    */
-  protected void persistNewEntity(
+  protected PolarisBaseEntity persistNewEntity(
       @Nonnull PolarisCallContext callCtx,
       @Nonnull TransactionalPersistence ms,
       @Nonnull PolarisBaseEntity entity) {
@@ -122,6 +122,7 @@ public class TransactionalMetaStoreManagerImpl extends 
BaseMetaStoreManager {
 
     // write it
     ms.writeEntityInCurrentTxn(callCtx, entity, true, null);
+    return entity;
   }
 
   /**
@@ -834,11 +835,9 @@ public class TransactionalMetaStoreManagerImpl extends 
BaseMetaStoreManager {
             .setClientId(principalSecrets.getPrincipalClientId())
             .build();
 
-    // now create and persist new catalog entity
-    this.persistNewEntity(callCtx, ms, updatedPrincipal);
-
-    // success, return the two entities
-    return new CreatePrincipalResult(updatedPrincipal, principalSecrets);
+    // persist new principal entity and return the two entities
+    return new CreatePrincipalResult(
+        this.persistNewEntity(callCtx, ms, updatedPrincipal), 
principalSecrets);
   }
 
   /** {@inheritDoc} */
@@ -1084,11 +1083,8 @@ public class TransactionalMetaStoreManagerImpl extends 
BaseMetaStoreManager {
           BaseResult.ReturnStatus.ENTITY_ALREADY_EXISTS, 
entityActiveRecord.getSubTypeCode());
     }
 
-    // persist that new entity
-    this.persistNewEntity(callCtx, ms, entity);
-
-    // done, return that newly created entity
-    return new EntityResult(entity);
+    // persist and return that newly created entity
+    return new EntityResult(this.persistNewEntity(callCtx, ms, entity));
   }
 
   /** {@inheritDoc} */
diff --git 
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
 
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
index 771fe41eb..88242e88d 100644
--- 
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
+++ 
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
@@ -41,6 +41,7 @@ import org.apache.polaris.core.entity.PolarisEntity;
 import org.apache.polaris.core.entity.PolarisEntitySubType;
 import org.apache.polaris.core.entity.PolarisEntityType;
 import org.apache.polaris.core.entity.PolarisTaskConstants;
+import org.apache.polaris.core.entity.PrincipalEntity;
 import org.apache.polaris.core.entity.TaskEntity;
 import org.apache.polaris.core.persistence.pagination.PageToken;
 import org.assertj.core.api.Assertions;
@@ -128,6 +129,44 @@ public abstract class BasePolarisMetaStoreManagerTest {
         .hasSize(2)
         .extracting(PolarisEntity::toCore)
         .containsExactly(PolarisEntity.toCore(task1), 
PolarisEntity.toCore(task2));
+
+    
Assertions.assertThat(createdEntities).containsExactlyInAnyOrderElementsOf(listedEntities);
+  }
+
+  @Test
+  protected void testCreatePrincipalReturnedEntitySameAsPersisted() {
+    PolarisMetaStoreManager metaStoreManager = 
polarisTestMetaStoreManager.polarisMetaStoreManager;
+    PolarisCallContext callCtx = 
polarisTestMetaStoreManager.polarisCallContext;
+    PolarisBaseEntity principalEntity =
+        metaStoreManager
+            .createPrincipal(
+                callCtx,
+                new PrincipalEntity.Builder()
+                    
.setId(metaStoreManager.generateNewEntityId(callCtx).getId())
+                    .setName("principal_test")
+                    .setCreateTimestamp(100L)
+                    .build())
+            .getPrincipal();
+
+    Assertions.assertThat(principalEntity)
+        .isNotNull()
+        .extracting(PolarisBaseEntity::getName)
+        .isEqualTo("principal_test");
+    Assertions.assertThat(principalEntity)
+        .extracting(PolarisBaseEntity::getCreateTimestamp)
+        .isEqualTo(100L);
+
+    PolarisBaseEntity fetchedPrincipal =
+        metaStoreManager
+            .readEntityByName(
+                callCtx,
+                null,
+                PolarisEntityType.PRINCIPAL,
+                PolarisEntitySubType.NULL_SUBTYPE,
+                "principal_test")
+            .getEntity();
+
+    Assertions.assertThat(principalEntity).isEqualTo(fetchedPrincipal);
   }
 
   @Test

Reply via email to