From b1e2ce177872f1fec09325f63257a49610e6ba6b Mon Sep 17 00:00:00 2001 From: Venkateshwaran Shanmugham Date: Fri, 29 May 2026 21:32:32 +0530 Subject: [PATCH 1/4] Fix optimized sibling check for JDBC persistence --- .../jdbc/JdbcBasePersistenceImpl.java | 11 ++- ...anagerWithJdbcBasePersistenceImplTest.java | 89 +++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index bbc917bc0b8..a9d2390bee8 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -42,6 +42,7 @@ import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.core.entity.PolarisEntityCore; import org.apache.polaris.core.entity.PolarisEntityId; import org.apache.polaris.core.entity.PolarisEntitySubType; @@ -810,8 +811,14 @@ Optional> hasOverlappingSiblings( if (!results.isEmpty()) { StorageLocation entityLocation = StorageLocation.of(entity.getBaseLocation()); for (PolarisBaseEntity result : results) { - StorageLocation potentialSiblingLocation = - StorageLocation.of(((LocationBasedEntity) result).getBaseLocation()); + // JDBC materializes persisted rows as PolarisBaseEntity, so use the stored location + // property instead of casting to LocationBasedEntity. + String siblingBaseLocation = + result.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION); + if (siblingBaseLocation == null || siblingBaseLocation.isBlank()) { + continue; + } + StorageLocation potentialSiblingLocation = StorageLocation.of(siblingBaseLocation); if (entityLocation.isChildOf(potentialSiblingLocation) || potentialSiblingLocation.isChildOf(entityLocation)) { return Optional.of(Optional.of(potentialSiblingLocation.toString())); diff --git a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java index ec1bc596911..dad6f55e5c4 100644 --- a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java +++ b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java @@ -22,16 +22,26 @@ import java.io.InputStream; import java.sql.SQLException; +import java.util.List; +import java.util.Map; import java.util.Optional; import javax.sql.DataSource; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; import org.apache.polaris.core.PolarisDiagnostics; import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.core.entity.LocationBasedEntity; +import org.apache.polaris.core.entity.PolarisBaseEntity; +import org.apache.polaris.core.entity.PolarisEntity; +import org.apache.polaris.core.entity.PolarisEntityConstants; +import org.apache.polaris.core.entity.PolarisEntitySubType; +import org.apache.polaris.core.entity.PolarisEntityType; import org.apache.polaris.core.persistence.AtomicOperationMetaStoreManager; import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest; import org.apache.polaris.core.persistence.PolarisTestMetaStoreManager; +import org.assertj.core.api.Assertions; import org.h2.jdbcx.JdbcConnectionPool; +import org.junit.jupiter.api.Test; import org.mockito.Mockito; public abstract class AtomicMetastoreManagerWithJdbcBasePersistenceImplTest @@ -79,6 +89,85 @@ protected PolarisTestMetaStoreManager createPolarisTestMetaStoreManager() { return new PolarisTestMetaStoreManager(metaStoreManager, callCtx); } + @Test + void testHasOverlappingSiblingsUsesStoredBaseLocation() { + // The optimized check relies on the location_without_scheme column added in schema v2. + if (schemaVersion() < 2) { + return; + } + + var metaStoreManager = polarisTestMetaStoreManager.polarisMetaStoreManager(); + var callContext = polarisTestMetaStoreManager.polarisCallContext(); + PolarisBaseEntity catalog = + new PolarisBaseEntity( + PolarisEntityConstants.getNullId(), + metaStoreManager.generateNewEntityId(callContext).getId(), + PolarisEntityType.CATALOG, + PolarisEntitySubType.NULL_SUBTYPE, + PolarisEntityConstants.getRootEntityId(), + "overlap_test_catalog"); + catalog = metaStoreManager.createCatalog(callContext, catalog, List.of()).getCatalog(); + + PolarisBaseEntity existingNamespace = + buildLocationBasedNamespace( + catalog, + metaStoreManager.generateNewEntityId(callContext).getId(), + "existing", + "s3://bucket/warehouse/existing/"); + metaStoreManager.createEntityIfNotExists( + callContext, List.of(PolarisEntity.toCore(catalog)), existingNamespace); + + TestLocationBasedEntity candidateNamespace = + new TestLocationBasedEntity( + buildLocationBasedNamespace( + catalog, + metaStoreManager.generateNewEntityId(callContext).getId(), + "candidate", + "s3://bucket/warehouse/existing/child")); + + Assertions.assertThat(metaStoreManager.hasOverlappingSiblings(callContext, candidateNamespace)) + .contains(Optional.of("s3://bucket/warehouse/existing/")); + + TestLocationBasedEntity nonOverlappingNamespace = + new TestLocationBasedEntity( + buildLocationBasedNamespace( + catalog, + metaStoreManager.generateNewEntityId(callContext).getId(), + "non_overlapping", + "s3://bucket/warehouse/non-overlapping")); + + Assertions.assertThat( + metaStoreManager.hasOverlappingSiblings(callContext, nonOverlappingNamespace)) + .contains(Optional.empty()); + } + + private static PolarisBaseEntity buildLocationBasedNamespace( + PolarisBaseEntity catalog, long entityId, String name, String baseLocation) { + return new PolarisBaseEntity.Builder() + .catalogId(catalog.getId()) + .parentId(catalog.getId()) + .id(entityId) + .typeCode(PolarisEntityType.NAMESPACE.getCode()) + .subTypeCode(PolarisEntitySubType.NULL_SUBTYPE.getCode()) + .name(name) + .propertiesAsMap( + Map.of(PolarisEntityConstants.ENTITY_BASE_LOCATION, baseLocation)) + .internalPropertiesAsMap(Map.of()) + .build(); + } + + private static class TestLocationBasedEntity extends PolarisEntity + implements LocationBasedEntity { + TestLocationBasedEntity(PolarisBaseEntity sourceEntity) { + super(sourceEntity); + } + + @Override + public String getBaseLocation() { + return getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION); + } + } + private static class H2JdbcConfiguration implements RelationalJdbcConfiguration { @Override From ee929d9008c70bd4394a2cb8ff59f798c577d174 Mon Sep 17 00:00:00 2001 From: Venkateshwaran Shanmugham Date: Fri, 29 May 2026 22:04:07 +0530 Subject: [PATCH 2/4] Apply Spotless formatting --- .../AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java index dad6f55e5c4..63755f72688 100644 --- a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java +++ b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java @@ -150,8 +150,7 @@ private static PolarisBaseEntity buildLocationBasedNamespace( .typeCode(PolarisEntityType.NAMESPACE.getCode()) .subTypeCode(PolarisEntitySubType.NULL_SUBTYPE.getCode()) .name(name) - .propertiesAsMap( - Map.of(PolarisEntityConstants.ENTITY_BASE_LOCATION, baseLocation)) + .propertiesAsMap(Map.of(PolarisEntityConstants.ENTITY_BASE_LOCATION, baseLocation)) .internalPropertiesAsMap(Map.of()) .build(); } From 2e25d39cb95fa5aba43abcf0fa0dd4483a7306fb Mon Sep 17 00:00:00 2001 From: Venkateshwaran Shanmugham Date: Sun, 31 May 2026 20:03:38 +0530 Subject: [PATCH 3/4] JDBC: use PolarisEntityUtils for sibling location checks --- .../jdbc/JdbcBasePersistenceImpl.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index a9d2390bee8..3d61d9d2eec 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -42,11 +42,11 @@ import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.core.entity.PolarisChangeTrackingVersions; import org.apache.polaris.core.entity.PolarisEntity; -import org.apache.polaris.core.entity.PolarisEntityConstants; import org.apache.polaris.core.entity.PolarisEntityCore; import org.apache.polaris.core.entity.PolarisEntityId; import org.apache.polaris.core.entity.PolarisEntitySubType; import org.apache.polaris.core.entity.PolarisEntityType; +import org.apache.polaris.core.entity.PolarisEntityUtils; import org.apache.polaris.core.entity.PolarisEvent; import org.apache.polaris.core.entity.PolarisGrantRecord; import org.apache.polaris.core.entity.PolarisPrincipalSecrets; @@ -811,17 +811,21 @@ Optional> hasOverlappingSiblings( if (!results.isEmpty()) { StorageLocation entityLocation = StorageLocation.of(entity.getBaseLocation()); for (PolarisBaseEntity result : results) { - // JDBC materializes persisted rows as PolarisBaseEntity, so use the stored location - // property instead of casting to LocationBasedEntity. - String siblingBaseLocation = - result.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION); - if (siblingBaseLocation == null || siblingBaseLocation.isBlank()) { - continue; - } - StorageLocation potentialSiblingLocation = StorageLocation.of(siblingBaseLocation); - if (entityLocation.isChildOf(potentialSiblingLocation) - || potentialSiblingLocation.isChildOf(entityLocation)) { - return Optional.of(Optional.of(potentialSiblingLocation.toString())); + // JDBC materializes persisted rows as PolarisBaseEntity; resolve location via entity + // utils + // instead of casting to LocationBasedEntity. + Optional overlappingSiblingLocation = + PolarisEntityUtils.asLocationBasedEntity(PolarisEntity.of(result)) + .map(LocationBasedEntity::getBaseLocation) + .filter(location -> location != null && !location.isBlank()) + .map(StorageLocation::of) + .filter( + potentialSiblingLocation -> + entityLocation.isChildOf(potentialSiblingLocation) + || potentialSiblingLocation.isChildOf(entityLocation)) + .map(StorageLocation::toString); + if (overlappingSiblingLocation.isPresent()) { + return Optional.of(overlappingSiblingLocation); } } } From d8668fcd3fabbd33f6e520aedd3a8c8676834970 Mon Sep 17 00:00:00 2001 From: Venkateshwaran Shanmugham Date: Tue, 2 Jun 2026 14:35:23 +0530 Subject: [PATCH 4/4] Address review feedback --- .../persistence/relational/jdbc/JdbcBasePersistenceImpl.java | 5 ++--- ...tomicMetastoreManagerWithJdbcBasePersistenceImplTest.java | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 3d61d9d2eec..4ca5b5ade27 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -811,9 +811,8 @@ Optional> hasOverlappingSiblings( if (!results.isEmpty()) { StorageLocation entityLocation = StorageLocation.of(entity.getBaseLocation()); for (PolarisBaseEntity result : results) { - // JDBC materializes persisted rows as PolarisBaseEntity; resolve location via entity - // utils - // instead of casting to LocationBasedEntity. + // JDBC materializes persisted rows as PolarisBaseEntity. Resolve the sibling location + // via PolarisEntityUtils instead of casting to LocationBasedEntity. Optional overlappingSiblingLocation = PolarisEntityUtils.asLocationBasedEntity(PolarisEntity.of(result)) .map(LocationBasedEntity::getBaseLocation) diff --git a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java index 63755f72688..54199d286b0 100644 --- a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java +++ b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java @@ -40,6 +40,7 @@ import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest; import org.apache.polaris.core.persistence.PolarisTestMetaStoreManager; import org.assertj.core.api.Assertions; +import org.assertj.core.api.Assumptions; import org.h2.jdbcx.JdbcConnectionPool; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -92,9 +93,7 @@ protected PolarisTestMetaStoreManager createPolarisTestMetaStoreManager() { @Test void testHasOverlappingSiblingsUsesStoredBaseLocation() { // The optimized check relies on the location_without_scheme column added in schema v2. - if (schemaVersion() < 2) { - return; - } + Assumptions.assumeThat(schemaVersion()).isGreaterThanOrEqualTo(2); var metaStoreManager = polarisTestMetaStoreManager.polarisMetaStoreManager(); var callContext = polarisTestMetaStoreManager.polarisCallContext();