tanmayrauth commented on code in PR #1406:
URL: https://github.com/apache/iceberg-go/pull/1406#discussion_r3525796760
##########
table/updates.go:
##########
@@ -772,6 +772,10 @@ func NewAddEncryptionKeyUpdate(key EncryptionKey)
*addEncryptionKeyUpdate {
}
func (u *addEncryptionKeyUpdate) Apply(builder *MetadataBuilder) error {
+ if err := u.EncryptionKey.Validate(); err != nil {
Review Comment:
This Validate() looks redundant — Apply just delegates to
builder.AddEncryptionKey on the next line, and that already calls
key.Validate() (metadata.go:1341), so an invalid key hits the same
ErrInvalidArgument either way. Your
TestAddEncryptionKeyUpdate_UnmarshalMissingFields_ApplyRejects still passes
through AddEncryptionKey without this check. Any reason to keep both? Either
drop this one or add a short comment on why the update path validates
independently, so the two don't drift.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
