gh-yzou commented on code in PR #1264:
URL: https://github.com/apache/polaris/pull/1264#discussion_r2019441012
##########
polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java:
##########
@@ -41,21 +42,45 @@ public enum PolarisPrivilege {
TABLE_CREATE(6, PolarisEntityType.NAMESPACE),
VIEW_CREATE(7, PolarisEntityType.NAMESPACE),
NAMESPACE_DROP(8, PolarisEntityType.NAMESPACE),
- TABLE_DROP(9, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_TABLE),
+ TABLE_DROP(
+ 9,
+ PolarisEntityType.TABLE_LIKE,
+ List.of(PolarisEntitySubType.ICEBERG_TABLE,
PolarisEntitySubType.GENERIC_TABLE),
+ PolarisEntityType.CATALOG_ROLE),
VIEW_DROP(10, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_VIEW),
NAMESPACE_LIST(11, PolarisEntityType.NAMESPACE),
TABLE_LIST(12, PolarisEntityType.NAMESPACE),
VIEW_LIST(13, PolarisEntityType.NAMESPACE),
NAMESPACE_READ_PROPERTIES(14, PolarisEntityType.NAMESPACE),
- TABLE_READ_PROPERTIES(15, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_TABLE),
+ TABLE_READ_PROPERTIES(
+ 15,
+ PolarisEntityType.TABLE_LIKE,
+ List.of(PolarisEntitySubType.ICEBERG_TABLE,
PolarisEntitySubType.GENERIC_TABLE),
+ PolarisEntityType.CATALOG_ROLE),
VIEW_READ_PROPERTIES(16, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_VIEW),
NAMESPACE_WRITE_PROPERTIES(17, PolarisEntityType.NAMESPACE),
- TABLE_WRITE_PROPERTIES(18, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_TABLE),
+ TABLE_WRITE_PROPERTIES(
+ 18,
+ PolarisEntityType.TABLE_LIKE,
+ List.of(PolarisEntitySubType.ICEBERG_TABLE,
PolarisEntitySubType.GENERIC_TABLE),
Review Comment:
I don't think we are providing that capabilities to generic tables today. we
can probably still keep it here for consistency, but will have no effect.
##########
polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java:
##########
@@ -162,7 +208,7 @@ public enum PolarisPrivilege {
private final PolarisEntityType securableType;
// the subtype of the securable for this privilege
- private final PolarisEntitySubType securableSubType;
+ private final List<PolarisEntitySubType> securableSubTypes;
Review Comment:
I would expect that we will need to update somewhere for the authentication
implementation to make sure it an understand the list of subtypes now, but
didn't see it, did I miss it somewhere? I am also fine that we do the actual
privilege in a separate PR.
##########
polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrivilege.java:
##########
@@ -41,21 +42,45 @@ public enum PolarisPrivilege {
TABLE_CREATE(6, PolarisEntityType.NAMESPACE),
VIEW_CREATE(7, PolarisEntityType.NAMESPACE),
NAMESPACE_DROP(8, PolarisEntityType.NAMESPACE),
- TABLE_DROP(9, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_TABLE),
+ TABLE_DROP(
+ 9,
+ PolarisEntityType.TABLE_LIKE,
+ List.of(PolarisEntitySubType.ICEBERG_TABLE,
PolarisEntitySubType.GENERIC_TABLE),
+ PolarisEntityType.CATALOG_ROLE),
VIEW_DROP(10, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_VIEW),
NAMESPACE_LIST(11, PolarisEntityType.NAMESPACE),
TABLE_LIST(12, PolarisEntityType.NAMESPACE),
VIEW_LIST(13, PolarisEntityType.NAMESPACE),
NAMESPACE_READ_PROPERTIES(14, PolarisEntityType.NAMESPACE),
- TABLE_READ_PROPERTIES(15, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_TABLE),
+ TABLE_READ_PROPERTIES(
+ 15,
+ PolarisEntityType.TABLE_LIKE,
Review Comment:
Do you know what READ_PROPERTIES privilege means here? does it mean we if
there is not read properties privilege, the load table response will not have
any properties show up in the property fields ? if that is the case, we
probably need to make sure we respect that for generic table also.
##########
service/common/src/main/java/org/apache/polaris/service/catalog/generic/GenericTableCatalogHandlerWrapper.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.service.catalog.generic;
+
+import jakarta.ws.rs.core.SecurityContext;
+import java.util.Map;
+import java.util.TreeSet;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.polaris.core.auth.PolarisAuthorizableOperation;
+import org.apache.polaris.core.auth.PolarisAuthorizer;
+import org.apache.polaris.core.config.FeatureConfiguration;
+import org.apache.polaris.core.context.CallContext;
+import org.apache.polaris.core.entity.PolarisEntitySubType;
+import org.apache.polaris.core.entity.table.GenericTableEntity;
+import org.apache.polaris.core.persistence.PolarisEntityManager;
+import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
+import org.apache.polaris.service.catalog.common.CatalogHandlerWrapper;
+import org.apache.polaris.service.types.GenericTable;
+import org.apache.polaris.service.types.ListGenericTablesResponse;
+import org.apache.polaris.service.types.LoadGenericTableResponse;
+
+public class GenericTableCatalogHandlerWrapper extends CatalogHandlerWrapper {
+
+ private PolarisMetaStoreManager metaStoreManager;
+
+ private GenericTableCatalog genericTableCatalog;
+
+ public GenericTableCatalogHandlerWrapper(
+ CallContext callContext,
+ PolarisEntityManager entityManager,
+ PolarisMetaStoreManager metaStoreManager,
+ SecurityContext securityContext,
+ String catalogName,
+ PolarisAuthorizer authorizer) {
+ super(callContext, entityManager, securityContext, catalogName,
authorizer);
+ this.metaStoreManager = metaStoreManager;
+ }
+
+ public void enforceGenericTablesEnabledOrThrow() {
+ boolean enabled =
+ callContext
+ .getPolarisCallContext()
+ .getConfigurationStore()
+ .getConfiguration(
+ callContext.getPolarisCallContext(),
FeatureConfiguration.ENABLE_GENERIC_TABLES);
+ if (!enabled) {
+ throw new UnsupportedOperationException("Generic table support is not
enabled");
+ }
+ }
+
+ @Override
+ protected void initializeCatalog() {
+ enforceGenericTablesEnabledOrThrow();
+ this.genericTableCatalog =
+ new GenericTableCatalog(metaStoreManager, callContext,
this.resolutionManifest);
+ }
+
+ public ListGenericTablesResponse listGenericTables(Namespace parent) {
+ PolarisAuthorizableOperation op = PolarisAuthorizableOperation.LIST_TABLES;
+ authorizeBasicNamespaceOperationOrThrow(op, parent);
+
+ return ListGenericTablesResponse.builder()
+ .setIdentifiers(new
TreeSet<>(genericTableCatalog.listGenericTables(parent)))
+ .build();
+ }
+
+ public LoadGenericTableResponse createGenericTable(
+ TableIdentifier identifier, String format, String doc, Map<String,
String> properties) {
+ PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.CREATE_TABLE_DIRECT;
+ authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier);
+
+ GenericTableEntity createdEntity =
+ this.genericTableCatalog.createGenericTable(identifier, format, doc,
properties);
+ GenericTable createdTable =
+ new GenericTable(
+ createdEntity.getName(),
+ createdEntity.getFormat(),
+ createdEntity.getDoc(),
+ createdEntity.getPropertiesAsMap());
+
+ return LoadGenericTableResponse.builder().setTable(createdTable).build();
+ }
+
+ public boolean dropGenericTable(TableIdentifier identifier) {
+ PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.DROP_TABLE_WITHOUT_PURGE;
+ authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier);
Review Comment:
That was the proposal in the design, which seems to me the most clear one to
the user, since privileges are assigned by admins. It would be confusing that
TABLE_DROP only works for Iceberg, and then have a GENERIC_TABLE_DROP for
generic table. It would be more user friendly that TABLE_DROP is for all table
drop, and later we can introduce ICEBERG_TABLE_DROP, GENEIC_TABLE_DROP for fine
grained control. Of course, we can see what other people think also.
I am also fine if we want to do the change in a separate PR to keep the
purpose more clear. @dennishuo It would be great if you could also help take a
look at this PR.
##########
service/common/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandlerWrapper.java:
##########
@@ -0,0 +1,354 @@
+/*
+ * 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.service.catalog.common;
+
+import jakarta.ws.rs.core.SecurityContext;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.exceptions.NoSuchTableException;
+import org.apache.iceberg.exceptions.NoSuchViewException;
+import org.apache.polaris.core.PolarisDiagnostics;
+import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal;
+import org.apache.polaris.core.auth.PolarisAuthorizableOperation;
+import org.apache.polaris.core.auth.PolarisAuthorizer;
+import org.apache.polaris.core.catalog.PolarisCatalogHelpers;
+import org.apache.polaris.core.context.CallContext;
+import org.apache.polaris.core.entity.PolarisEntitySubType;
+import org.apache.polaris.core.entity.PolarisEntityType;
+import org.apache.polaris.core.persistence.PolarisEntityManager;
+import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
+import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest;
+import org.apache.polaris.core.persistence.resolver.ResolverPath;
+import org.apache.polaris.core.persistence.resolver.ResolverStatus;
+
+/**
+ * An ABC for catalog wrappers which provides authorize methods that should be
called before a
+ * request is actually forwarded to a catalog. Child types must implement
`initializeCatalog` which
+ * will be called after a successful authorization.
+ */
+public abstract class CatalogHandlerWrapper {
+
+ // Initialized in the authorize methods.
+ protected PolarisResolutionManifest resolutionManifest = null;
+
+ protected final CallContext callContext;
+ protected final PolarisEntityManager entityManager;
+ protected final String catalogName;
+ protected final AuthenticatedPolarisPrincipal authenticatedPrincipal;
+ protected final SecurityContext securityContext;
+ protected final PolarisAuthorizer authorizer;
+
+ public CatalogHandlerWrapper(
+ CallContext callContext,
+ PolarisEntityManager entityManager,
+ SecurityContext securityContext,
+ String catalogName,
+ PolarisAuthorizer authorizer) {
+ this.callContext = callContext;
+ this.entityManager = entityManager;
+ this.catalogName = catalogName;
+ PolarisDiagnostics diagServices =
callContext.getPolarisCallContext().getDiagServices();
+ diagServices.checkNotNull(securityContext, "null_security_context");
+ diagServices.checkNotNull(securityContext.getUserPrincipal(),
"null_user_principal");
+ diagServices.check(
+ securityContext.getUserPrincipal() instanceof
AuthenticatedPolarisPrincipal,
+ "invalid_principal_type",
+ "Principal must be an AuthenticatedPolarisPrincipal");
+ this.securityContext = securityContext;
+ this.authenticatedPrincipal =
+ (AuthenticatedPolarisPrincipal) securityContext.getUserPrincipal();
+ this.authorizer = authorizer;
+ }
+
+ /** Initialize the catalog once authorized. Called after all `authorize...`
methods. */
+ protected abstract void initializeCatalog();
+
+ protected void authorizeBasicNamespaceOperationOrThrow(
+ PolarisAuthorizableOperation op, Namespace namespace) {
+ authorizeBasicNamespaceOperationOrThrow(op, namespace, null, null);
+ }
+
+ protected void authorizeBasicNamespaceOperationOrThrow(
+ PolarisAuthorizableOperation op,
+ Namespace namespace,
+ List<Namespace> extraPassthroughNamespaces,
+ List<TableIdentifier> extraPassthroughTableLikes) {
+ resolutionManifest =
+ entityManager.prepareResolutionManifest(callContext, securityContext,
catalogName);
+ resolutionManifest.addPath(
+ new ResolverPath(Arrays.asList(namespace.levels()),
PolarisEntityType.NAMESPACE),
+ namespace);
+
+ if (extraPassthroughNamespaces != null) {
+ for (Namespace ns : extraPassthroughNamespaces) {
+ resolutionManifest.addPassthroughPath(
+ new ResolverPath(
+ Arrays.asList(ns.levels()), PolarisEntityType.NAMESPACE, true
/* optional */),
+ ns);
+ }
+ }
+ if (extraPassthroughTableLikes != null) {
+ for (TableIdentifier id : extraPassthroughTableLikes) {
+ resolutionManifest.addPassthroughPath(
+ new ResolverPath(
+ PolarisCatalogHelpers.tableIdentifierToList(id),
+ PolarisEntityType.TABLE_LIKE,
+ true /* optional */),
+ id);
+ }
+ }
+ resolutionManifest.resolveAll();
+ PolarisResolvedPathWrapper target =
resolutionManifest.getResolvedPath(namespace, true);
+ if (target == null) {
+ throw new NoSuchNamespaceException("Namespace does not exist: %s",
namespace);
+ }
+ authorizer.authorizeOrThrow(
+ authenticatedPrincipal,
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ op,
+ target,
+ null /* secondary */);
+
+ initializeCatalog();
+ }
+
+ protected void authorizeCreateNamespaceUnderNamespaceOperationOrThrow(
+ PolarisAuthorizableOperation op, Namespace namespace) {
+ resolutionManifest =
+ entityManager.prepareResolutionManifest(callContext, securityContext,
catalogName);
+
+ Namespace parentNamespace =
PolarisCatalogHelpers.getParentNamespace(namespace);
+ resolutionManifest.addPath(
+ new ResolverPath(Arrays.asList(parentNamespace.levels()),
PolarisEntityType.NAMESPACE),
+ parentNamespace);
+
+ // When creating an entity under a namespace, the authz target is the
parentNamespace, but we
+ // must also add the actual path that will be created as an "optional"
passthrough resolution
+ // path to indicate that the underlying catalog is "allowed" to check the
creation path for
+ // a conflicting entity.
+ resolutionManifest.addPassthroughPath(
+ new ResolverPath(
+ Arrays.asList(namespace.levels()), PolarisEntityType.NAMESPACE,
true /* optional */),
+ namespace);
+ resolutionManifest.resolveAll();
+ PolarisResolvedPathWrapper target =
resolutionManifest.getResolvedPath(parentNamespace, true);
+ if (target == null) {
+ throw new NoSuchNamespaceException("Namespace does not exist: %s",
parentNamespace);
+ }
+ authorizer.authorizeOrThrow(
+ authenticatedPrincipal,
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ op,
+ target,
+ null /* secondary */);
+
+ initializeCatalog();
+ }
+
+ protected void authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
+ PolarisAuthorizableOperation op, TableIdentifier identifier) {
+ Namespace namespace = identifier.namespace();
+
+ resolutionManifest =
+ entityManager.prepareResolutionManifest(callContext, securityContext,
catalogName);
+ resolutionManifest.addPath(
+ new ResolverPath(Arrays.asList(namespace.levels()),
PolarisEntityType.NAMESPACE),
+ namespace);
+
+ // When creating an entity under a namespace, the authz target is the
namespace, but we must
+ // also
+ // add the actual path that will be created as an "optional" passthrough
resolution path to
+ // indicate that the underlying catalog is "allowed" to check the creation
path for a
+ // conflicting
+ // entity.
+ resolutionManifest.addPassthroughPath(
+ new ResolverPath(
+ PolarisCatalogHelpers.tableIdentifierToList(identifier),
+ PolarisEntityType.TABLE_LIKE,
+ true /* optional */),
+ identifier);
+ resolutionManifest.resolveAll();
+ PolarisResolvedPathWrapper target =
resolutionManifest.getResolvedPath(namespace, true);
+ if (target == null) {
+ throw new NoSuchNamespaceException("Namespace does not exist: %s",
namespace);
+ }
+ authorizer.authorizeOrThrow(
+ authenticatedPrincipal,
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ op,
+ target,
+ null /* secondary */);
+
+ initializeCatalog();
+ }
+
+ protected void authorizeBasicTableLikeOperationOrThrow(
+ PolarisAuthorizableOperation op, PolarisEntitySubType subType,
TableIdentifier identifier) {
+ resolutionManifest =
+ entityManager.prepareResolutionManifest(callContext, securityContext,
catalogName);
+
+ // The underlying Catalog is also allowed to fetch "fresh" versions of the
target entity.
+ resolutionManifest.addPassthroughPath(
+ new ResolverPath(
+ PolarisCatalogHelpers.tableIdentifierToList(identifier),
+ PolarisEntityType.TABLE_LIKE,
+ true /* optional */),
+ identifier);
+ resolutionManifest.resolveAll();
+ PolarisResolvedPathWrapper target =
+ resolutionManifest.getResolvedPath(identifier,
PolarisEntityType.TABLE_LIKE, subType, true);
+ if (target == null) {
+ if (subType == PolarisEntitySubType.ICEBERG_TABLE
+ || subType == PolarisEntitySubType.GENERIC_TABLE) {
+ throw new NoSuchTableException("Table does not exist: %s", identifier);
+ } else {
+ throw new NoSuchViewException("View does not exist: %s", identifier);
+ }
+ }
+ authorizer.authorizeOrThrow(
+ authenticatedPrincipal,
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ op,
+ target,
+ null /* secondary */);
+
+ initializeCatalog();
+ }
+
+ protected void authorizeCollectionOfTableLikeOperationOrThrow(
+ PolarisAuthorizableOperation op,
+ final PolarisEntitySubType subType,
+ List<TableIdentifier> ids) {
+ resolutionManifest =
+ entityManager.prepareResolutionManifest(callContext, securityContext,
catalogName);
+ ids.forEach(
+ identifier ->
+ resolutionManifest.addPassthroughPath(
+ new ResolverPath(
+ PolarisCatalogHelpers.tableIdentifierToList(identifier),
+ PolarisEntityType.TABLE_LIKE),
+ identifier));
+
+ ResolverStatus status = resolutionManifest.resolveAll();
+
+ // If one of the paths failed to resolve, throw exception based on the one
that
+ // we first failed to resolve.
+ if (status.getStatus() ==
ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED) {
+ TableIdentifier identifier =
+ PolarisCatalogHelpers.listToTableIdentifier(
+ status.getFailedToResolvePath().getEntityNames());
+ if (subType == PolarisEntitySubType.ICEBERG_TABLE) {
+ throw new NoSuchTableException("Table does not exist: %s", identifier);
+ } else {
+ throw new NoSuchViewException("View does not exist: %s", identifier);
+ }
+ }
+
+ List<PolarisResolvedPathWrapper> targets =
+ ids.stream()
+ .map(
+ identifier ->
+ Optional.ofNullable(
+ resolutionManifest.getResolvedPath(
+ identifier, PolarisEntityType.TABLE_LIKE,
subType, true))
+ .orElseThrow(
+ () ->
+ subType == PolarisEntitySubType.ICEBERG_TABLE
+ ? new NoSuchTableException(
+ "Table does not exist: %s", identifier)
+ : new NoSuchViewException(
+ "View does not exist: %s",
identifier)))
+ .toList();
+ authorizer.authorizeOrThrow(
+ authenticatedPrincipal,
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ op,
+ targets,
+ null /* secondaries */);
+
+ initializeCatalog();
+ }
+
+ protected void authorizeRenameTableLikeOperationOrThrow(
+ PolarisAuthorizableOperation op,
+ PolarisEntitySubType subType,
+ TableIdentifier src,
+ TableIdentifier dst) {
+ resolutionManifest =
+ entityManager.prepareResolutionManifest(callContext, securityContext,
catalogName);
+ // Add src, dstParent, and dst(optional)
+ resolutionManifest.addPath(
+ new ResolverPath(
+ PolarisCatalogHelpers.tableIdentifierToList(src),
PolarisEntityType.TABLE_LIKE),
+ src);
+ resolutionManifest.addPath(
+ new ResolverPath(Arrays.asList(dst.namespace().levels()),
PolarisEntityType.NAMESPACE),
+ dst.namespace());
+ resolutionManifest.addPath(
+ new ResolverPath(
+ PolarisCatalogHelpers.tableIdentifierToList(dst),
+ PolarisEntityType.TABLE_LIKE,
+ true /* optional */),
+ dst);
+ ResolverStatus status = resolutionManifest.resolveAll();
+ if (status.getStatus() ==
ResolverStatus.StatusEnum.PATH_COULD_NOT_BE_FULLY_RESOLVED
+ && status.getFailedToResolvePath().getLastEntityType() ==
PolarisEntityType.NAMESPACE) {
+ throw new NoSuchNamespaceException("Namespace does not exist: %s",
dst.namespace());
+ } else if (resolutionManifest.getResolvedPath(src,
PolarisEntityType.TABLE_LIKE, subType)
+ == null) {
+ if (subType == PolarisEntitySubType.ICEBERG_TABLE) {
+ throw new NoSuchTableException("Table does not exist: %s", src);
+ } else {
+ throw new NoSuchViewException("View does not exist: %s", src);
+ }
+ }
+
+ // Normally, since we added the dst as an optional path, we'd expect it to
only get resolved
+ // up to its parent namespace, and for there to be no TABLE_LIKE already
in the dst in which
+ // case the leafSubType will be NULL_SUBTYPE.
+ // If there is a conflicting TABLE or VIEW, this leafSubType will indicate
that conflicting
+ // type.
+ // TODO: Possibly modify the exception thrown depending on whether the
caller has privileges
+ // on the parent namespace.
+ PolarisEntitySubType dstLeafSubType =
resolutionManifest.getLeafSubType(dst);
+ if (dstLeafSubType == PolarisEntitySubType.ICEBERG_TABLE) {
+ throw new AlreadyExistsException("Cannot rename %s to %s. Table already
exists", src, dst);
+ } else if (dstLeafSubType == PolarisEntitySubType.ICEBERG_VIEW) {
Review Comment:
That part seems checking whether we can rename the ICEBEG TABLE to a target
name, and today ICEBERG_TABLE, ICEBERG_VIEW and GENERIC_TABLE are all under the
same namespace and require to have unique table.
The check here is trying to see whether the target name already exists, so
the name could also possibly exist with a generic table. If that is the case,
we can also give an more obvious error message. Actually I see yo add a check
below already
--
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-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
