[GitHub] [iceberg] kbendick commented on a change in pull request #3670: HiveTableOperations: update HMS transient_lastDdlTime param on snapshot writes

[GitBox <gi...@apache.org>]

kbendick commented on a change in pull request #3670:
URL: https://github.com/apache/iceberg/pull/3670#discussion_r762610090



##########
File path: hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
##########
@@ -362,6 +362,9 @@ private void setHmsTableParameters(String newMetadataLocation, Table tbl, Map<St
     // remove any props from HMS that are no longer present in Iceberg table props
     obsoleteProps.forEach(parameters::remove);
 
+    // remove the DDL_TIME so it gets refreshed

Review comment:
       Nit: you might consider formatting this comment more generically like the one above. Something like `// remove any props that are controlled by HMS so it can handle them`. That way it's also clear where to add more properties like that in the future.
   
   Up to you though. There's nothing wrong per-se with the current comment. Just this has come up a few times on Slack and I'm not sure if there's other properties that might fall into this category.
   




-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org