sririshindra commented on code in PR #3719:
URL: https://github.com/apache/polaris/pull/3719#discussion_r2891135539
##########
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:
agreed. I documented this in two places: (1) code-level note + explicit
BadRequest message, and (2) unreleased federation docs under operational notes.
Current behavior is explicit: overwrite register is supported only for internal
Polaris catalogs.
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -287,25 +287,75 @@ protected Map<String, String> properties() {
@Override
public Table registerTable(TableIdentifier identifier, String
metadataFileLocation) {
+ return registerTable(identifier, metadataFileLocation, false);
+ }
+
+ /**
+ * Register a table with optional overwrite semantics.
+ *
+ * <p>When {@code overwrite} is false (the default) this behaves like a
normal register and will
+ * fail if the table already exists. When {@code overwrite} is true and the
named table already
+ * exists, this method updates the table's stored metadata-location to point
at the provided
+ * metadata file. The overwrite path performs additional validation to
ensure the supplied
+ * metadata file and its location are consistent with the table's resolved
storage configuration.
+ *
+ * @param identifier the table identifier
+ * @param metadataFileLocation the metadata file location
+ * @param overwrite if true, update existing table metadata; if false, throw
exception if table
+ * exists
+ * @return the registered table
+ */
+ public Table registerTable(
Review Comment:
I agree, I will raise a separate PR in the Iceberg repo for this. As the
support for overwrite is added in the rest interface
https://github.com/apache/iceberg/pull/15248/changes it is probably a good idea
to add the same in the core Iceberg as well.
--
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]
