flyingImer commented on code in PR #3719:
URL: https://github.com/apache/polaris/pull/3719#discussion_r2892101339
##########
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(
+ TableIdentifier identifier, String metadataFileLocation, boolean
overwrite) {
+ LOGGER.debug(
+ "registerTable called with identifier={}, metadataFileLocation={},
overwrite={}",
+ identifier,
+ metadataFileLocation,
+ overwrite);
Preconditions.checkArgument(
identifier != null && isValidIdentifier(identifier), "Invalid
identifier: %s", identifier);
Preconditions.checkArgument(
metadataFileLocation != null && !metadataFileLocation.isEmpty(),
"Cannot register an empty metadata file location as a table");
int lastSlashIndex = metadataFileLocation.lastIndexOf("/");
+ LOGGER.debug(
+ "Computed lastSlashIndex={} for metadataFileLocation={}",
+ lastSlashIndex,
+ metadataFileLocation);
Preconditions.checkArgument(
lastSlashIndex != -1,
"Invalid metadata file location; metadata file location must be
absolute and contain a '/': %s",
metadataFileLocation);
// Throw an exception if this table already exists in the catalog.
- if (tableExists(identifier)) {
+ boolean tableExists = tableExists(identifier);
+ LOGGER.debug("tableExists for identifier {} = {}", identifier,
tableExists);
+ if (!overwrite && tableExists) {
+ LOGGER.debug("Table already exists and overwrite is false for
identifier={}", identifier);
throw new AlreadyExistsException("Table already exists: %s", identifier);
}
String locationDir = metadataFileLocation.substring(0, lastSlashIndex);
+ LOGGER.debug(
+ "Derived locationDir={} for metadataFileLocation={}", locationDir,
metadataFileLocation);
+ if (!tableExists) {
+ LOGGER.debug(
+ "Registering new table for identifier={}, metadataFileLocation={}",
+ identifier,
+ metadataFileLocation);
+ return registerNewTable(identifier, metadataFileLocation, locationDir);
+ }
+
+ LOGGER.debug(
+ "Overwriting registered table for identifier={},
metadataFileLocation={}",
+ identifier,
+ metadataFileLocation);
+ return overwriteRegisteredTable(identifier, metadataFileLocation,
locationDir);
Review Comment:
IIUC, this path comes from overwrite=true and tableExists=true, in such
case, would additional authz be required? i.e., privileges on the existing
table entity, not just the namespace.
--
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]
