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

Reply via email to