danielcweeks commented on a change in pull request #1156:
URL: https://github.com/apache/iceberg/pull/1156#discussion_r451838028
##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
##########
@@ -110,9 +110,17 @@ public Transaction newReplaceTableTransaction(
throw new NoSuchTableException("No such table: " + identifier);
}
- String baseLocation = location != null ? location :
defaultWarehouseLocation(identifier);
Map<String, String> tableProperties = properties != null ? properties :
Maps.newHashMap();
- TableMetadata metadata = TableMetadata.newTableMetadata(schema, spec,
baseLocation, tableProperties);
+
+ TableMetadata metadata;
+ if (ops.current() != null) {
Review comment:
I'm not clear on whether this really should be the right behavior.
Basically we're saying that a replace table will keep the existing location (as
opposed to using defaults). I suspect we don't have create or replace with
location semantics, but this is making some assumptions that a replacement is
somehow the same as the old. If we were to go with id based pathing
convention, this wouldn't work.
I don't think this is an issue at this point, but it might make sense to
push this down to the location provider.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
