stevenzwu commented on code in PR #12228:
URL: https://github.com/apache/iceberg/pull/12228#discussion_r2019505267
##########
core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java:
##########
@@ -71,23 +70,35 @@ public Table loadTable(TableIdentifier identifier) {
}
@Override
- public Table registerTable(TableIdentifier identifier, String
metadataFileLocation) {
+ public Table registerTable(
+ TableIdentifier identifier, String metadataFileLocation, boolean
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");
- // Throw an exception if this table already exists in the catalog.
- if (tableExists(identifier)) {
+ // If the table already exists and overwriting is disabled, throw an
exception.
+ if (tableExists(identifier) && !overwrite) {
throw new AlreadyExistsException("Table already exists: %s", identifier);
}
TableOperations ops = newTableOps(identifier);
- InputFile metadataFile = ops.io().newInputFile(metadataFileLocation);
- TableMetadata metadata = TableMetadataParser.read(ops.io(), metadataFile);
- ops.commit(null, metadata);
-
+ TableMetadata newMetadata =
+ TableMetadataParser.read(ops.io(),
ops.io().newInputFile(metadataFileLocation));
+
+ TableMetadata existing = ops.current();
+ if (existing != null && overwrite) {
+ if (existing.metadataFileLocation().equals(metadataFileLocation)) {
+ LOG.info(
+ "The requested metadata matches the existing metadata. No changes
will be committed.");
+ return new BaseTable(ops, fullTableName(name(), identifier),
metricsReporter());
+ }
+ dropTable(identifier, false /* Keep all data and metadata files */);
Review Comment:
Steve changed the implementation to delete then register. It would work
around the commit API limitation (write a new metadata json file). But this
choice has the potential problem of leaving the table in a bad intermediate
state (deleted but no re-registered). Is this sth we are willing to accept?
If we really want to implement the overwrite properly, we would need to
potentially change/expand the `TableOperation` interface to allow the atomic
swap.
--
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...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
