You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by ad...@apache.org on 2026/10/28 20:18:46 UTC
(polaris) branch main updated: Validate table locations against the namespace on rename and for existing tables (#5856)
This is an automated email from the ASF dual-hosted git repository.
adutra 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 17926bf9f5 Validate table locations against the namespace on rename and for existing tables (#5856)
17926bf9f5 is described below
commit 17926bf9f578f93f0cee2b34803611183dd227e9
Author: Alexandre Dutra <ad...@apache.org>
AuthorDate: Wed Oct 28 21:18:39 2026 +0100
Validate table locations against the namespace on rename and for existing tables (#5856)
Renaming a table or view into another namespace now validates its locations
as if it were created in that namespace: the catalog's allowed locations, the
destination namespace's location unless ALLOW_UNSTRUCTURED_TABLE_LOCATION is
enabled, and overlap with the destination's siblings.
For existing tables, the structured location check now uses the location of
the table's parent namespace, in addition to the table's own location. This
applies when committing to a table and when vending credentials for it.
---
CHANGELOG.md | 13 +-
.../nosql/metastore/NoSqlMetaStore.java | 51 ++-
.../nosql/metastore/NoSqlMetaStoreManager.java | 15 +-
.../nosql/metastore/TestNoSqlMetaStoreManager.java | 84 +++--
.../relational/jdbc/JdbcBasePersistenceImpl.java | 35 +-
.../relational/jdbc/QueryGenerator.java | 48 ++-
...toreManagerWithJdbcBasePersistenceImplTest.java | 15 +-
.../relational/jdbc/QueryGeneratorTest.java | 88 ++++-
.../AtomicOperationMetaStoreManager.java | 10 +-
.../polaris/core/persistence/BasePersistence.java | 32 +-
.../core/persistence/LocationOverlapResult.java | 36 ++
.../core/persistence/PolarisMetaStoreManager.java | 24 +-
.../TransactionWorkspaceMetaStoreManager.java | 10 +-
.../TransactionalMetaStoreManagerImpl.java | 14 +-
.../TreeMapTransactionalPersistenceImpl.java | 25 +-
.../polaris/core/storage/LocationRestrictions.java | 31 +-
.../storage/PolarisStorageConfigurationInfo.java | 60 ++--
.../PolarisTreeMapMetaStoreManagerTest.java | 33 +-
.../PolarisStorageConfigurationInfoTest.java | 79 +++++
.../catalog/iceberg/LocalIcebergCatalog.java | 109 +++++-
.../AbstractIcebergCatalogHandlerAuthzTest.java | 23 ++
.../iceberg/IcebergAllowedLocationTest.java | 387 +++++++++++++++++++++
22 files changed, 1037 insertions(+), 185 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index beda011412..4fc70c2be9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -98,6 +98,11 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- The event attribute `EventAttributes.ACCESS_DELEGATION_MODE` (`String`) has been replaced with
`EventAttributes.ACCESS_DELEGATION_MODES` (`List<String>`). Accordingly, the OpenTelemetry event
listener now emits `polaris.access_delegation_modes` instead of `polaris.access_delegation_mode`.
+- Table and view renames now validate locations against the destination namespace. Existing tables
+ also validate their namespace location during commits and credential vending. With
+ `ALLOW_UNSTRUCTURED_TABLE_LOCATION=false` (default), operations involving locations outside the
+ namespace now return HTTP 403. Catalogs that enable unstructured locations remain subject to
+ catalog allowlists and configured overlap checks.
### New Features
@@ -116,8 +121,12 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- Azure credential vending now maps MSAL authentication failures using their HTTP status codes,
including the existing `401` to `403` and `404` to `400` mappings.
- `PolarisMetaStoreManager.hasOverlappingSiblings` and `BasePersistence.hasOverlappingSiblings` now
- take the entity's resolved parent path, so implementations exclude the entity's own ancestors
- without re-reading the parent chain from the metastore.
+ take the ids of the entity's catalog and parent namespaces, so implementations exclude them
+ without re-reading the parent chain from the metastore, and return a `LocationOverlapResult`. They
+ also take an optional search root: when set, only the entities located at or below it are
+ considered, which allows renamed entities to be checked against the destination namespace only.
+ Implementations must also ignore the entity itself, identified by its id, so that an already
+ persisted entity does not conflict with its own location.
- Apache Iceberg has been upgraded to 1.12.0. The Java REST client now encodes spaces in namespace
and table names as `%20` instead of `+` (apache/iceberg#15989). Polaris decodes path segments
per RFC 3986 and has always treated `+` as a literal character, so the server behaves the same for
diff --git a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
index 6c61c021da..7ee0dc4060 100644
--- a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
+++ b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStore.java
@@ -630,7 +630,7 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
}
<T extends PolarisEntity & LocationBasedEntity> Optional<String> hasOverlappingSiblings(
- List<PolarisEntityCore> parentPath, T entity) {
+ T entity, Set<Long> parentPathIds, @Nullable StorageLocation searchRoot) {
var baseLocation = entity.getBaseLocation();
if (baseLocation == null) {
return Optional.empty();
@@ -641,13 +641,38 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
// The entity's own parent namespaces may contain its location: they always do when locations
// follow the namespace tree, as default locations do. Such an ancestor is not a sibling.
- var ancestorIds = parentPath.stream().map(PolarisEntityCore::getId).collect(Collectors.toSet());
-
- return hasOverlappingSiblings(catalogId, checkLocation, ancestorIds, entity.getId());
+ return hasOverlappingSiblings(
+ catalogId, checkLocation, parentPathIds, entity.getId(), searchRoot);
}
Optional<String> hasOverlappingSiblings(
- long catalogId, String checkLocation, Set<Long> ancestorIds, long entityId) {
+ long catalogId,
+ String checkLocation,
+ Set<Long> ancestorIds,
+ long entityId,
+ @Nullable StorageLocation searchRoot) {
+ var locationIdentifier = identifierFromLocationString(checkLocation);
+ var searchRootIdentifier =
+ searchRoot == null ? null : identifierFromLocationString(searchRoot.withoutScheme());
+
+ // With a search root, only the entities located inside the search root are considered. The
+ // candidates are
+ // the descendants of the location (including an entity at exactly the location) and its
+ // ancestors:
+ // - If the location is inside the search root, all its descendants are inside the search root,
+ // but only
+ // the ancestors that are at least as deep as the search root are.
+ // - If the search root is inside the location, only the descendants of the search root can
+ // match, and no
+ // ancestor can: it would be shallower than the search root.
+ // - Otherwise, nothing can match.
+ var locationInsideSearchRoot =
+ searchRootIdentifier == null || locationIdentifier.startsWith(searchRootIdentifier);
+ if (!locationInsideSearchRoot && !searchRootIdentifier.startsWith(locationIdentifier)) {
+ return Optional.empty();
+ }
+ var scanIdentifier = locationInsideSearchRoot ? locationIdentifier : searchRootIdentifier;
+
return memoizedIndexedAccess
.catalogContent(catalogId)
.refObj()
@@ -663,18 +688,15 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
var byName =
catalogStateObj.nameToObjRef().indexForRead(persistence, OBJ_REF_SERIALIZER);
- var locationIdentifier = identifierFromLocationString(checkLocation);
- var locationIndexKey = locationIdentifier.toIndexKey();
-
// Check for children and exact matches first using forward iteration (preserves
// existing test expectations on which conflicting location is reported).
// Also iterate fully in case early entries are filtered out.
- var iter = locationsIndex.iterator(locationIndexKey, null, false);
+ var iter = locationsIndex.iterator(scanIdentifier.toIndexKey(), null, false);
while (iter.hasNext()) {
var elem = iter.next();
var elemKey = elem.key();
var elemIdentifier = indexKeyToIdentifier(elemKey);
- if (!elemIdentifier.startsWith(locationIdentifier)) {
+ if (!elemIdentifier.startsWith(scanIdentifier)) {
break; // No more matches due to ordering
}
@@ -704,7 +726,14 @@ class NoSqlMetaStore extends NonFunctionalBasePersistence {
// Check for parent (prefix) overlaps. These have shorter keys and are missed by
// forward iteration starting at the full target key.
- for (int i = 1; i <= locationIdentifier.length(); i++) {
+ if (!locationInsideSearchRoot) {
+ // Location is outside the search root: no ancestor could be inside it,
+ // so skip prefix checks.
+ return Optional.empty();
+ }
+ var firstPrefixLength =
+ searchRootIdentifier == null ? 1 : Math.max(1, searchRootIdentifier.length());
+ for (int i = firstPrefixLength; i <= locationIdentifier.length(); i++) {
var prefixElements = locationIdentifier.elements().subList(0, i);
var prefix = ContentIdentifier.identifier(prefixElements);
var prefixKey = prefix.toIndexKey();
diff --git a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStoreManager.java b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStoreManager.java
index 2277e9e8a8..d873ecc197 100644
--- a/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStoreManager.java
+++ b/persistence/nosql/persistence/metastore/src/main/java/org/apache/polaris/persistence/nosql/metastore/NoSqlMetaStoreManager.java
@@ -30,6 +30,7 @@ import java.time.Clock;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
@@ -50,6 +51,7 @@ import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
import org.apache.polaris.core.entity.PolarisPrivilege;
import org.apache.polaris.core.entity.PolarisTaskConstants;
import org.apache.polaris.core.entity.PrincipalEntity;
+import org.apache.polaris.core.persistence.LocationOverlapResult;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolarisObjectMapperUtil;
import org.apache.polaris.core.persistence.RenameEntityUtil;
@@ -75,6 +77,7 @@ import org.apache.polaris.core.persistence.pagination.Page;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.apache.polaris.core.policy.PolicyEntity;
import org.apache.polaris.core.policy.PolicyType;
+import org.apache.polaris.core.storage.StorageLocation;
import org.apache.polaris.persistence.nosql.metastore.mutation.GrantsMutation;
import org.apache.polaris.persistence.nosql.metastore.privs.SecurableGranteePrivilegeTuple;
import org.jspecify.annotations.NonNull;
@@ -492,11 +495,15 @@ record NoSqlMetaStoreManager(
@Override
public <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
- return Optional.of(ms(callContext).hasOverlappingSiblings(parentPath, entity));
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
+ return ms(callContext)
+ .hasOverlappingSiblings(entity, parentPathIds, searchRoot)
+ .<LocationOverlapResult>map(LocationOverlapResult.Overlap::new)
+ .orElseGet(LocationOverlapResult.NoOverlap::new);
}
@NonNull
diff --git a/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java b/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java
index 1016fd48e8..b176d766cc 100644
--- a/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java
+++ b/persistence/nosql/persistence/metastore/src/test/java/org/apache/polaris/persistence/nosql/metastore/TestNoSqlMetaStoreManager.java
@@ -39,7 +39,7 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
+import java.util.Set;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.IntStream;
@@ -51,7 +51,6 @@ import org.apache.polaris.core.context.RealmContext;
import org.apache.polaris.core.entity.CatalogEntity;
import org.apache.polaris.core.entity.NamespaceEntity;
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.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntitySubType;
@@ -59,6 +58,7 @@ import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisPrivilege;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest;
+import org.apache.polaris.core.persistence.LocationOverlapResult;
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolarisTestMetaStoreManager;
@@ -74,6 +74,7 @@ import org.apache.polaris.core.persistence.pagination.Page;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.apache.polaris.core.policy.PolicyEntity;
import org.apache.polaris.core.policy.PredefinedPolicyTypes;
+import org.apache.polaris.core.storage.StorageLocation;
import org.apache.polaris.ids.api.MonotonicClock;
import org.apache.polaris.persistence.nosql.api.Persistence;
import org.apache.polaris.persistence.nosql.api.RealmPersistenceFactory;
@@ -263,13 +264,34 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
soft.assertThat(
metaStore.hasOverlappingSiblings(
callContext,
- List.of(PolarisEntity.toCore(catalog)),
new NamespaceEntity.Builder(Namespace.of("x"))
.setCatalogId(catalog.getId())
.setBaseLocation("s3://bucket/foo/newchild/")
- .build()))
- .isPresent()
- .contains(Optional.of("s3://bucket/foo/"));
+ .build(),
+ Set.of(catalog.getId()),
+ null))
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/foo/"));
+
+ // With a search root, only the entities located inside it are considered.
+ var newChild =
+ new NamespaceEntity.Builder(Namespace.of("x"))
+ .setCatalogId(catalog.getId())
+ .setBaseLocation("s3://bucket/foo/newchild/")
+ .build();
+ soft.assertThat(
+ metaStore.hasOverlappingSiblings(
+ callContext,
+ newChild,
+ Set.of(catalog.getId()),
+ StorageLocation.of("s3://bucket/foo/")))
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/foo/"));
+ soft.assertThat(
+ metaStore.hasOverlappingSiblings(
+ callContext,
+ newChild,
+ Set.of(catalog.getId()),
+ StorageLocation.of("s3://bucket/other/")))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
// The same location is a conflict, unless the entity is the persisted one (e.g. a rename).
var renamedFoo =
@@ -279,9 +301,8 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
.build();
soft.assertThat(
metaStore.hasOverlappingSiblings(
- callContext, List.of(PolarisEntity.toCore(catalog)), renamedFoo))
- .isPresent()
- .contains(Optional.of("s3://bucket/foo/"));
+ callContext, renamedFoo, Set.of(catalog.getId()), null))
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/foo/"));
var persistedFoo =
new NamespaceEntity.Builder(Namespace.of("renamed"))
.setId(nsFoo.getEntity().getId())
@@ -290,9 +311,8 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
.build();
soft.assertThat(
metaStore.hasOverlappingSiblings(
- callContext, List.of(PolarisEntity.toCore(catalog)), persistedFoo))
- .isPresent()
- .contains(Optional.empty());
+ callContext, persistedFoo, Set.of(catalog.getId()), null))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
var nsBar =
createEntity(
@@ -323,13 +343,13 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
soft.assertThat(
metaStore.hasOverlappingSiblings(
callContext,
- List.of(PolarisEntity.toCore(catalog)),
new NamespaceEntity.Builder(Namespace.of("x"))
.setCatalogId(catalog.getId())
.setBaseLocation("s3://bucket/foo/")
- .build()))
- .isPresent()
- .contains(Optional.of("s3://bucket/foo/"));
+ .build(),
+ Set.of(catalog.getId()),
+ null))
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/foo/"));
for (var check :
List.of(
@@ -340,25 +360,25 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
soft.assertThat(
metaStore.hasOverlappingSiblings(
callContext,
- List.of(PolarisEntity.toCore(catalog)),
new NamespaceEntity.Builder(Namespace.of("x"))
.setCatalogId(catalog.getId())
.setBaseLocation(check)
- .build()))
- .isPresent()
- .contains(Optional.of("s3://bucket/foo/bar/"));
+ .build(),
+ Set.of(catalog.getId()),
+ null))
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/foo/bar/"));
}
soft.assertThat(
metaStore.hasOverlappingSiblings(
callContext,
- List.of(PolarisEntity.toCore(catalog)),
new NamespaceEntity.Builder(Namespace.of("x"))
.setCatalogId(catalog.getId())
.setBaseLocation("s3://other/data/stuff/")
- .build()))
- .isPresent()
- .contains(Optional.empty());
+ .build(),
+ Set.of(catalog.getId()),
+ null))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
// Drop one of the entities with the duplicate base location
metaStore.dropEntityIfExists(callContext, List.of(catalog), nsFoobar.getEntity(), null, false);
@@ -366,13 +386,13 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
soft.assertThat(
metaStore.hasOverlappingSiblings(
callContext,
- List.of(PolarisEntity.toCore(catalog)),
new NamespaceEntity.Builder(Namespace.of("x"))
.setCatalogId(catalog.getId())
.setBaseLocation("s3://bucket/foo/bar")
- .build()))
- .isPresent()
- .contains(Optional.of("s3://bucket/foo/bar/"));
+ .build(),
+ Set.of(catalog.getId()),
+ null))
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/foo/bar/"));
// Drop one of the entities with the duplicate base location
metaStore.dropEntityIfExists(callContext, List.of(catalog), nsFoobar2.getEntity(), null, false);
@@ -384,13 +404,13 @@ public class TestNoSqlMetaStoreManager extends BasePolarisMetaStoreManagerTest {
soft.assertThat(
metaStore.hasOverlappingSiblings(
callContext,
- List.of(PolarisEntity.toCore(catalog)),
new NamespaceEntity.Builder(Namespace.of("x"))
.setCatalogId(catalog.getId())
.setBaseLocation("s3://bucket/foo/bar")
- .build()))
- .isPresent()
- .contains(Optional.empty());
+ .build(),
+ Set.of(catalog.getId()),
+ null))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
}
@Test
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 dc1da5b005..a3439bae33 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
@@ -57,6 +57,7 @@ import org.apache.polaris.core.exceptions.AlreadyExistsException;
import org.apache.polaris.core.persistence.BasePersistence;
import org.apache.polaris.core.persistence.EntityAlreadyExistsException;
import org.apache.polaris.core.persistence.IntegrationPersistence;
+import org.apache.polaris.core.persistence.LocationOverlapResult;
import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException;
import org.apache.polaris.core.persistence.PrincipalSecretsGenerator;
import org.apache.polaris.core.persistence.RetryOnConcurrencyException;
@@ -793,31 +794,39 @@ public class JdbcBasePersistenceImpl implements BasePersistence, IntegrationPers
/** {@inheritDoc} */
@Override
public <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
if (entity.getBaseLocation().chars().filter(ch -> ch == '/').count()
> MAX_LOCATION_COMPONENTS) {
- return Optional.empty();
+ return new LocationOverlapResult.Unsupported();
+ }
+
+ StorageLocation entityLocation = StorageLocation.of(entity.getBaseLocation());
+
+ if (searchRoot != null) {
+ // Only the entities located inside the search root are considered: if the search root and the
+ // entity's
+ // location are not nested, there is none that can overlap the location.
+ if (!entityLocation.isChildOf(searchRoot) && !searchRoot.isChildOf(entityLocation)) {
+ return new LocationOverlapResult.NoOverlap();
+ }
}
PreparedQuery query =
QueryGenerator.generateOverlapQuery(
- realmId, entity.getCatalogId(), entity.getBaseLocation());
+ realmId, entity.getCatalogId(), entity.getBaseLocation(), searchRoot);
try {
var results = datasourceOperations.executeSelect(query, new ModelEntity());
if (results.isEmpty()) {
- return Optional.of(Optional.empty());
+ return new LocationOverlapResult.NoOverlap();
}
// The query matches every entity whose location is an ancestor of, equal to, or a descendant
// of the entity's location. The entity's own parent namespaces may match the ancestor terms:
// they always do when locations follow the namespace tree, as default locations do. Such an
// ancestor is not a sibling.
- Set<Long> ancestorIds =
- parentPath.stream().map(PolarisEntityCore::getId).collect(Collectors.toSet());
-
- StorageLocation entityLocation = StorageLocation.of(entity.getBaseLocation());
for (PolarisBaseEntity result : results) {
if (result.getId() == entity.getId()) {
continue;
@@ -835,14 +844,14 @@ public class JdbcBasePersistenceImpl implements BasePersistence, IntegrationPers
boolean containsEntity = entityLocation.isChildOf(resultLocation.get());
boolean containedByEntity = resultLocation.get().isChildOf(entityLocation);
// An ancestor may contain the entity, but the entity may not sit at exactly its location.
- if (containsEntity && !containedByEntity && ancestorIds.contains(result.getId())) {
+ if (containsEntity && !containedByEntity && parentPathIds.contains(result.getId())) {
continue;
}
if (containsEntity || containedByEntity) {
- return Optional.of(Optional.of(resultLocation.get().toString()));
+ return new LocationOverlapResult.Overlap(resultLocation.get().toString());
}
}
- return Optional.of(Optional.empty());
+ return new LocationOverlapResult.NoOverlap();
} catch (SQLException e) {
LOGGER.error(
"Failed to retrieve location overlap for location {} due to {}",
diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java
index 9d0b776c18..314d3d8e33 100644
--- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java
+++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/QueryGenerator.java
@@ -19,6 +19,7 @@
package org.apache.polaris.persistence.relational.jdbc;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -384,7 +385,7 @@ public class QueryGenerator {
*/
@VisibleForTesting
public static PreparedQuery generateOverlapQuery(
- String realmId, long catalogId, String baseLocation) {
+ String realmId, long catalogId, String baseLocation, @Nullable StorageLocation searchRoot) {
StorageLocation baseStorageLocation = StorageLocation.of(baseLocation);
String locationWithoutScheme = baseStorageLocation.withoutScheme();
@@ -393,10 +394,7 @@ public class QueryGenerator {
// Normalize by stripping any trailing slash so the scan below only emits meaningful prefixes.
// The slash-terminated form is re-added as the last prefix term below.
- String normalizedLocation = locationWithoutScheme;
- if (normalizedLocation.endsWith("/")) {
- normalizedLocation = normalizedLocation.substring(0, normalizedLocation.length() - 1);
- }
+ String normalizedLocation = stripTrailingSlash(locationWithoutScheme);
Set<String> prefixTerms = new LinkedHashSet<>();
for (int i = 0; i < normalizedLocation.length(); i++) {
@@ -410,6 +408,35 @@ public class QueryGenerator {
prefixTerms.add(normalizedLocation);
prefixTerms.add(normalizedLocation + "/");
+ // The location whose descendants are matched by the LIKE condition below.
+ String descendantsRoot = locationWithoutScheme;
+
+ if (searchRoot != null) {
+ // Only the entities located inside the search root, i.e. at exactly the search root or below
+ // it, can
+ // match.
+ if (baseStorageLocation.isChildOf(searchRoot)) {
+ // All the descendants of the location are inside the search root, but only some of its
+ // ancestors.
+ // The prefixes are compared as plain strings, like the stored locations are by the query.
+ String searchRootPrefix = StorageLocation.ensureTrailingSlash(searchRoot.withoutScheme());
+ prefixTerms.removeIf(
+ prefix -> !StorageLocation.ensureTrailingSlash(prefix).startsWith(searchRootPrefix));
+ } else {
+ // Only the descendants of the search root can match, and no ancestor of the location can.
+ Preconditions.checkArgument(
+ searchRoot.isChildOf(baseStorageLocation),
+ "The search root %s and the location %s are not nested",
+ searchRoot,
+ baseLocation);
+ String normalizedSearchRoot = stripTrailingSlash(searchRoot.withoutScheme());
+ prefixTerms.clear();
+ prefixTerms.add(normalizedSearchRoot);
+ prefixTerms.add(normalizedSearchRoot + "/");
+ descendantsRoot = normalizedSearchRoot;
+ }
+ }
+
for (String prefix : prefixTerms) {
// Skip only "/", which can never be a meaningful storage location. "//" is kept: in
// ALLOW_NAMESPACE_CUSTOM_LOCATION mode a namespace location may be a bare scheme root like
@@ -430,8 +457,7 @@ public class QueryGenerator {
// (the caller's precise re-check still filters those) and a literal \ could hide true
// descendants.
conditions.add("location_without_scheme LIKE ? ESCAPE '\\'");
- parameters.add(
- escapeLikePattern(StorageLocation.ensureTrailingSlash(locationWithoutScheme)) + "%");
+ parameters.add(escapeLikePattern(StorageLocation.ensureTrailingSlash(descendantsRoot)) + "%");
String locationClause = String.join(" OR ", conditions);
String clause = " WHERE realm_id = ? AND catalog_id = ? AND (" + locationClause + ")";
@@ -449,6 +475,14 @@ public class QueryGenerator {
return new PreparedQuery(query.sql(), where.parameters());
}
+ /**
+ * Strips a single trailing slash, if any. Unlike stripping all of them, this keeps the bare
+ * scheme roots, which are persisted as {@code //} or {@code ///}, meaningful.
+ */
+ private static String stripTrailingSlash(String location) {
+ return location.endsWith("/") ? location.substring(0, location.length() - 1) : location;
+ }
+
/**
* Escapes the LIKE metacharacters {@code %} and {@code _} and the escape character {@code \} in a
* literal so it can be used safely as a fixed prefix in a {@code LIKE ? ESCAPE '\'} pattern. The
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 ae4a0aac28..38939d1e56 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
@@ -25,7 +25,7 @@ import java.io.InputStream;
import java.sql.SQLException;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
+import java.util.Set;
import javax.sql.DataSource;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
@@ -40,6 +40,7 @@ import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.persistence.AtomicOperationMetaStoreManager;
import org.apache.polaris.core.persistence.BasePersistence;
import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest;
+import org.apache.polaris.core.persistence.LocationOverlapResult;
import org.apache.polaris.core.persistence.PolarisTestMetaStoreManager;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.assertj.core.api.Assertions;
@@ -132,8 +133,8 @@ public abstract class AtomicMetastoreManagerWithJdbcBasePersistenceImplTest
Assertions.assertThat(
metaStoreManager.hasOverlappingSiblings(
- callContext, List.of(PolarisEntity.toCore(catalog)), candidateNamespace))
- .contains(Optional.of("s3://bucket/warehouse/existing/"));
+ callContext, candidateNamespace, Set.of(catalog.getId()), null))
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/warehouse/existing/"));
TestLocationBasedEntity nonOverlappingNamespace =
new TestLocationBasedEntity(
@@ -145,8 +146,8 @@ public abstract class AtomicMetastoreManagerWithJdbcBasePersistenceImplTest
Assertions.assertThat(
metaStoreManager.hasOverlappingSiblings(
- callContext, List.of(PolarisEntity.toCore(catalog)), nonOverlappingNamespace))
- .contains(Optional.empty());
+ callContext, nonOverlappingNamespace, Set.of(catalog.getId()), null))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
// The persisted entity itself, e.g. when renamed, does not conflict with its own location.
TestLocationBasedEntity renamedNamespace =
@@ -156,8 +157,8 @@ public abstract class AtomicMetastoreManagerWithJdbcBasePersistenceImplTest
Assertions.assertThat(
metaStoreManager.hasOverlappingSiblings(
- callContext, List.of(PolarisEntity.toCore(catalog)), renamedNamespace))
- .contains(Optional.empty());
+ callContext, renamedNamespace, Set.of(catalog.getId()), null))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
}
@Test
diff --git a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java
index bbb3131338..126b2beb71 100644
--- a/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java
+++ b/persistence/relational-jdbc/src/test/java/org/apache/polaris/persistence/relational/jdbc/QueryGeneratorTest.java
@@ -35,6 +35,7 @@ import java.util.Map;
import java.util.Set;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisEntityId;
+import org.apache.polaris.core.storage.StorageLocation;
import org.apache.polaris.persistence.relational.jdbc.models.ModelEntity;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -327,9 +328,10 @@ public class QueryGeneratorTest {
+ " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+ " location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+ " location_without_scheme LIKE ? ESCAPE '\\')",
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/tmp/location/").sql());
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/tmp/location/", null)
+ .sql());
Assertions.assertThatCollection(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/tmp/location/")
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/tmp/location/", null)
.parameters())
.containsExactly(
"realmId",
@@ -346,7 +348,7 @@ public class QueryGeneratorTest {
// A location without a trailing slash produces the same prefix terms so that ancestors stored
// in either form are matched.
Assertions.assertThatCollection(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/tmp/location")
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/tmp/location", null)
.parameters())
.containsExactly(
"realmId",
@@ -369,9 +371,10 @@ public class QueryGeneratorTest {
+ " ENTITIES WHERE realm_id = ? AND catalog_id = ? AND (location_without_scheme = ?"
+ " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+ " location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme LIKE ? ESCAPE '\\')",
- QueryGenerator.generateOverlapQuery("realmId", -123, "/tmp/location/").sql());
+ QueryGenerator.generateOverlapQuery("realmId", -123, "/tmp/location/", null).sql());
Assertions.assertThatCollection(
- QueryGenerator.generateOverlapQuery("realmId", -123, "/tmp/location/").parameters())
+ QueryGenerator.generateOverlapQuery("realmId", -123, "/tmp/location/", null)
+ .parameters())
.containsExactly(
"realmId",
-123L,
@@ -390,9 +393,10 @@ public class QueryGeneratorTest {
+ " FROM ENTITIES WHERE realm_id = ? AND catalog_id = ? AND (location_without_scheme = ?"
+ " OR location_without_scheme = ? OR location_without_scheme = ? OR location_without_scheme = ? OR"
+ " location_without_scheme = ? OR location_without_scheme LIKE ? ESCAPE '\\')",
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://バケツ/\"loc.ation\"/").sql());
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://バケツ/\"loc.ation\"/", null)
+ .sql());
Assertions.assertThatCollection(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://バケツ/\"loc.ation\"/")
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://バケツ/\"loc.ation\"/", null)
.parameters())
.containsExactly(
"realmId",
@@ -411,17 +415,20 @@ public class QueryGeneratorTest {
// so an ancestor stored at the bare scheme root s3:// (ALLOW_NAMESPACE_CUSTOM_LOCATION mode)
// is still matched. Regression pair: namespace at "//" with a table under "//bucket/ns/t".
Assertions.assertThat(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/ns/t/").parameters())
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/ns/t/", null)
+ .parameters())
.contains("//")
.doesNotContain("/");
// Same without a trailing slash (prefix walk normalizes; slash-only skip is unchanged).
Assertions.assertThat(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/ns/t").parameters())
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/ns/t", null)
+ .parameters())
.contains("//")
.doesNotContain("/");
// file: locations: the "///" root and "//" are kept, but "/" is not emitted.
Assertions.assertThat(
- QueryGenerator.generateOverlapQuery("realmId", -123, "file:///tmp/data/").parameters())
+ QueryGenerator.generateOverlapQuery("realmId", -123, "file:///tmp/data/", null)
+ .parameters())
.contains("//", "///")
.doesNotContain("/");
}
@@ -432,11 +439,11 @@ public class QueryGeneratorTest {
// an ESCAPE clause) so they match literally rather than as wildcards; the exact-match prefix
// terms use "=" and must keep the raw characters.
assertTrue(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/a_b/c%d/")
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/a_b/c%d/", null)
.sql()
.contains("location_without_scheme LIKE ? ESCAPE '\\'"));
Assertions.assertThat(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/a_b/c%d/")
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/a_b/c%d/", null)
.parameters())
.endsWith("//bucket/a\\_b/c\\%d/%")
.contains("//bucket/a_b", "//bucket/a_b/c%d");
@@ -448,10 +455,65 @@ public class QueryGeneratorTest {
// escaped
// so a descendant whose path contains a backslash is still matched rather than silently missed.
Assertions.assertThat(
- QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/a\\b/").parameters())
+ QueryGenerator.generateOverlapQuery("realmId", -123, "s3://bucket/a\\b/", null)
+ .parameters())
.endsWith("//bucket/a\\\\b/%");
}
+ @Test
+ void generateOverlapQueryWithSearchRootContainingLocation() {
+ // All the descendants of the location are inside the search root, but only some of its
+ // ancestors.
+ var query =
+ QueryGenerator.generateOverlapQuery(
+ "realmId", -123, "s3://bucket/ns/t/", StorageLocation.of("s3://bucket/ns/"));
+ Assertions.assertThat(query.sql()).containsOnlyOnce("LIKE");
+ Assertions.assertThat(query.parameters())
+ .containsExactly(
+ "realmId",
+ -123L,
+ "//bucket/ns",
+ "//bucket/ns/",
+ "//bucket/ns/t",
+ "//bucket/ns/t/",
+ "//bucket/ns/t/%");
+ }
+
+ @Test
+ void generateOverlapQueryWithSearchRootInsideLocation() {
+ // Only the descendants of the search root can match, and no ancestor of the location: the
+ // search root's
+ // LIKE condition replaces the location's.
+ var query =
+ QueryGenerator.generateOverlapQuery(
+ "realmId", -123, "s3://bucket/ns/", StorageLocation.of("s3://bucket/ns/a_b"));
+ Assertions.assertThat(query.sql()).containsOnlyOnce("LIKE");
+ Assertions.assertThat(query.parameters())
+ .containsExactly(
+ "realmId", -123L, "//bucket/ns/a_b", "//bucket/ns/a_b/", "//bucket/ns/a\\_b/%");
+ }
+
+ @Test
+ void generateOverlapQueryRejectsUnrelatedSearchRoot() {
+ Assertions.assertThatThrownBy(
+ () ->
+ QueryGenerator.generateOverlapQuery(
+ "realmId", -123, "s3://bucket/ns/t/", StorageLocation.of("s3://bucket/other/")))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ @Test
+ void generateOverlapQueryDoesNotRewriteSchemeLessLocations() {
+ // Without their scheme, "s3://bucket/a/" is "//bucket/a/" and "file:///bucket/a/" is
+ // "///bucket/a/": they are different locations, which must not be conflated into the same
+ // file:/// location.
+ Assertions.assertThatThrownBy(
+ () ->
+ QueryGenerator.generateOverlapQuery(
+ "realmId", -123, "s3://bucket/a/", StorageLocation.of("file:///bucket/a/")))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
@Test
void testGenerateExistsQuery() {
Map<String, Object> params = new HashMap<>();
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 4db61c03c8..96795089d5 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
@@ -76,6 +76,7 @@ import org.apache.polaris.core.policy.PolicyMappingUtil;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageIntegration;
+import org.apache.polaris.core.storage.StorageLocation;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
@@ -1802,12 +1803,13 @@ public class AtomicOperationMetaStoreManager extends BaseMetaStoreManager {
/** {@inheritDoc} */
@Override
public <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
BasePersistence ms = callContext.getMetaStore();
- return ms.hasOverlappingSiblings(callContext, parentPath, entity);
+ return ms.hasOverlappingSiblings(callContext, entity, parentPathIds, searchRoot);
}
@Override
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java
index 75e89a548a..2530ac7201 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/BasePersistence.java
@@ -19,7 +19,7 @@
package org.apache.polaris.core.persistence;
import java.util.List;
-import java.util.Optional;
+import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import org.apache.polaris.core.PolarisCallContext;
@@ -37,6 +37,7 @@ import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.persistence.pagination.Page;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.apache.polaris.core.policy.PolicyMappingPersistence;
+import org.apache.polaris.core.storage.StorageLocation;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
@@ -406,29 +407,32 @@ public interface BasePersistence extends PolicyMappingPersistence {
* OPTIMIZED_SIBLING_CHECK}; the fallback used when that flag is off inspects only the entities
* directly under the same parent.
*
- * <p>The entity's own ancestors, given by {@code parentPath}, are excluded: a parent namespace
- * whose location strictly contains the entity's location is not reported, since default locations
- * nest under the parent's location. An ancestor whose location equals the entity's location is
- * still reported.
+ * <p>The ancestors given by {@code parentPathIds} are excluded: a parent namespace whose location
+ * strictly contains the entity's location is not reported, since default locations nest under the
+ * parent's location. An ancestor whose location equals the entity's location is still reported.
*
* <p>The entity itself, identified by its id, is never reported. Implementations should avoid
* checking the entity against itself when it is already persisted, e.g. on a location update or a
* rename, so that it does not conflict with its own persisted location.
*
* @param callContext the polaris call context
- * @param parentPath the entity's resolved parent path: the catalog first, then each parent
- * namespace, not including the entity itself
* @param entity the entity whose base location to check
- * @return Optional.of(Optional.of(location)) with the base location of a conflicting entity,
- * Optional.of(Optional.empty()) if there is no conflict, and Optional.empty() if the
- * metastore doesn't support this operation
+ * @param parentPathIds IDs of the entity's catalog and parent namespaces. A parent whose location
+ * strictly contains the entity's location is not a conflict. An exact location match is still
+ * a conflict.
+ * @param searchRoot root location to search for overlapping entities, or null to search the whole
+ * catalog. Only entities located at or below this path are checked.
+ * @return {@link LocationOverlapResult.Overlap} with the base location of a conflicting entity,
+ * {@link LocationOverlapResult.NoOverlap} if there is no conflict, and {@link
+ * LocationOverlapResult.Unsupported} if the metastore doesn't support this operation
*/
default <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
- return Optional.empty();
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
+ return new LocationOverlapResult.Unsupported();
}
/**
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocationOverlapResult.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocationOverlapResult.java
new file mode 100644
index 0000000000..d91e0e0b16
--- /dev/null
+++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocationOverlapResult.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.core.persistence;
+
+/** The outcome of {@link PolarisMetaStoreManager#hasOverlappingSiblings}. */
+public sealed interface LocationOverlapResult {
+
+ /** The metastore does not support the check; callers must fall back to another strategy. */
+ record Unsupported() implements LocationOverlapResult {}
+
+ /** The entity's location does not overlap the location of any other entity. */
+ record NoOverlap() implements LocationOverlapResult {}
+
+ /**
+ * The entity's location overlaps the location of another entity.
+ *
+ * @param conflictingLocation the base location of the conflicting entity
+ */
+ record Overlap(String conflictingLocation) implements LocationOverlapResult {}
+}
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java
index f3ca7b3f16..7c029170f4 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java
@@ -21,6 +21,7 @@ package org.apache.polaris.core.persistence;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.function.Predicate;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.auth.AuthBootstrapUtil;
@@ -52,6 +53,7 @@ import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
import org.apache.polaris.core.persistence.pagination.Page;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.apache.polaris.core.policy.PolarisPolicyMappingManager;
+import org.apache.polaris.core.storage.StorageLocation;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
@@ -472,19 +474,23 @@ public interface PolarisMetaStoreManager
* checking the entity against itself in case of updates or renames.
*
* @param callContext the polaris call context
- * @param parentPath the entity's resolved parent path: the catalog first, then each parent
- * namespace, not including the entity itself
* @param entity the entity whose base location to check
- * @return Optional.of(Optional.of(location)) with the base location of a conflicting entity,
- * Optional.of(Optional.empty()) if there is no conflict, and Optional.empty() if the
- * metastore doesn't support this operation
+ * @param parentPathIds IDs of the entity's catalog and parent namespaces. A parent whose location
+ * strictly contains the entity's location is not a conflict. An exact location match is still
+ * a conflict.
+ * @param searchRoot root location to search for overlapping entities, or null to search the whole
+ * catalog. Only entities located at or below this path are checked.
+ * @return {@link LocationOverlapResult.Overlap} with the base location of a conflicting entity,
+ * {@link LocationOverlapResult.NoOverlap} if there is no conflict, and {@link
+ * LocationOverlapResult.Unsupported} if the metastore doesn't support this operation
*/
default <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
- return Optional.empty();
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
+ return new LocationOverlapResult.Unsupported();
}
/**
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java
index 2bcdbb96f7..3bcc9fd6bc 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java
@@ -22,7 +22,7 @@ import com.google.common.collect.ImmutableList;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
+import java.util.Set;
import java.util.function.Predicate;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDiagnostics;
@@ -57,6 +57,7 @@ import org.apache.polaris.core.persistence.pagination.Page;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.apache.polaris.core.policy.PolicyEntity;
import org.apache.polaris.core.policy.PolicyType;
+import org.apache.polaris.core.storage.StorageLocation;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
@@ -366,10 +367,11 @@ public class TransactionWorkspaceMetaStoreManager implements PolarisMetaStoreMan
/** {@inheritDoc} */
@Override
public <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
throw illegalMethodError("hasOverlappingSiblings");
}
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 4f981863a3..d1a71ca0c2 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
@@ -25,7 +25,6 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
@@ -52,6 +51,7 @@ import org.apache.polaris.core.entity.PolarisPrivilege;
import org.apache.polaris.core.entity.PolarisTaskConstants;
import org.apache.polaris.core.entity.PrincipalEntity;
import org.apache.polaris.core.persistence.BaseMetaStoreManager;
+import org.apache.polaris.core.persistence.LocationOverlapResult;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolarisObjectMapperUtil;
import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException;
@@ -82,6 +82,7 @@ import org.apache.polaris.core.policy.PolicyMappingUtil;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageIntegration;
+import org.apache.polaris.core.storage.StorageLocation;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
@@ -2321,15 +2322,18 @@ public class TransactionalMetaStoreManagerImpl extends BaseMetaStoreManager {
/** {@inheritDoc} */
@Override
public <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
TransactionalPersistence ms = ((TransactionalPersistence) callContext.getMetaStore());
return ms.runInTransaction(
callContext,
() -> {
- return callContext.getMetaStore().hasOverlappingSiblings(callContext, parentPath, entity);
+ return callContext
+ .getMetaStore()
+ .hasOverlappingSiblings(callContext, entity, parentPathIds, searchRoot);
});
}
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
index abea2b4c1b..9c51c1005c 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
@@ -43,6 +43,7 @@ import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.entity.PolarisPrincipalSecrets;
import org.apache.polaris.core.exceptions.AlreadyExistsException;
+import org.apache.polaris.core.persistence.LocationOverlapResult;
import org.apache.polaris.core.persistence.PrincipalSecretsGenerator;
import org.apache.polaris.core.persistence.pagination.EntityIdToken;
import org.apache.polaris.core.persistence.pagination.Page;
@@ -673,20 +674,21 @@ public class TreeMapTransactionalPersistenceImpl extends AbstractTransactionalPe
/** {@inheritDoc} */
@Override
public <T extends PolarisEntity & LocationBasedEntity>
- Optional<Optional<String>> hasOverlappingSiblings(
+ LocationOverlapResult hasOverlappingSiblings(
@NonNull PolarisCallContext callContext,
- @NonNull List<PolarisEntityCore> parentPath,
- T entity) {
+ T entity,
+ @NonNull Set<Long> parentPathIds,
+ @Nullable StorageLocation searchRoot) {
// TODO we could optimize this full scan
StorageLocation entityLocationWithoutScheme =
StorageLocation.of(StorageLocation.of(entity.getBaseLocation()).withoutScheme());
List<PolarisBaseEntity> allEntities = this.store.getSliceEntities().readRange("");
+ StorageLocation searchRootWithoutScheme =
+ searchRoot == null ? null : StorageLocation.of(searchRoot.withoutScheme());
+
// The entity's own parent namespaces may contain its location: they always do when locations
// follow the namespace tree, as default locations do. Such an ancestor is not a sibling.
- Set<Long> ancestorIds =
- parentPath.stream().map(PolarisEntityCore::getId).collect(Collectors.toSet());
-
for (PolarisBaseEntity siblingEntity : allEntities) {
if (siblingEntity.getId() == entity.getId()) {
continue;
@@ -694,19 +696,24 @@ public class TreeMapTransactionalPersistenceImpl extends AbstractTransactionalPe
Optional<StorageLocation> maybeSiblingLocationWithoutScheme =
getEntityLocationWithoutScheme(siblingEntity).map(StorageLocation::of);
if (maybeSiblingLocationWithoutScheme.isPresent()) {
+ if (searchRootWithoutScheme != null
+ && !maybeSiblingLocationWithoutScheme.get().isChildOf(searchRootWithoutScheme)) {
+ continue;
+ }
boolean containsEntity =
entityLocationWithoutScheme.isChildOf(maybeSiblingLocationWithoutScheme.get());
boolean containedByEntity =
maybeSiblingLocationWithoutScheme.get().isChildOf(entityLocationWithoutScheme);
// An ancestor may contain the entity, but the entity may not sit at exactly its location.
- if (containsEntity && !containedByEntity && ancestorIds.contains(siblingEntity.getId())) {
+ if (containsEntity && !containedByEntity && parentPathIds.contains(siblingEntity.getId())) {
continue;
}
if (containsEntity || containedByEntity) {
- return Optional.of(Optional.of(maybeSiblingLocationWithoutScheme.toString()));
+ return new LocationOverlapResult.Overlap(
+ siblingEntity.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION));
}
}
}
- return Optional.of(Optional.empty());
+ return new LocationOverlapResult.NoOverlap();
}
}
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/LocationRestrictions.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/LocationRestrictions.java
index 48bc49add2..6abbb0e8c1 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/storage/LocationRestrictions.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/LocationRestrictions.java
@@ -46,17 +46,33 @@ public class LocationRestrictions {
/**
* The parent location for structured table enforcement.
*
- * <p>When non-null, this location represents the root under which all new tables must be created,
+ * <p>When non-null, this location represents the root under which all tables must reside,
* enforcing a structured hierarchy in addition to residing under {@code allowedLocations}. When
- * null, table creation is allowed anywhere within the {@code allowedLocations}.
+ * null, tables are allowed anywhere within the {@code allowedLocations}.
*/
private final String parentLocation;
+ /**
+ * The location of an existing table-like entity for structured table enforcement.
+ *
+ * <p>When non-null, the requested locations must also reside under the entity's own location, in
+ * addition to the parent location.
+ */
+ private final String entityLocation;
+
public LocationRestrictions(
- @NonNull PolarisStorageConfigurationInfo storageConfigurationInfo, String parentLocation) {
+ @NonNull PolarisStorageConfigurationInfo storageConfigurationInfo,
+ String parentLocation,
+ String entityLocation) {
this.allowedLocations = storageConfigurationInfo.getAllowedLocations();
allowedLocations.forEach(storageConfigurationInfo::validatePrefixForStorageType);
this.parentLocation = parentLocation;
+ this.entityLocation = entityLocation;
+ }
+
+ public LocationRestrictions(
+ @NonNull PolarisStorageConfigurationInfo storageConfigurationInfo, String parentLocation) {
+ this(storageConfigurationInfo, parentLocation, null);
}
public LocationRestrictions(@NonNull PolarisStorageConfigurationInfo storageConfigurationInfo) {
@@ -70,12 +86,14 @@ public class LocationRestrictions {
*
* <ul>
* <li>The parent location (if configured) for structured table enforcement
+ * <li>The entity location (if configured) for structured table enforcement
* <li>The allowed locations list for general access permissions
* </ul>
*
* <p>The validation ensures that all requested locations conform to the storage access
- * restrictions defined for this context. If a parent location is configured, all requests must be
- * under that location hierarchy in addition to being within the allowed locations.
+ * restrictions defined for this context. If a parent or entity location is configured, all
+ * requests must be under that location hierarchy in addition to being within the allowed
+ * locations.
*
* @param realmConfig the realm configuration containing storage validation rules
* @param identifier the table identifier for which locations are being validated
@@ -88,6 +106,9 @@ public class LocationRestrictions {
if (parentLocation != null) {
validateLocations(realmConfig, List.of(parentLocation), requestLocations, identifier);
}
+ if (entityLocation != null) {
+ validateLocations(realmConfig, List.of(entityLocation), requestLocations, identifier);
+ }
validateLocations(realmConfig, allowedLocations, requestLocations, identifier);
}
diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java
index 9c4bc80870..ef5c06704c 100644
--- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java
+++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java
@@ -35,6 +35,7 @@ import org.apache.polaris.core.entity.CatalogEntity;
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.PolarisEntityType;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo;
@@ -117,45 +118,54 @@ public abstract class PolarisStorageConfigurationInfo {
return DEFAULT_MAPPER.readValue(jsonStr, PolarisStorageConfigurationInfo.class);
}
+ /**
+ * Returns the location restrictions for the given entity path.
+ *
+ * <p>The parent location is the nearest base location among the ancestors of a table-like leaf,
+ * or the nearest base location in the path otherwise.
+ *
+ * <p>When the path ends with an existing table-like entity, the entity's own base location is a
+ * separate restriction, so that it cannot stand in for its parent's.
+ */
public static Optional<LocationRestrictions> forEntityPath(
RealmConfig realmConfig, List<PolarisEntity> entityPath) {
return findStorageConfigFromHierarchy(entityPath)
.map(
configInfo -> {
- List<PolarisEntity> entityPathReversed = new ArrayList<>(entityPath);
- Collections.reverse(entityPathReversed);
-
- String baseLocation =
- entityPathReversed.stream()
- .flatMap(
- e ->
- Optional.ofNullable(
- e.getPropertiesAsMap()
- .get(PolarisEntityConstants.ENTITY_BASE_LOCATION))
- .stream())
- .findFirst()
- .orElse(null);
CatalogEntity catalog = CatalogEntity.of(entityPath.get(0));
+ PolarisEntity leaf = entityPath.get(entityPath.size() - 1);
boolean allowEscape =
realmConfig.getConfig(
FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION, catalog);
- if (!allowEscape
- && catalog.getCatalogType() != Catalog.TypeEnum.EXTERNAL
- && baseLocation != null) {
- LOGGER.debug(
- "Not allowing unstructured table location for entity: {}",
- entityPathReversed.get(0).getName());
- return new LocationRestrictions(configInfo, baseLocation);
- } else {
- LOGGER.debug(
- "Allowing unstructured table location for entity: {}",
- entityPathReversed.get(0).getName());
-
+ if (allowEscape || catalog.getCatalogType() == Catalog.TypeEnum.EXTERNAL) {
+ LOGGER.debug("Allowing unstructured table location for entity: {}", leaf.getName());
return new LocationRestrictions(configInfo);
}
+ boolean leafIsTableLike = leaf.getType() == PolarisEntityType.TABLE_LIKE;
+ String parentLocation = getParentLocation(entityPath, leafIsTableLike);
+ String entityLocation = leafIsTableLike ? getBaseLocation(leaf) : null;
+ LOGGER.debug(
+ "Not allowing unstructured table location for entity: {}", leaf.getName());
+ return new LocationRestrictions(configInfo, parentLocation, entityLocation);
});
}
+ private static @Nullable String getBaseLocation(PolarisEntity entity) {
+ return entity.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION);
+ }
+
+ private static @Nullable String getParentLocation(
+ List<PolarisEntity> entityPath, boolean leafIsTableLike) {
+ List<PolarisEntity> parentPath =
+ leafIsTableLike ? entityPath.subList(0, entityPath.size() - 1) : entityPath;
+ List<PolarisEntity> parentPathReversed = new ArrayList<>(parentPath);
+ Collections.reverse(parentPathReversed);
+ return parentPathReversed.stream()
+ .flatMap(e -> Optional.ofNullable(getBaseLocation(e)).stream())
+ .findFirst()
+ .orElse(null);
+ }
+
public static @NonNull Optional<PolarisEntity> findEntityWithStorageConfigFromHierarchy(
PolarisResolvedPathWrapper resolvedStorageEntity) {
return findEntityWithStorageConfigFromHierarchy(resolvedStorageEntity.getRawFullPath());
diff --git a/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreManagerTest.java b/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreManagerTest.java
index 6437e1eb32..12c32abcdf 100644
--- a/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreManagerTest.java
+++ b/polaris-core/src/test/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreManagerTest.java
@@ -23,7 +23,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
+import java.util.Set;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
import org.apache.polaris.core.PolarisDiagnostics;
@@ -36,6 +36,7 @@ import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.persistence.transactional.TransactionalMetaStoreManagerImpl;
import org.apache.polaris.core.persistence.transactional.TreeMapMetaStore;
import org.apache.polaris.core.persistence.transactional.TreeMapTransactionalPersistenceImpl;
+import org.apache.polaris.core.storage.StorageLocation;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
@@ -72,18 +73,36 @@ public class PolarisTreeMapMetaStoreManagerTest extends BasePolarisMetaStoreMana
metaStoreManager.createEntityIfNotExists(
callContext, List.of(PolarisEntity.toCore(catalog)), existing);
- var parentPath = List.of(PolarisEntity.toCore(catalog));
+ var parentPathIds = Set.of(catalog.getId());
// A different entity at the same location is a conflict.
var other = new LocationBasedTestEntity(buildNamespace(catalog, existing.getId() + 1));
- var conflict = metaStoreManager.hasOverlappingSiblings(callContext, parentPath, other);
- assertThat(conflict).isPresent();
- assertThat(conflict.get()).isPresent();
+ var conflict = metaStoreManager.hasOverlappingSiblings(callContext, other, parentPathIds, null);
+ // The conflicting location is the location of the existing entity, as it is stored.
+ assertThat(conflict)
+ .isEqualTo(new LocationOverlapResult.Overlap("s3://bucket/warehouse/existing/"));
// The persisted entity itself, e.g. when renamed, is not a conflict.
var self = new LocationBasedTestEntity(buildNamespace(catalog, existing.getId()));
- assertThat(metaStoreManager.hasOverlappingSiblings(callContext, parentPath, self))
- .contains(Optional.empty());
+ assertThat(metaStoreManager.hasOverlappingSiblings(callContext, self, parentPathIds, null))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
+
+ // With a search root, only the entities located inside it are considered.
+ assertThat(
+ metaStoreManager.hasOverlappingSiblings(
+ callContext, other, parentPathIds, StorageLocation.of("s3://bucket/warehouse/")))
+ .isInstanceOf(LocationOverlapResult.Overlap.class);
+ assertThat(
+ metaStoreManager.hasOverlappingSiblings(
+ callContext,
+ other,
+ parentPathIds,
+ StorageLocation.of("s3://bucket/warehouse/existing/")))
+ .isInstanceOf(LocationOverlapResult.Overlap.class);
+ assertThat(
+ metaStoreManager.hasOverlappingSiblings(
+ callContext, other, parentPathIds, StorageLocation.of("s3://bucket/elsewhere/")))
+ .isEqualTo(new LocationOverlapResult.NoOverlap());
}
private static PolarisBaseEntity buildNamespace(PolarisBaseEntity catalog, long entityId) {
diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfoTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfoTest.java
index fdf2769767..998b01a24f 100644
--- a/polaris-core/src/test/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfoTest.java
+++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfoTest.java
@@ -19,9 +19,21 @@
package org.apache.polaris.core.storage;
+import static org.apache.polaris.core.config.RealmConfigurationSource.EMPTY_CONFIG;
import static org.junit.jupiter.params.provider.Arguments.arguments;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
import java.util.stream.Stream;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.ForbiddenException;
+import org.apache.polaris.core.config.RealmConfig;
+import org.apache.polaris.core.config.RealmConfigImpl;
+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.storage.aws.AwsStorageConfigurationInfo;
import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo;
import org.apache.polaris.core.storage.gcp.GcpStorageConfigurationInfo;
@@ -125,4 +137,71 @@ public class PolarisStorageConfigurationInfoTest {
.build(),
"{\"@type\":\"FileStorageConfigurationInfo\",\"allowedLocations\":[\"file:///tmp/bar\",\"file:///meep/moo\"],\"storageType\":\"FILE\",\"fileIoImplClassName\":\"org.apache.iceberg.hadoop.HadoopFileIO\"}"));
}
+
+ @ParameterizedTest
+ @MethodSource
+ public void forEntityPathStructuredLocations(
+ List<PolarisEntity> entityPath, String location, boolean allowed) {
+ RealmConfig realmConfig = new RealmConfigImpl(EMPTY_CONFIG, () -> "test-realm");
+ TableIdentifier identifier = TableIdentifier.of("ns", "t");
+ LocationRestrictions restrictions =
+ PolarisStorageConfigurationInfo.forEntityPath(realmConfig, entityPath).orElseThrow();
+ if (allowed) {
+ soft.assertThatCode(() -> restrictions.validate(realmConfig, identifier, Set.of(location)))
+ .doesNotThrowAnyException();
+ } else {
+ soft.assertThatThrownBy(
+ () -> restrictions.validate(realmConfig, identifier, Set.of(location)))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid locations");
+ }
+ }
+
+ static Stream<Arguments> forEntityPathStructuredLocations() {
+ PolarisEntity catalog =
+ new PolarisEntity.Builder()
+ .setType(PolarisEntityType.CATALOG)
+ .setName("catalog")
+ .setInternalProperties(
+ Map.of(
+ PolarisEntityConstants.getStorageConfigInfoPropertyName(),
+ AwsStorageConfigurationInfo.builder()
+ .addAllowedLocations("s3://lake/")
+ .roleARN("arn:aws:iam::123456789012:role/polaris-test")
+ .build()
+ .serialize()))
+ .build();
+ PolarisEntity namespace = entity(PolarisEntityType.NAMESPACE, "ns", "s3://lake/ns/");
+ PolarisEntity table = entity(PolarisEntityType.TABLE_LIKE, "t", "s3://lake/ns/t/");
+ // A table outside its namespace, as left behind by renaming it from another namespace.
+ PolarisEntity movedTable =
+ entity(PolarisEntityType.TABLE_LIKE, "moved", "s3://lake/other/moved/");
+ // Tables created by notifications have no base location.
+ PolarisEntity tableWithoutLocation = entity(PolarisEntityType.TABLE_LIKE, "notified", null);
+
+ List<PolarisEntity> namespacePath = List.of(catalog, namespace);
+ List<PolarisEntity> tablePath = List.of(catalog, namespace, table);
+ List<PolarisEntity> movedTablePath = List.of(catalog, namespace, movedTable);
+ List<PolarisEntity> noLocationTablePath = List.of(catalog, namespace, tableWithoutLocation);
+ return Stream.of(
+ arguments(namespacePath, "s3://lake/ns/t/", true),
+ arguments(namespacePath, "s3://lake/other/t/", false),
+ arguments(tablePath, "s3://lake/ns/t/", true),
+ arguments(tablePath, "s3://lake/ns/t/data/", true),
+ arguments(tablePath, "s3://lake/ns/elsewhere/", false),
+ arguments(movedTablePath, "s3://lake/other/moved/", false),
+ arguments(noLocationTablePath, "s3://lake/ns/notified/", true),
+ arguments(noLocationTablePath, "s3://lake/other/notified/", false));
+ }
+
+ private static PolarisEntity entity(PolarisEntityType type, String name, String location) {
+ PolarisEntity.Builder builder = new PolarisEntity.Builder().setType(type).setName(name);
+ if (type == PolarisEntityType.TABLE_LIKE) {
+ builder.setSubType(PolarisEntitySubType.ICEBERG_TABLE);
+ }
+ if (location != null) {
+ builder.setProperties(Map.of(PolarisEntityConstants.ENTITY_BASE_LOCATION, location));
+ }
+ return builder.build();
+ }
}
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
index 45314640bd..525ed87e3b 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java
@@ -119,6 +119,7 @@ import org.apache.polaris.core.entity.PolarisTaskConstants;
import org.apache.polaris.core.entity.table.IcebergTableLikeEntity;
import org.apache.polaris.core.exceptions.CommitConflictException;
import org.apache.polaris.core.exceptions.PolarisServiceUnavailableException;
+import org.apache.polaris.core.persistence.LocationOverlapResult;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
@@ -1429,6 +1430,20 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
List<PolarisEntity> resolvedNamespace,
String location,
PolarisEntity entity) {
+ validateNoLocationOverlap(catalog, identifier, resolvedNamespace, location, entity, null);
+ }
+
+ /**
+ * @param searchRoot if not null, only the entities located at or below this location are
+ * considered by the optimized sibling check
+ */
+ private void validateNoLocationOverlap(
+ CatalogEntity catalog,
+ TableIdentifier identifier,
+ List<PolarisEntity> resolvedNamespace,
+ String location,
+ PolarisEntity entity,
+ @Nullable StorageLocation searchRoot) {
boolean validateViewOverlap =
realmConfig.getConfig(BehaviorChangeConfiguration.VALIDATE_VIEW_LOCATION_OVERLAP);
@@ -1459,7 +1474,7 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
.setProperties(Map.of(PolarisEntityConstants.ENTITY_BASE_LOCATION, location))
.build());
- validateNoLocationOverlap(virtualEntity, resolvedNamespace);
+ validateNoLocationOverlap(virtualEntity, resolvedNamespace, searchRoot);
}
}
@@ -1536,26 +1551,32 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
*/
private <T extends PolarisEntity & LocationBasedEntity> void validateNoLocationOverlap(
T entity, List<PolarisEntity> parentPath) {
+ validateNoLocationOverlap(entity, parentPath, null);
+ }
+
+ private <T extends PolarisEntity & LocationBasedEntity> void validateNoLocationOverlap(
+ T entity, List<PolarisEntity> parentPath, @Nullable StorageLocation searchRoot) {
String location = entity.getBaseLocation();
String name = entity.getName();
// Attempt to directly query for siblings
- boolean useOptimizedSiblingCheck =
- realmConfig.getConfig(FeatureConfiguration.OPTIMIZED_SIBLING_CHECK);
- if (useOptimizedSiblingCheck) {
- Optional<Optional<String>> directSiblingCheckResult =
- getMetaStoreManager()
- .hasOverlappingSiblings(
- getCurrentPolarisContext(), PolarisEntity.toCoreList(parentPath), entity);
- if (directSiblingCheckResult.isPresent()) {
- if (directSiblingCheckResult.get().isPresent()) {
- throw new org.apache.iceberg.exceptions.ForbiddenException(
- "Unable to create entity at location '%s' because it conflicts with existing table or namespace at %s",
- location, directSiblingCheckResult.get().get());
- } else {
+ if (realmConfig.getConfig(FeatureConfiguration.OPTIMIZED_SIBLING_CHECK)) {
+ // The entity's own parents may contain its location without being reported.
+ Set<Long> parentPathIds =
+ parentPath.stream().map(PolarisEntity::getId).collect(Collectors.toSet());
+ switch (getMetaStoreManager()
+ .hasOverlappingSiblings(getCurrentPolarisContext(), entity, parentPathIds, searchRoot)) {
+ case LocationOverlapResult.Overlap overlap ->
+ throw new ForbiddenException(
+ "Unable to create entity at location '%s' because it conflicts with existing table or namespace at %s",
+ location, overlap.conflictingLocation());
+ case LocationOverlapResult.NoOverlap ignored -> {
return;
}
+ case LocationOverlapResult.Unsupported ignored -> {
+ // Continue with list-based overlap validation below.
+ }
}
}
@@ -2794,6 +2815,14 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
}
newCatalogPath = resolvedNewParentEntities.getRawFullPath();
+ validateLocationsForRenamedTableLike(
+ subType,
+ from,
+ to,
+ IcebergTableLikeEntity.of(leafEntity),
+ resolvedEntities,
+ resolvedNewParentEntities);
+
// the "to" table has a new parent and a new name / namespace path
toEntity =
new IcebergTableLikeEntity.Builder(IcebergTableLikeEntity.of(leafEntity))
@@ -2888,6 +2917,58 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
}
}
+ /**
+ * A table-like moved to another namespace keeps its locations, so they must satisfy the location
+ * rules that creating it in the new namespace would enforce.
+ */
+ private void validateLocationsForRenamedTableLike(
+ PolarisEntitySubType subType,
+ TableIdentifier from,
+ TableIdentifier to,
+ IcebergTableLikeEntity entity,
+ PolarisResolvedPathWrapper resolvedEntities,
+ PolarisResolvedPathWrapper resolvedNewParent) {
+ Set<String> locations;
+ if (subType == PolarisEntitySubType.ICEBERG_VIEW) {
+ // Views do not persist their base location: it only exists in the view metadata.
+ String metadataLocation = entity.getMetadataLocation();
+ FileIO fileIO =
+ loadFileIOForTableLike(
+ from,
+ Set.of(metadataLocation.substring(0, metadataLocation.lastIndexOf("/"))),
+ resolvedEntities,
+ new HashMap<>(tableDefaultProperties),
+ Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));
+ locations = Set.of(ViewMetadataParser.read(fileIO, metadataLocation).location());
+ } else {
+ locations =
+ StorageUtil.getLocationsUsedByTable(
+ entity.getBaseLocation(), entity.getInternalPropertiesAsMap());
+ }
+ Set<String> locationsToValidate = new HashSet<>(locations);
+ if (entity.getMetadataLocation() != null) {
+ locationsToValidate.add(entity.getMetadataLocation());
+ }
+ validateLocationsForTableLike(to, locationsToValidate, resolvedNewParent);
+
+ // A rename does not change the entity's locations, so its relationship with entities outside
+ // the destination namespace does not change either: the optimized sibling check, which
+ // searches the whole catalog, must only consider the entities inside the destination location.
+ // Otherwise, it would report the namespaces that already contain the entity's locations, e.g.
+ // its previous parents.
+ StorageLocation destinationLocation =
+ StorageLocation.of(resolveLocationForPath(diagnostics, resolvedNewParent.getRawFullPath()));
+ for (String location : locations) {
+ validateNoLocationOverlap(
+ catalogEntity,
+ to,
+ resolvedNewParent.getRawFullPath(),
+ location,
+ entity,
+ destinationLocation);
+ }
+ }
+
/**
* Caller must fill in all entity fields except parentId, since the caller may not want to
* duplicate the logic to try to resolve parentIds before constructing the proposed entity. This
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java
index 6bf5e2868b..61d5fc3ae4 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractIcebergCatalogHandlerAuthzTest.java
@@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableMap;
import jakarta.inject.Inject;
import java.time.Duration;
import java.util.EnumSet;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -66,6 +67,7 @@ import org.apache.iceberg.view.ImmutableViewVersion;
import org.apache.polaris.core.admin.model.CreateCatalogRequest;
import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
+import org.apache.polaris.core.admin.model.UpdateCatalogRequest;
import org.apache.polaris.core.auth.PolarisPrincipal;
import org.apache.polaris.core.auth.PolarisPrincipalAttributes;
import org.apache.polaris.core.catalog.LocalCatalogFactory;
@@ -1325,8 +1327,26 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
.createTests();
}
+ /**
+ * A table or view renamed into another namespace keeps its location, outside the destination
+ * namespace, which is only allowed with unstructured table locations.
+ */
+ private void allowUnstructuredTableLocations() {
+ CatalogEntity catalog = newRootAdminService().getCatalog(CATALOG_NAME);
+ Map<String, String> properties = new HashMap<>(catalog.getPropertiesAsMap());
+ properties.put(FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true");
+ newRootAdminService()
+ .updateCatalog(
+ CATALOG_NAME,
+ UpdateCatalogRequest.builder()
+ .setCurrentEntityVersion(catalog.getEntityVersion())
+ .setProperties(properties)
+ .build());
+ }
+
@TestFactory
Stream<DynamicNode> testRenameTablePrivileges() {
+ allowUnstructuredTableLocations();
final TableIdentifier srcTable = TABLE_NS1_1;
final TableIdentifier dstTable = TableIdentifier.of(NS1AA, "newtable");
final RenameTableRequest rename1 =
@@ -1346,6 +1366,7 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
@Test
public void testRenameTablePrivilegesOnWrongSourceOrDestination() {
+ allowUnstructuredTableLocations();
final TableIdentifier srcTable = TABLE_NS2_1;
final TableIdentifier dstTable = TableIdentifier.of(NS1AA, "newtable");
final RenameTableRequest rename1 =
@@ -1607,6 +1628,7 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
@TestFactory
Stream<DynamicNode> testRenameViewPrivileges() {
+ allowUnstructuredTableLocations();
final TableIdentifier srcView = VIEW_NS1_1;
final TableIdentifier dstView = TableIdentifier.of(NS1AA, "newview");
final RenameTableRequest rename1 =
@@ -1626,6 +1648,7 @@ public abstract class AbstractIcebergCatalogHandlerAuthzTest extends PolarisAuth
@Test
public void testRenameViewPrivilegesOnWrongSourceOrDestination() {
+ allowUnstructuredTableLocations();
final TableIdentifier srcView = VIEW_NS2_1;
final TableIdentifier dstView = TableIdentifier.of(NS1AA, "newview");
final RenameTableRequest rename1 =
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java
index 057fdca903..3341b7a9d9 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java
@@ -19,6 +19,8 @@
package org.apache.polaris.service.catalog.iceberg;
+import static org.apache.polaris.core.config.FeatureConfiguration.ALLOW_TABLE_LOCATION_OVERLAP;
+import static org.apache.polaris.core.config.FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION;
import static org.apache.polaris.core.config.FeatureConfiguration.OPTIMIZED_SIBLING_CHECK;
import static org.apache.polaris.core.entity.table.IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY;
import static org.apache.polaris.service.admin.PolarisAuthzTestBase.SCHEMA;
@@ -63,6 +65,7 @@ import org.apache.iceberg.rest.requests.CreateTableRequest;
import org.apache.iceberg.rest.requests.CreateViewRequest;
import org.apache.iceberg.rest.requests.ImmutableCreateViewRequest;
import org.apache.iceberg.rest.requests.ImmutableRegisterTableRequest;
+import org.apache.iceberg.rest.requests.RenameTableRequest;
import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
import org.apache.iceberg.rest.requests.UpdateTableRequest;
import org.apache.iceberg.rest.responses.GetNamespaceResponse;
@@ -81,6 +84,9 @@ import org.apache.polaris.service.catalog.AccessDelegationMode;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;
public class IcebergAllowedLocationTest {
@@ -1163,6 +1169,387 @@ public class IcebergAllowedLocationTest {
.hasMessageContaining("outside the catalog's current allowed locations");
}
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testRenameTableRejectsNamespaceNotContainingItsLocation(
+ boolean optimizedSiblingCheck, @TempDir Path tmpDir) {
+ var services = getRenameTestServices(optimizedSiblingCheck);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(services, Map.of(), catalogLocation, List.of(catalogLocation));
+ createRenameNamespaces(services);
+ createTable(services, "src", "t", null);
+
+ TableIdentifier source = TableIdentifier.of("src", "t");
+ assertThatThrownBy(() -> renameTable(services, source, TableIdentifier.of("dst", "t")))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid locations");
+ newHandler(services).tableExists(source);
+
+ // Renaming within the namespace keeps the table inside the namespace location.
+ TableIdentifier renamed = TableIdentifier.of("src", "t2");
+ renameTable(services, source, renamed);
+ newHandler(services).tableExists(renamed);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testRenameTableToOtherNamespaceWithUnstructuredLocations(
+ boolean optimizedSiblingCheck, @TempDir Path tmpDir) {
+ var services = getRenameTestServices(optimizedSiblingCheck);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(
+ services,
+ Map.of(ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true"),
+ catalogLocation,
+ List.of(catalogLocation));
+ createRenameNamespaces(services);
+ createTable(services, "src", "t", null);
+
+ TableIdentifier renamed = TableIdentifier.of("dst", "t");
+ renameTable(services, TableIdentifier.of("src", "t"), renamed);
+ newHandler(services).tableExists(renamed);
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "true,t,true",
+ "false,t,true",
+ "true,child,true",
+ "false,child,true",
+ "true,t,false",
+ "false,t,false",
+ "true,child,false",
+ "false,child,false"
+ })
+ void testRenameTableIntoAncestorNamespaceRequiresLocationOverlap(
+ boolean allowOverlap,
+ String destinationName,
+ boolean optimizedSiblingCheck,
+ @TempDir Path tmpDir) {
+ var services = getRenameTestServices(optimizedSiblingCheck);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(
+ services,
+ allowOverlap ? Map.of(ALLOW_TABLE_LOCATION_OVERLAP.catalogConfig(), "true") : Map.of(),
+ catalogLocation,
+ List.of(catalogLocation));
+ Namespace parent = Namespace.of("parent");
+ Namespace child = Namespace.of("parent", "child");
+ Namespace sibling = Namespace.of("sibling");
+ for (Namespace ns : List.of(parent, child, sibling)) {
+ newHandler(services)
+ .createNamespace(CreateNamespaceRequest.builder().withNamespace(ns).build());
+ }
+ newHandler(services)
+ .createTableDirect(
+ child,
+ CreateTableRequest.builder().withName("t").withSchema(SCHEMA).build(),
+ EnumSet.noneOf(AccessDelegationMode.class),
+ Optional.empty());
+ TableIdentifier source = TableIdentifier.of(child, "t");
+
+ // The table's location is not under the sibling namespace, whether overlaps are allowed or not.
+ assertThatThrownBy(() -> renameTable(services, source, TableIdentifier.of(sibling, "t")))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid locations");
+
+ // The parent namespace contains the table's location, but so does the child namespace, which
+ // becomes the table's sibling. This holds even if the destination name equals the child
+ // namespace's name: tables and namespaces are distinct entities.
+ TableIdentifier renamed = TableIdentifier.of(parent, destinationName);
+ if (allowOverlap) {
+ renameTable(services, source, renamed);
+ newHandler(services).tableExists(renamed);
+ } else {
+ assertThatThrownBy(() -> renameTable(services, source, renamed))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("conflicts with existing table or namespace");
+ newHandler(services).tableExists(source);
+ }
+ }
+
+ @Test
+ void testRenameTableRejectsLocationOverlappingNewSiblings(@TempDir Path tmpDir) {
+ // Create two tables in different namespaces, but with overlapping locations, then attempt to
+ // rename one into the other's namespace.
+ // This test scenario is not possible with the optimized sibling check: table locations are
+ // checked globally per catalog, so the creation of the second table would have been rejected,
+ // even with ALLOW_UNSTRUCTURED_TABLE_LOCATION enabled.
+ var services = getRenameTestServices(false);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(
+ services,
+ Map.of(ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true"),
+ catalogLocation,
+ List.of(catalogLocation));
+ createRenameNamespaces(services);
+ String sharedLocation = catalogLocation + "/shared";
+ createTable(services, "dst", "a", sharedLocation);
+ createTable(services, "src", "b", sharedLocation + "/b");
+
+ assertThatThrownBy(
+ () ->
+ renameTable(
+ services, TableIdentifier.of("src", "b"), TableIdentifier.of("dst", "b")))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("conflicts with existing table or namespace");
+ }
+
+ @Test
+ void testRenameViewRejectsNamespaceNotContainingItsLocation(@TempDir Path tmpDir) {
+ var services = getRenameTestServices(false);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(services, Map.of(), catalogLocation, List.of(catalogLocation));
+ createRenameNamespaces(services);
+ createView(services, "src", "v");
+
+ TableIdentifier source = TableIdentifier.of("src", "v");
+ assertThatThrownBy(() -> renameView(services, source, TableIdentifier.of("dst", "v")))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid locations");
+ newHandler(services).viewExists(source);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testRenameViewToOtherNamespaceWithUnstructuredLocations(
+ boolean optimizedSiblingCheck, @TempDir Path tmpDir) {
+ var services = getRenameTestServices(optimizedSiblingCheck);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(
+ services,
+ Map.of(ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true"),
+ catalogLocation,
+ List.of(catalogLocation));
+ createRenameNamespaces(services);
+ createView(services, "src", "v");
+
+ TableIdentifier renamed = TableIdentifier.of("dst", "v");
+ renameView(services, TableIdentifier.of("src", "v"), renamed);
+ newHandler(services).viewExists(renamed);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testRepeatedRenamesWithUnstructuredLocations(
+ boolean optimizedSiblingCheck, @TempDir Path tmpDir) {
+ var services = getRenameTestServices(optimizedSiblingCheck);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(
+ services,
+ Map.of(ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true"),
+ catalogLocation,
+ List.of(catalogLocation));
+ createRenameNamespaces(services);
+ assertThat(createNamespaceStatus(services, "third", null))
+ .isEqualTo(Response.Status.OK.getStatusCode());
+ createTable(services, "src", "t", null);
+
+ // The table's data stays under the "src" namespace, whichever namespace it is renamed into.
+ TableIdentifier inDst = TableIdentifier.of("dst", "t");
+ renameTable(services, TableIdentifier.of("src", "t"), inDst);
+ newHandler(services).tableExists(inDst);
+
+ TableIdentifier inThird = TableIdentifier.of("third", "t");
+ renameTable(services, inDst, inThird);
+ newHandler(services).tableExists(inThird);
+
+ TableIdentifier backInSrc = TableIdentifier.of("src", "t");
+ renameTable(services, inThird, backInSrc);
+ newHandler(services).tableExists(backInSrc);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void testRenameViewIntoAncestorNamespaceRejectsOverlappingSibling(
+ boolean optimizedSiblingCheck, @TempDir Path tmpDir) {
+ var services = getRenameTestServices(optimizedSiblingCheck);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(services, Map.of(), catalogLocation, List.of(catalogLocation));
+ Namespace parent = Namespace.of("parent");
+ Namespace child = Namespace.of("parent", "child");
+ for (Namespace ns : List.of(parent, child)) {
+ newHandler(services)
+ .createNamespace(CreateNamespaceRequest.builder().withNamespace(ns).build());
+ }
+ newHandler(services)
+ .createView(
+ child,
+ ImmutableCreateViewRequest.builder()
+ .name("v")
+ .schema(SCHEMA)
+ .viewVersion(VIEW_VERSION)
+ .build());
+ TableIdentifier source = TableIdentifier.of(child, "v");
+
+ // The view's location is under the parent namespace, but also under the child namespace, which
+ // becomes the view's sibling.
+ assertThatThrownBy(() -> renameView(services, source, TableIdentifier.of(parent, "v")))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("conflicts with existing table or namespace");
+ newHandler(services).viewExists(source);
+ }
+
+ @Test
+ void testCredentialVendingRejectsTableOutsideItsNamespace(@TempDir Path tmpDir) {
+ var services = getRenameTestServices(false);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(
+ services,
+ Map.of(ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true"),
+ catalogLocation,
+ List.of(catalogLocation));
+ createRenameNamespaces(services);
+ createTable(services, "src", "t", null);
+
+ // While unstructured locations are allowed, the renamed table keeps its location in "src".
+ TableIdentifier renamed = TableIdentifier.of("dst", "t");
+ renameTable(services, TableIdentifier.of("src", "t"), renamed);
+ newHandler(services).loadCredentials(renamed, Optional.empty());
+
+ // Once they are disallowed, the table must not get credentials outside its namespace.
+ // The table becomes unusable.
+ updateCatalogProperties(
+ services, Map.of(ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "false"));
+ assertThatThrownBy(() -> newHandler(services).loadCredentials(renamed, Optional.empty()))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid locations");
+ assertThatThrownBy(
+ () ->
+ newHandler(services)
+ .loadTable(
+ renamed,
+ "all",
+ null,
+ EnumSet.of(AccessDelegationMode.VENDED_CREDENTIALS),
+ Optional.empty()))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid locations");
+ }
+
+ @Test
+ void testUpdateTableRejectsWriteDataPathOutsideTableLocation(@TempDir Path tmpDir) {
+ var services = getRenameTestServices(false);
+ var catalogLocation = tmpDir.resolve("warehouse").toAbsolutePath().toUri().toString();
+ createCatalog(services, Map.of(), catalogLocation, List.of(catalogLocation));
+ createRenameNamespaces(services);
+ createTable(services, "src", "t", null);
+
+ // Inside the namespace, but outside the table's own location.
+ TableIdentifier tableId = TableIdentifier.of("src", "t");
+ String writeDataPath = catalogLocation + "/" + catalog + "/src/elsewhere";
+ UpdateTableRequest updateRequest =
+ UpdateTableRequest.create(
+ tableId,
+ List.of(),
+ List.of(new MetadataUpdate.SetProperties(Map.of("write.data.path", writeDataPath))));
+ assertThatThrownBy(() -> newHandler(services).updateTable(tableId, updateRequest))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid locations");
+ }
+
+ private void updateCatalogProperties(TestServices services, Map<String, String> updates) {
+ Catalog fetched;
+ try (Response getResp =
+ services
+ .catalogsApi()
+ .getCatalog(catalog, services.realmContext(), services.securityContext())) {
+ assertThat(getResp.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
+ fetched = getResp.readEntity(Catalog.class);
+ }
+
+ Map<String, String> properties = new HashMap<>(fetched.getProperties().toMap());
+ properties.putAll(updates);
+ UpdateCatalogRequest updateRequest =
+ UpdateCatalogRequest.builder()
+ .setCurrentEntityVersion(fetched.getEntityVersion())
+ .setProperties(properties)
+ .build();
+ try (Response updateResp =
+ services
+ .catalogsApi()
+ .updateCatalog(
+ catalog, updateRequest, services.realmContext(), services.securityContext())) {
+ assertThat(updateResp.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
+ }
+ }
+
+ private static TestServices getRenameTestServices(boolean optimizedSiblingCheck) {
+ return TestServices.builder()
+ .config(
+ Map.of(
+ "ALLOW_INSECURE_STORAGE_TYPES",
+ "true",
+ "SUPPORTED_CATALOG_STORAGE_TYPES",
+ List.of("FILE"),
+ OPTIMIZED_SIBLING_CHECK.key(),
+ String.valueOf(optimizedSiblingCheck)))
+ .build();
+ }
+
+ private void createRenameNamespaces(TestServices services) {
+ assertThat(createNamespaceStatus(services, "src", null))
+ .isEqualTo(Response.Status.OK.getStatusCode());
+ assertThat(createNamespaceStatus(services, "dst", null))
+ .isEqualTo(Response.Status.OK.getStatusCode());
+ }
+
+ private void createTable(
+ TestServices services, String namespaceName, String tableName, String location) {
+ CreateTableRequest.Builder request =
+ CreateTableRequest.builder().withName(tableName).withSchema(SCHEMA);
+ if (location != null) {
+ request.withLocation(location);
+ }
+ try (Response response =
+ services
+ .restApi()
+ .createTable(
+ catalog,
+ namespaceName,
+ request.build(),
+ null,
+ IDEMPOTENCY_KEY,
+ services.realmContext(),
+ services.securityContext())) {
+ assertThat(response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
+ }
+ }
+
+ private void createView(TestServices services, String namespaceName, String viewName) {
+ CreateViewRequest request =
+ ImmutableCreateViewRequest.builder()
+ .name(viewName)
+ .schema(SCHEMA)
+ .viewVersion(VIEW_VERSION)
+ .build();
+ try (Response response =
+ services
+ .restApi()
+ .createView(
+ catalog,
+ namespaceName,
+ request,
+ services.realmContext(),
+ services.securityContext())) {
+ assertThat(response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode());
+ }
+ }
+
+ private void renameView(TestServices services, TableIdentifier from, TableIdentifier to) {
+ newHandler(services)
+ .renameView(RenameTableRequest.builder().withSource(from).withDestination(to).build());
+ }
+
+ private void renameTable(TestServices services, TableIdentifier from, TableIdentifier to) {
+ newHandler(services)
+ .renameTable(RenameTableRequest.builder().withSource(from).withDestination(to).build());
+ }
+
+ private IcebergCatalogHandler newHandler(TestServices services) {
+ return services.catalogAdapter().newHandler(services.securityContext(), catalog);
+ }
+
@SuppressWarnings({"deprecation", "RedundantSuppression"})
private record TestSnapshot(
long sequenceNumber,