lliangyu-lin commented on code in PR #395:
URL: https://github.com/apache/iceberg-go/pull/395#discussion_r2059393092


##########
catalog/glue/glue.go:
##########
@@ -330,6 +331,53 @@ func (c *Catalog) RegisterTable(ctx context.Context, 
identifier table.Identifier
        return c.LoadTable(ctx, identifier, nil)
 }
 
+// SetTableProperties updates the properties of an existing Iceberg table.
+// The properties will be merged with existing properties. If a property key 
already exists,
+// its value will be overwritten.
+func (c *Catalog) SetTableProperties(ctx context.Context, identifier 
table.Identifier, props iceberg.Properties) error {
+       database, tableName, err := identifierToGlueTable(identifier)
+       if err != nil {
+               return err
+       }
+
+       existingTable, err := c.getTable(ctx, database, tableName)
+       if err != nil {
+               return err
+       }
+
+       updatedParameters := map[string]string{}
+       for k, v := range existingTable.Parameters {
+               updatedParameters[k] = v
+       }
+       for k, v := range props {
+               updatedParameters[k] = v
+       }

Review Comment:
   I did some checks with your change locally. I think only updating the table 
properties on Glue catalog might not be enough. When I try loading the updated 
table, the properties in the table metadata is not updated. This is because 
glue `LoadTable` is [reading the metadata 
file](https://github.com/apache/iceberg-go/blob/main/catalog/glue/glue.go#L219-L222),
 but Glue update table API call doesn't create a new metadata file.
   
   I raised this PR https://github.com/apache/iceberg-go/pull/403 on using the 
CommitTable which directly hooked into the CLI and can directly use the set and 
unset properties. 
https://github.com/apache/iceberg-go/blob/main/table/updates.go#L36-L45. Feel 
free to take a look.



-- 
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