zeroshade commented on code in PR #1737:
URL: https://github.com/apache/iceberg-go/pull/1737#discussion_r3833276195
##########
table/transaction.go:
##########
@@ -3266,6 +3269,14 @@ func (t *Transaction) Commit(ctx context.Context)
(*Table, error) {
return t.tbl, nil
}
+func validateGCEnabledForSnapshotExpiration(props iceberg.Properties) error {
+ if !props.GetBool(GCEnabledKey, GCEnabledDefault) {
Review Comment:
`GetBool(..., true)` fails open here: Go's parser accepts values such as
`"1"` as true, while parse errors such as `"garbage"` or `"false "` return the
true default. That means malformed/non-Java values can still enable snapshot
expiration and physical file deletion. Iceberg Java only enables this property
for case-insensitive `"true"`. Please make this safety gate fail closed or
exactly match Java parsing, use the same helper for post-commit cleanup and
`PurgeFiles`, and add tests for these values.
--
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]
