nandorKollar commented on code in PR #3719:
URL: https://github.com/apache/polaris/pull/3719#discussion_r2879221911
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -612,13 +615,133 @@ public LoadTableResponse createTableStaged(
* @return ETagged {@link LoadTableResponse} to uniquely identify the table
metadata
*/
public LoadTableResponse registerTable(Namespace namespace,
RegisterTableRequest request) {
- PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.REGISTER_TABLE;
- authorizeCreateTableLikeUnderNamespaceOperationOrThrow(
- op, TableIdentifier.of(namespace, request.name()));
+ TableIdentifier identifier = TableIdentifier.of(namespace, request.name());
+ boolean overwrite = request.overwrite();
+
+ LOGGER.debug(
+ "registerTable: identifier={}, overwrite={}, request={}", identifier,
overwrite, request);
+ if (overwrite) {
+ LOGGER.debug("registerTable: overwrite requested for {}", identifier);
+ authorizeRegisterTableOverwriteOrCreate(identifier);
+ return registerTableWithOverwrite(identifier, request);
+ }
+
+ // Creating new table requires REGISTER_TABLE privilege
+ PolarisAuthorizableOperation op =
PolarisAuthorizableOperation.REGISTER_TABLE;
+ authorizeCreateTableLikeUnderNamespaceOperationOrThrow(op, identifier);
+ LOGGER.debug("registerTable: authorized REGISTER_TABLE for {}",
identifier);
return catalogHandlerUtils().registerTable(baseCatalog, namespace,
request);
}
+ /**
+ * Authorize registerTable with overwrite=true.
+ *
+ * <p>If the table exists, require REGISTER_TABLE_OVERWRITE on the table;
otherwise require
+ * REGISTER_TABLE on the parent namespace.
+ *
+ * <p>Resolve both the namespace and an optional passthrough table path in
one pass because the
+ * standard helpers either assume the table exists or always authorize
against the namespace.
+ * Also, baseCatalog.tableExists() cannot be used here since
initializeCatalog() has not run.
+ */
+ private void authorizeRegisterTableOverwriteOrCreate(TableIdentifier
identifier) {
+ LOGGER.debug("authorizeRegisterTableOverwriteOrCreate: start for {}",
identifier);
+ // Build a resolution manifest that includes the namespace and optional
table path.
+ resolutionManifest = newResolutionManifest();
+ resolutionManifest.addPath(
+ new ResolverPath(
+ Arrays.asList(identifier.namespace().levels()),
PolarisEntityType.NAMESPACE),
+ identifier.namespace());
+ resolutionManifest.addPassthroughPath(
+ new ResolverPath(
+ PolarisCatalogHelpers.tableIdentifierToList(identifier),
+ PolarisEntityType.TABLE_LIKE,
+ true /* optional */),
+ identifier);
+ resolutionManifest.resolveAll();
+ PolarisResolvedPathWrapper tableTarget =
+ resolutionManifest.getResolvedPath(
+ identifier, PolarisEntityType.TABLE_LIKE,
PolarisEntitySubType.ICEBERG_TABLE, true);
+
+ if (tableTarget != null) {
+ LOGGER.debug(
+ "authorizeRegisterTableOverwriteOrCreate: found existing table
target for {}, requiring REGISTER_TABLE_OVERWRITE",
+ identifier);
+ // Overwrite on an existing table requires full metadata permissions.
+ authorizer()
+ .authorizeOrThrow(
+ polarisPrincipal(),
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ PolarisAuthorizableOperation.REGISTER_TABLE_OVERWRITE,
+ tableTarget,
+ null /* secondary */);
+ initializeCatalog();
+ LOGGER.debug(
+ "registerTable: overwrite=true, authorized for
REGISTER_TABLE_OVERWRITE on existing table {}",
+ identifier);
+ return;
+ }
+
+ // Table doesn't exist, fall back to standard register-table authorization.
+ LOGGER.debug(
+ "authorizeRegisterTableOverwriteOrCreate: table not found for {},
falling back to REGISTER_TABLE on namespace",
+ identifier);
+ PolarisResolvedPathWrapper namespaceTarget =
+ resolutionManifest.getResolvedPath(identifier.namespace(), true);
+ if (namespaceTarget == null) {
+ LOGGER.debug(
+ "authorizeRegisterTableOverwriteOrCreate: namespace not found for
{}",
+ identifier.namespace());
+ throw new NoSuchNamespaceException("Namespace does not exist: %s",
identifier.namespace());
+ }
+ authorizer()
+ .authorizeOrThrow(
+ polarisPrincipal(),
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ PolarisAuthorizableOperation.REGISTER_TABLE,
+ namespaceTarget,
+ null /* secondary */);
+ initializeCatalog();
+ LOGGER.debug(
+ "authorizeRegisterTableOverwriteOrCreate: authorized REGISTER_TABLE on
namespace {}",
+ identifier.namespace());
+ }
+
+ private LoadTableResponse registerTableWithOverwrite(
+ TableIdentifier identifier, RegisterTableRequest request) {
+ LOGGER.debug(
+ "registerTableWithOverwrite: identifier={}, metadataLocation={}",
+ identifier,
+ request.metadataLocation());
+ // Handle Polaris-specific overwrite logic
+ if (baseCatalog instanceof IcebergCatalog icebergCatalog) {
Review Comment:
I think this won't work with federated catalog, where `baseCatalog` is a
`RESTCatalog`. We should probably document this limitation.
--
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]
