You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "iprithv (via GitHub)" <gi...@apache.org> on 2026/10/01 21:32:31 UTC
[PR] Reject tables and views created at their namespace's own location [polaris]
iprithv opened a new pull request, #5674:
URL: https://github.com/apache/polaris/pull/5674
The default (non-optimized) sibling-overlap check in `LocalIcebergCatalog.validateNoLocationOverlap` compared a new table/view location only against sibling tables and child namespaces — never the containing namespace itself. As a result a table or view created at **exactly its namespace's own location** was accepted, even though the optimized sibling check (`OPTIMIZED_SIBLING_CHECK=true`, used by JDBC/NoSQL) already rejects it.
Such a location is a prefix of every sibling created later under that namespace (blocking all of them) and over-scopes any vended credentials to the whole namespace subtree.
This change mirrors the optimized path in the list-based path: an entity whose location equals (or contains) an ancestor's location is rejected, while normal strict nesting under an ancestor remains allowed.
Current behavior differs by config: the same create is rejected with `OPTIMIZED_SIBLING_CHECK` enabled but silently accepted on the default path. The default path had no test coverage with overlap enforcement on — all existing overlap profiles force `OPTIMIZED_SIBLING_CHECK=true`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject tables and views created at their namespace's own location [polaris]
Posted by "iprithv (via GitHub)" <gi...@apache.org>.
iprithv commented on PR #5674:
URL: https://github.com/apache/polaris/pull/5674#issuecomment-6043922862
@dimas-b resolved conflicts :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject tables and views created at their namespace's own location [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5674:
URL: https://github.com/apache/polaris/pull/5674#discussion_r4171398272
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -1625,6 +1625,38 @@ private <T extends PolarisEntity & LocationBasedEntity> void validateNoLocationO
});
StorageLocation targetLocation = StorageLocation.of(location);
+
+ // A parent namespace naturally contains the entity's location when locations follow the
+ // namespace tree, as default locations do. That nesting is allowed, but the entity may not sit
+ // at exactly a containing namespace's location: such a location is a prefix of every sibling
+ // created later under that namespace (blocking all of them) and over-scopes any credentials
+ // vended for the entity to the whole namespace subtree. The optimized sibling check already
+ // rejects this; mirror the exact-location case here so the default list-based path has the same
+ // coverage. Only the equality case is rejected (and only against namespaces, not the catalog):
+ // an entity may legitimately sit at the catalog's own base location, and strict containment in
+ // either direction is handled by normal nesting and the sibling checks below.
+ for (PolarisEntity ancestor : parentPath) {
+ if (ancestor.getType() != PolarisEntityType.NAMESPACE) {
+ continue;
+ }
+ PolarisEntityUtils.asLocationBasedEntity(ancestor)
+ .map(LocationBasedEntity::getBaseLocation)
+ .filter(loc -> loc != null && !loc.isBlank())
+ .map(StorageLocation::of)
+ .ifPresent(
+ ancestorLocation -> {
+ // isChildOf treats equal locations as a match, so containment in both directions
+ // means the two locations are the same (ignoring trailing-slash differences).
+ if (targetLocation.isChildOf(ancestorLocation)
+ && ancestorLocation.isChildOf(targetLocation)) {
Review Comment:
nit: why not do a lenient equality comparison? Adding a method for that could probably be helpful in the future :thinking:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject tables and views created at their namespace's own location [polaris]
Posted by "iprithv (via GitHub)" <gi...@apache.org>.
iprithv commented on code in PR #5674:
URL: https://github.com/apache/polaris/pull/5674#discussion_r4177509113
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -1625,6 +1625,38 @@ private <T extends PolarisEntity & LocationBasedEntity> void validateNoLocationO
});
StorageLocation targetLocation = StorageLocation.of(location);
+
+ // A parent namespace naturally contains the entity's location when locations follow the
+ // namespace tree, as default locations do. That nesting is allowed, but the entity may not sit
+ // at exactly a containing namespace's location: such a location is a prefix of every sibling
+ // created later under that namespace (blocking all of them) and over-scopes any credentials
+ // vended for the entity to the whole namespace subtree. The optimized sibling check already
+ // rejects this; mirror the exact-location case here so the default list-based path has the same
+ // coverage. Only the equality case is rejected (and only against namespaces, not the catalog):
+ // an entity may legitimately sit at the catalog's own base location, and strict containment in
+ // either direction is handled by normal nesting and the sibling checks below.
+ for (PolarisEntity ancestor : parentPath) {
+ if (ancestor.getType() != PolarisEntityType.NAMESPACE) {
+ continue;
+ }
+ PolarisEntityUtils.asLocationBasedEntity(ancestor)
+ .map(LocationBasedEntity::getBaseLocation)
+ .filter(loc -> loc != null && !loc.isBlank())
+ .map(StorageLocation::of)
+ .ifPresent(
+ ancestorLocation -> {
+ // isChildOf treats equal locations as a match, so containment in both directions
+ // means the two locations are the same (ignoring trailing-slash differences).
+ if (targetLocation.isChildOf(ancestorLocation)
+ && ancestorLocation.isChildOf(targetLocation)) {
Review Comment:
Good call. Added `StorageLocation.isSameLocation(StorageLocation)` to handle trailing-slash and prefix differences. It uses `isChildOf` both ways, so provider-specific overrides still apply.
Also replaced the inline check with this method. PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject tables and views created at their namespace's own location [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #5674:
URL: https://github.com/apache/polaris/pull/5674
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Reject tables and views created at their namespace's own location [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5674:
URL: https://github.com/apache/polaris/pull/5674#discussion_r4171398272
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java:
##########
@@ -1625,6 +1625,38 @@ private <T extends PolarisEntity & LocationBasedEntity> void validateNoLocationO
});
StorageLocation targetLocation = StorageLocation.of(location);
+
+ // A parent namespace naturally contains the entity's location when locations follow the
+ // namespace tree, as default locations do. That nesting is allowed, but the entity may not sit
+ // at exactly a containing namespace's location: such a location is a prefix of every sibling
+ // created later under that namespace (blocking all of them) and over-scopes any credentials
+ // vended for the entity to the whole namespace subtree. The optimized sibling check already
+ // rejects this; mirror the exact-location case here so the default list-based path has the same
+ // coverage. Only the equality case is rejected (and only against namespaces, not the catalog):
+ // an entity may legitimately sit at the catalog's own base location, and strict containment in
+ // either direction is handled by normal nesting and the sibling checks below.
+ for (PolarisEntity ancestor : parentPath) {
+ if (ancestor.getType() != PolarisEntityType.NAMESPACE) {
+ continue;
+ }
+ PolarisEntityUtils.asLocationBasedEntity(ancestor)
+ .map(LocationBasedEntity::getBaseLocation)
+ .filter(loc -> loc != null && !loc.isBlank())
+ .map(StorageLocation::of)
+ .ifPresent(
+ ancestorLocation -> {
+ // isChildOf treats equal locations as a match, so containment in both directions
+ // means the two locations are the same (ignoring trailing-slash differences).
+ if (targetLocation.isChildOf(ancestorLocation)
+ && ancestorLocation.isChildOf(targetLocation)) {
Review Comment:
nit: why not do (add?) an equality comparison?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org