rdblue commented on code in PR #10253: URL: https://github.com/apache/iceberg/pull/10253#discussion_r1876425086
########## core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java: ########## @@ -1341,6 +1343,8 @@ public View create() { Preconditions.checkState( null != defaultNamespace, "Cannot create view without specifying a default namespace"); + this.schema = TypeUtil.assignFreshIds(schema, new AtomicInteger(0)::incrementAndGet); Review Comment: It looks like this is being done in the wrong place because we need to assign fresh IDs in both the REST catalog here and in `BaseMetastoreViewCatalog`. We don't want to create a situation where implementations need to know to assign fresh IDs. For tables, the initial assignment is done in `TableMetadata.newTableMetadata` and the reassignment for replace happens in `TableMetadata.buildReplacement`. Views should have a similar approach, where creating a replacement `ViewMetadata` automatically handles the reassignment so callers don't need to know and do it manually. -- 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