This is an automated email from the ASF dual-hosted git repository.
emaynard 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 6c02252e WASB Support (#344)
6c02252e is described below
commit 6c02252e339cedd53d7ac64a87ce52b3bb4d27d2
Author: Eric Maynard <[email protected]>
AuthorDate: Tue Oct 8 13:40:27 2024 -0400
WASB Support (#344)
* multiple prefixes
* stable tests
* autolint
* add todos
* implement todos
* autolint
* more fixes
* autolint
* fix test
* fix check
* move tenant ID to env var
* another fix
* autolint
* fix another location check (?)
* autolint
* clean up for review
* polish
* autolint
* adjustments per review
* autolint
---
.../core/storage/FileStorageConfigurationInfo.java | 4 +-
.../core/storage/InMemoryStorageIntegration.java | 31 +++-----
.../storage/PolarisStorageConfigurationInfo.java | 19 +++--
.../polaris/core/storage/StorageLocation.java | 90 ++++++++++++++++++++++
.../polaris/core/storage/azure/AzureLocation.java | 51 ++++++++++--
.../polaris/core/storage/StorageUtilTest.java | 2 +-
.../aws/AwsCredentialsStorageIntegrationTest.java | 2 +-
.../AzureCredentialStorageIntegrationTest.java | 7 +-
.../service/storage/azure/AzureLocationTest.java | 25 +++++-
.../polaris/service/admin/PolarisAdminService.java | 22 +++---
.../service/catalog/BasePolarisCatalog.java | 23 +++---
.../polaris/service/entity/CatalogEntityTest.java | 6 +-
12 files changed, 212 insertions(+), 70 deletions(-)
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/FileStorageConfigurationInfo.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/FileStorageConfigurationInfo.java
index e5e1a319..2847d5a8 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/FileStorageConfigurationInfo.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/FileStorageConfigurationInfo.java
@@ -20,6 +20,7 @@ package org.apache.polaris.core.storage;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.List;
+import java.util.Locale;
import org.jetbrains.annotations.NotNull;
/**
@@ -42,7 +43,8 @@ public class FileStorageConfigurationInfo extends
PolarisStorageConfigurationInf
@Override
public void validatePrefixForStorageType(String loc) {
- if (!loc.startsWith(getStorageType().getPrefix())
+ if (getStorageType().getPrefixes().stream()
+ .noneMatch(p -> loc.toLowerCase(Locale.ROOT).startsWith(p))
&& !loc.startsWith("file:/")
&& !loc.startsWith("/")
&& !loc.equals("*")) {
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java
index 4197760f..ce130668 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java
@@ -19,11 +19,10 @@
package org.apache.polaris.core.storage;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
-import java.util.NavigableSet;
import java.util.Optional;
import java.util.Set;
-import java.util.TreeSet;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.apache.polaris.core.context.CallContext;
@@ -62,8 +61,7 @@ public abstract class InMemoryStorageIntegration<T extends
PolarisStorageConfigu
@NotNull Set<String> locations) {
// trim trailing / from allowed locations so that locations missing the
trailing slash still
// match
- // TODO: Canonicalize with URI and compare scheme/authority/path
components separately
- TreeSet<String> allowedLocations =
+ Set<String> allowedLocationStrings =
storageConfig.getAllowedLocations().stream()
.map(
str -> {
@@ -74,7 +72,10 @@ public abstract class InMemoryStorageIntegration<T extends
PolarisStorageConfigu
}
})
.map(str -> str.replace("file:///", "file:/"))
- .collect(Collectors.toCollection(TreeSet::new));
+ .collect(Collectors.toSet());
+ List<StorageLocation> allowedLocations =
+
allowedLocationStrings.stream().map(StorageLocation::of).collect(Collectors.toList());
+
boolean allowWildcardLocation =
Optional.ofNullable(CallContext.getCurrentContext())
.flatMap(c -> Optional.ofNullable(c.getPolarisCallContext()))
@@ -84,7 +85,7 @@ public abstract class InMemoryStorageIntegration<T extends
PolarisStorageConfigu
.getConfiguration(pc, "ALLOW_WILDCARD_LOCATION",
false))
.orElse(false);
- if (allowWildcardLocation && allowedLocations.contains("*")) {
+ if (allowWildcardLocation && allowedLocationStrings.contains("*")) {
return locations.stream()
.collect(
Collectors.toMap(
@@ -100,21 +101,9 @@ public abstract class InMemoryStorageIntegration<T extends
PolarisStorageConfigu
}
Map<String, Map<PolarisStorageActions, ValidationResult>> resultMap = new
HashMap<>();
for (String rawLocation : locations) {
- String location = rawLocation.replace("file:///", "file:/");
- StringBuilder builder = new StringBuilder();
- NavigableSet<String> prefixes = allowedLocations;
- boolean validLocation = false;
- for (char c : location.toCharArray()) {
- builder.append(c);
- prefixes = allowedLocations.tailSet(builder.toString(), true);
- if (prefixes.isEmpty()) {
- break;
- } else if (prefixes.first().equals(builder.toString())) {
- validLocation = true;
- break;
- }
- }
- final boolean isValidLocation = validLocation;
+ StorageLocation storageLocation = StorageLocation.of(rawLocation);
+ final boolean isValidLocation =
+ allowedLocations.stream().anyMatch(storageLocation::isChildOf);
Map<PolarisStorageActions, ValidationResult> locationResult =
actions.stream()
.collect(
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 0239a9fe..4b284926 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
@@ -222,10 +222,11 @@ public abstract class PolarisStorageConfigurationInfo {
/** Validate if the provided allowed locations are valid for the storage
type */
protected void validatePrefixForStorageType(String loc) {
- if (!loc.toLowerCase(Locale.ROOT).startsWith(storageType.prefix)) {
+ if (storageType.prefixes.stream().noneMatch(p ->
loc.toLowerCase(Locale.ROOT).startsWith(p))) {
throw new IllegalArgumentException(
String.format(
- "Location prefix not allowed: '%s', expected prefix: '%s'", loc,
storageType.prefix));
+ "Location prefix not allowed: '%s', expected prefixes: '%s'",
+ loc, String.join(",", storageType.prefixes)));
}
}
@@ -240,19 +241,23 @@ public abstract class PolarisStorageConfigurationInfo {
/** Polaris' storage type, each has a fixed prefix for its location */
public enum StorageType {
S3("s3://"),
- AZURE("abfs"), // abfs or abfss
+ AZURE(List.of("abfs://", "wasb://", "abfss://", "wasbs://")),
GCS("gs://"),
FILE("file://"),
;
- final String prefix;
+ private final List<String> prefixes;
StorageType(String prefix) {
- this.prefix = prefix;
+ this.prefixes = List.of(prefix);
}
- public String getPrefix() {
- return prefix;
+ StorageType(List<String> prefixes) {
+ this.prefixes = prefixes;
+ }
+
+ public List<String> getPrefixes() {
+ return prefixes;
}
}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageLocation.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageLocation.java
new file mode 100644
index 00000000..51def1fb
--- /dev/null
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageLocation.java
@@ -0,0 +1,90 @@
+/*
+ * 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.storage;
+
+import jakarta.validation.constraints.NotNull;
+import java.net.URI;
+import org.apache.polaris.core.storage.azure.AzureLocation;
+
+/** An abstraction over a storage location */
+public class StorageLocation {
+ private final String location;
+
+ /** Create a StorageLocation from a String path */
+ public static StorageLocation of(String location) {
+ // TODO implement StorageLocation for all supported file systems and add
isValidLocation
+ if (AzureLocation.isAzureLocation(location)) {
+ return new AzureLocation(location);
+ } else {
+ return new StorageLocation(location);
+ }
+ }
+
+ protected StorageLocation(@NotNull String location) {
+ if (location == null) {
+ this.location = null;
+ } else if (location.startsWith("file:/") &&
!location.startsWith("file:///")) {
+ this.location = URI.create(location.replaceFirst("file:/+",
"file:///")).toString();
+ } else {
+ this.location = URI.create(location).toString();
+ }
+ }
+
+ /** If a path doesn't end in `/`, this will add one */
+ protected final String ensureTrailingSlash(String location) {
+ if (location == null || location.endsWith("/")) {
+ return location;
+ } else {
+ return location + "/";
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ return location.hashCode();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (obj instanceof StorageLocation) {
+ return location.equals(((StorageLocation) obj).location);
+ } else {
+ return false;
+ }
+ }
+
+ @Override
+ public String toString() {
+ return location;
+ }
+
+ /**
+ * Returns true if this StorageLocation's location string starts with the
other StorageLocation's
+ * location string
+ */
+ public boolean isChildOf(StorageLocation potentialParent) {
+ if (this.location == null || potentialParent.location == null) {
+ return false;
+ } else {
+ String slashTerminatedLocation = ensureTrailingSlash(this.location);
+ String slashTerminatedParentLocation =
ensureTrailingSlash(potentialParent.location);
+ return slashTerminatedLocation.startsWith(slashTerminatedParentLocation);
+ }
+ }
+}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureLocation.java
b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureLocation.java
index 80b86f53..e7211593 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureLocation.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureLocation.java
@@ -20,17 +20,18 @@ package org.apache.polaris.core.storage.azure;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.apache.polaris.core.storage.StorageLocation;
import org.jetbrains.annotations.NotNull;
/** This class represents all information for a azure location */
-public class AzureLocation {
- /** The pattern only allows abfs[s] now because the ResovlingFileIO only
accept ADLSFileIO */
- private static final Pattern URI_PATTERN =
Pattern.compile("^abfss?://([^/?#]+)(.*)?$");
+public class AzureLocation extends StorageLocation {
+ private static final Pattern URI_PATTERN =
Pattern.compile("^(abfss?|wasbs?)://([^/?#]+)(.*)?$");
public static final String ADLS_ENDPOINT = "dfs.core.windows.net";
public static final String BLOB_ENDPOINT = "blob.core.windows.net";
+ private final String scheme;
private final String storageAccount;
private final String container;
@@ -40,16 +41,19 @@ public class AzureLocation {
/**
* Construct an Azure location object from a location uri, it should follow
this pattern:
*
- * <p>{@code abfs[s]://[<container>@]<storage account host>/<file path>}
+ * <p>{@code (abfs|wasb)[s]://[<container>@]<storage account host>/<file
path>}
*
* @param location a uri
*/
public AzureLocation(@NotNull String location) {
+ super(location);
+
Matcher matcher = URI_PATTERN.matcher(location);
if (!matcher.matches()) {
- throw new IllegalArgumentException("Invalid azure adls location uri " +
location);
+ throw new IllegalArgumentException("Invalid azure location uri " +
location);
}
- String authority = matcher.group(1);
+ this.scheme = matcher.group(1);
+ String authority = matcher.group(2);
// look for <container>@<storage account host>
String[] parts = authority.split("@", -1);
@@ -65,7 +69,7 @@ public class AzureLocation {
}
this.storageAccount = hostParts[0];
this.endpoint = hostParts[1];
- String path = matcher.group(2);
+ String path = matcher.group(3);
filePath = path == null ? "" : path.startsWith("/") ? path.substring(1) :
path;
}
@@ -88,4 +92,37 @@ public class AzureLocation {
public String getFilePath() {
return filePath;
}
+
+ /** Get the scheme */
+ public String getScheme() {
+ return scheme;
+ }
+
+ /**
+ * Returns true if the object this StorageLocation refers to is a child of
the object referred to
+ * by the other StorageLocation.
+ */
+ @Override
+ public boolean isChildOf(@NotNull StorageLocation potentialParent) {
+ if (potentialParent instanceof AzureLocation) {
+ AzureLocation potentialAzureParent = (AzureLocation) potentialParent;
+ if (this.container.equals(potentialAzureParent.container)) {
+ if (this.storageAccount.equals(potentialAzureParent.storageAccount)) {
+ String slashTerminatedFilePath = ensureTrailingSlash(this.filePath);
+ String slashTerminatedParentFilePath =
ensureTrailingSlash(potentialAzureParent.filePath);
+ return
slashTerminatedFilePath.startsWith(slashTerminatedParentFilePath);
+ }
+ }
+ }
+ return false;
+ }
+
+ /** Return true if the input location appears to be an Azure path */
+ public static boolean isAzureLocation(String location) {
+ if (location == null) {
+ return false;
+ }
+ Matcher matcher = URI_PATTERN.matcher(location);
+ return matcher.matches();
+ }
}
diff --git
a/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java
b/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java
index 8e842d24..99a80d75 100644
---
a/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java
+++
b/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java
@@ -31,7 +31,7 @@ public class StorageUtilTest {
}
@ParameterizedTest
- @ValueSource(strings = {"s3", "gcs", "abfs", "file"})
+ @ValueSource(strings = {"s3", "gcs", "abfs", "wasb", "file"})
public void testAbsolutePaths(String scheme) {
Assertions.assertThat(StorageUtil.getBucket(scheme +
"://bucket/path/file.txt"))
.isEqualTo("bucket");
diff --git
a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
index 6f0b564b..f152bd88 100644
---
a/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
+++
b/polaris-core/src/test/java/org/apache/polaris/service/storage/aws/AwsCredentialsStorageIntegrationTest.java
@@ -496,6 +496,6 @@ class AwsCredentialsStorageIntegrationTest {
private static @NotNull String s3Path(
String bucket, String keyPrefix,
PolarisStorageConfigurationInfo.StorageType storageType) {
- return storageType.getPrefix() + bucket + "/" + keyPrefix;
+ return "s3://" + bucket + "/" + keyPrefix;
}
}
diff --git
a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java
b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java
index 7ec4a910..fd7ac9a8 100644
---
a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java
+++
b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureCredentialStorageIntegrationTest.java
@@ -65,10 +65,13 @@ public class AzureCredentialStorageIntegrationTest {
private final String clientId = System.getenv("AZURE_CLIENT_ID");
private final String clientSecret = System.getenv("AZURE_CLIENT_SECRET");
- private final String tenantId = "d479c7c9-2632-445a-b22d-7c19e68774f6";
+ private final String tenantId = System.getenv("AZURE_TENANT_ID");
private void assumeEnvVariablesNotNull() {
- Assumptions.assumeThat(Strings.isNullOrEmpty(clientId) ||
Strings.isNullOrEmpty(clientSecret))
+ Assumptions.assumeThat(
+ Strings.isNullOrEmpty(clientId)
+ || Strings.isNullOrEmpty(clientSecret)
+ || Strings.isNullOrEmpty(tenantId))
.describedAs("Null Azure testing environment variables!")
.isFalse();
}
diff --git
a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureLocationTest.java
b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureLocationTest.java
index f08ae800..8040d289 100644
---
a/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureLocationTest.java
+++
b/polaris-core/src/test/java/org/apache/polaris/service/storage/azure/AzureLocationTest.java
@@ -18,6 +18,7 @@
*/
package org.apache.polaris.service.storage.azure;
+import org.apache.polaris.core.storage.StorageLocation;
import org.apache.polaris.core.storage.azure.AzureLocation;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -34,12 +35,28 @@ public class AzureLocationTest {
Assertions.assertThat(azureLocation.getFilePath()).isEqualTo("myfile");
}
+ @Test
+ public void testWasbLocation() {
+ String uri =
"wasb://[email protected]/myfile";
+ AzureLocation azureLocation = new AzureLocation(uri);
+ Assertions.assertThat(azureLocation.getContainer()).isEqualTo("container");
+
Assertions.assertThat(azureLocation.getStorageAccount()).isEqualTo("storageaccount");
+
Assertions.assertThat(azureLocation.getEndpoint()).isEqualTo("blob.core.windows.net");
+ Assertions.assertThat(azureLocation.getFilePath()).isEqualTo("myfile");
+ }
+
+ @Test
+ public void testCrossSchemeComparisons() {
+ StorageLocation abfsLocation =
+
AzureLocation.of("abfss://[email protected]/some/file/x");
+ StorageLocation wasbLocation =
+
AzureLocation.of("wasb://[email protected]/some/file");
+ Assertions.assertThat(abfsLocation).isNotEqualTo(wasbLocation);
+ Assertions.assertThat(abfsLocation.isChildOf(wasbLocation)).isTrue();
+ }
+
@Test
public void testLocation_negative_cases() {
- Assertions.assertThatThrownBy(
- () ->
- new
AzureLocation("wasbs://[email protected]/myfile"))
- .isInstanceOf(IllegalArgumentException.class);
Assertions.assertThatThrownBy(
() -> new
AzureLocation("abfss://storageaccount.blob.core.windows.net/myfile"))
.isInstanceOf(IllegalArgumentException.class);
diff --git
a/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
b/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
index c0dfcd81..9c93f3e5 100644
---
a/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
+++
b/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java
@@ -78,6 +78,7 @@ import
org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest;
import org.apache.polaris.core.persistence.resolver.ResolverPath;
import org.apache.polaris.core.persistence.resolver.ResolverStatus;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
+import org.apache.polaris.core.storage.StorageLocation;
import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo;
import org.jetbrains.annotations.NotNull;
@@ -517,17 +518,18 @@ public class PolarisAdminService {
return false;
}
return getCatalogLocations(existingCatalog).stream()
+ .map(StorageLocation::of)
.anyMatch(
- existingLocation ->
- newCatalogLocations.stream()
- .anyMatch(
- newLocation -> {
- if (newLocation == null ||
existingLocation == null) {
- return false;
- }
- return
newLocation.startsWith(existingLocation)
- ||
existingLocation.startsWith(newLocation);
- }));
+ existingLocation -> {
+ return newCatalogLocations.stream()
+ .anyMatch(
+ newLocationString -> {
+ StorageLocation newLocation =
+ StorageLocation.of(newLocationString);
+ return
newLocation.isChildOf(existingLocation)
+ ||
existingLocation.isChildOf(newLocation);
+ });
+ });
});
}
diff --git
a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
index 7b687e9a..fc552a5b 100644
---
a/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
+++
b/polaris-service/src/main/java/org/apache/polaris/service/catalog/BasePolarisCatalog.java
@@ -25,7 +25,6 @@ import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import java.io.Closeable;
import java.io.IOException;
-import java.net.URI;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -98,6 +97,7 @@ import
org.apache.polaris.core.storage.InMemoryStorageIntegration;
import org.apache.polaris.core.storage.PolarisStorageActions;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageIntegration;
+import org.apache.polaris.core.storage.StorageLocation;
import org.apache.polaris.core.storage.aws.PolarisS3FileIOClientFactory;
import org.apache.polaris.service.task.TaskExecutor;
import org.apache.polaris.service.types.NotificationRequest;
@@ -1125,6 +1125,8 @@ public class BasePolarisCatalog extends
BaseMetastoreViewCatalog
"Unable to resolve sibling entities to validate location - could not
resolve"
+ status.getFailedToResolvedEntityName());
}
+
+ StorageLocation targetLocation = StorageLocation.of(location);
Stream.concat(
siblingTables.stream()
.filter(tbl -> !tbl.name().equals(name))
@@ -1145,15 +1147,14 @@ public class BasePolarisCatalog extends
BaseMetastoreViewCatalog
.getBaseLocation();
}))
.filter(java.util.Objects::nonNull)
+ .map(StorageLocation::of)
.forEach(
siblingLocation -> {
- URI target = URI.create(location);
- URI existing = URI.create(siblingLocation);
- if (isUnderParentLocation(target, existing)
- || isUnderParentLocation(existing, target)) {
+ if (targetLocation.isChildOf(siblingLocation)
+ || siblingLocation.isChildOf(targetLocation)) {
throw new org.apache.iceberg.exceptions.ForbiddenException(
"Unable to create table at location '%s' because it
conflicts with existing table or namespace at location '%s'",
- target, existing);
+ targetLocation, siblingLocation);
}
});
}
@@ -1406,9 +1407,9 @@ public class BasePolarisCatalog extends
BaseMetastoreViewCatalog
metadata.location(),
identifier,
metadata.metadataFileLocation());
- if (!isUnderParentLocation(
- URI.create(metadata.metadataFileLocation()),
- URI.create(metadata.location() + "/metadata").normalize())) {
+ StorageLocation metadataFileLocation =
StorageLocation.of(metadata.metadataFileLocation());
+ StorageLocation baseLocation = StorageLocation.of(metadata.location());
+ if (!metadataFileLocation.isChildOf(baseLocation)) {
throw new BadRequestException(
"Metadata location %s is not allowed outside of table location %s",
metadata.metadataFileLocation(), metadata.location());
@@ -1792,10 +1793,6 @@ public class BasePolarisCatalog extends
BaseMetastoreViewCatalog
}
}
- private static boolean isUnderParentLocation(URI childLocation, URI
expectedParentLocation) {
- return
!expectedParentLocation.relativize(childLocation).equals(childLocation);
- }
-
private void updateTableLike(TableIdentifier identifier, PolarisEntity
entity) {
PolarisResolvedPathWrapper resolvedEntities =
resolvedEntityView.getResolvedPath(identifier, entity.getSubType());
diff --git
a/polaris-service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java
b/polaris-service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java
index 1381ea0e..167a8768 100644
---
a/polaris-service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java
+++
b/polaris-service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java
@@ -56,7 +56,7 @@ public class CatalogEntityTest {
Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(awsCatalog))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining(
- "Location prefix not allowed: 'unsupportPrefix://mybucket/path',
expected prefix: 's3://'");
+ "Location prefix not allowed: 'unsupportPrefix://mybucket/path',
expected prefixes");
// Invalid azure prefix
AzureStorageConfigInfo azureStorageConfigModel =
@@ -76,7 +76,7 @@ public class CatalogEntityTest {
.build();
Assertions.assertThatThrownBy(() ->
CatalogEntity.fromCatalog(azureCatalog))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("Invalid azure adls location uri
unsupportPrefix://mybucket/path");
+ .hasMessageContaining("Invalid azure location uri
unsupportPrefix://mybucket/path");
// invalid gcp prefix
GcpStorageConfigInfo gcpStorageConfigModel =
@@ -94,7 +94,7 @@ public class CatalogEntityTest {
Assertions.assertThatThrownBy(() -> CatalogEntity.fromCatalog(gcpCatalog))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining(
- "Location prefix not allowed: 'unsupportPrefix://mybucket/path',
expected prefix: 'gs://'");
+ "Location prefix not allowed: 'unsupportPrefix://mybucket/path',
expected prefixes");
}
@Test
