gh-yzou commented on code in PR #4408:
URL: https://github.com/apache/polaris/pull/4408#discussion_r3307287931
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1242,70 +1241,87 @@ private <T extends PolarisEntity & LocationBasedEntity>
void validateNoLocationO
return Namespace.of(newLevels);
})
.toList();
- LOGGER.debug(
- "Resolving {} sibling entities to validate location",
- siblingTables.size() + siblingNamespaces.size());
- PolarisResolutionManifest resolutionManifest =
- new PolarisResolutionManifest(
- diagnostics,
- callContext.getRealmContext(),
- resolverFactory,
- principal,
- parentPath.getFirst().getName());
+ List<ResolvedPathKey> pathsToResolve =
+ new ArrayList<>(siblingTables.size() + siblingNamespaces.size());
siblingTables.forEach(
- tbl ->
- resolutionManifest.addPath(
- new ResolverPath(
- PolarisCatalogHelpers.tableIdentifierToList(tbl),
- PolarisEntityType.TABLE_LIKE)));
+ tbl -> {
+ if (!tbl.name().equals(name)) {
+ pathsToResolve.add(ResolvedPathKey.ofTableLike(tbl));
+ }
+ });
siblingNamespaces.forEach(
- ns ->
- resolutionManifest.addPath(
- new ResolverPath(Arrays.asList(ns.levels()),
PolarisEntityType.NAMESPACE)));
+ ns -> {
+ if (!ns.level(ns.length() - 1).equals(name)) {
+ pathsToResolve.add(ResolvedPathKey.ofNamespace(ns));
+ }
+ });
+
+ StorageLocation targetLocation = StorageLocation.of(location);
+ for (PolarisEntity entityToCheck :
+ resolveOptionalPaths(pathsToResolve, parentPath.getFirst().getName()))
{
+ String loc =
+
entityToCheck.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION);
Review Comment:
I think that was purely based on whether the property is set through the API
spec or not. i always had the confusion that the internal property is not for
user facing properties, but Eric was pointing out that it is the opposite way.
The location property is configured through spec API, and user facing, and
therefore it is under internal property.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1242,70 +1241,87 @@ private <T extends PolarisEntity & LocationBasedEntity>
void validateNoLocationO
return Namespace.of(newLevels);
})
.toList();
- LOGGER.debug(
- "Resolving {} sibling entities to validate location",
- siblingTables.size() + siblingNamespaces.size());
- PolarisResolutionManifest resolutionManifest =
- new PolarisResolutionManifest(
- diagnostics,
- callContext.getRealmContext(),
- resolverFactory,
- principal,
- parentPath.getFirst().getName());
+ List<ResolvedPathKey> pathsToResolve =
+ new ArrayList<>(siblingTables.size() + siblingNamespaces.size());
siblingTables.forEach(
- tbl ->
- resolutionManifest.addPath(
- new ResolverPath(
- PolarisCatalogHelpers.tableIdentifierToList(tbl),
- PolarisEntityType.TABLE_LIKE)));
+ tbl -> {
+ if (!tbl.name().equals(name)) {
+ pathsToResolve.add(ResolvedPathKey.ofTableLike(tbl));
+ }
+ });
siblingNamespaces.forEach(
- ns ->
- resolutionManifest.addPath(
- new ResolverPath(Arrays.asList(ns.levels()),
PolarisEntityType.NAMESPACE)));
+ ns -> {
+ if (!ns.level(ns.length() - 1).equals(name)) {
+ pathsToResolve.add(ResolvedPathKey.ofNamespace(ns));
+ }
+ });
+
+ StorageLocation targetLocation = StorageLocation.of(location);
+ for (PolarisEntity entityToCheck :
+ resolveOptionalPaths(pathsToResolve, parentPath.getFirst().getName()))
{
+ String loc =
+
entityToCheck.getPropertiesAsMap().get(PolarisEntityConstants.ENTITY_BASE_LOCATION);
Review Comment:
I think that was purely based on whether the property is set through the API
spec or not. i always had the confusion that the internal property is not for
user facing properties, but Eric was pointing out that it is the opposite way.
The location property is configured through spec API, and user facing, and
therefore it is under internal property based on guideline
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
