You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/07/10 21:54:36 UTC

[PR] fix: validate retention durations [iceberg-go]

fallintoplace opened a new pull request, #1443:
URL: https://github.com/apache/iceberg-go/pull/1443

   Reject invalid retention and cleanup inputs before any scan or metadata mutation.
   
   This covers negative ages, invalid retain-last values, non-finite day durations, and time.Duration overflow boundaries. Zero age remains supported where the library contract allows it.
   
   Tests: go test ./table ./cmd/iceberg; go test -short ./... (io/gocloud hangs in existing tests).


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


Re: [PR] fix: validate retention durations [iceberg-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #1443:
URL: https://github.com/apache/iceberg-go/pull/1443#discussion_r3566743399


##########
table/orphan_cleanup.go:
##########
@@ -95,6 +96,9 @@ func WithLocation(location string) OrphanCleanupOption {
 func WithFilesOlderThan(duration time.Duration) OrphanCleanupOption {
 	return func(cfg *orphanCleanupConfig) {
 		cfg.olderThan = duration
+		if duration < 0 && cfg.validationErr == nil {

Review Comment:
   This rejection changes existing behavior: the integration suite calls `WithFilesOlderThan(-1*time.Hour)` in several places (e.g. `table/orphan_cleanup_integration_test.go:226,273,314,363,403`) to intentionally consider *all* files (including future timestamps), so those tests will now fail at option-validation before scanning. A negative-duration unit test was added, but the integration fixtures using a negative age weren't updated. Please either update those call sites to a non-negative age (and set file mtimes accordingly), or keep an explicit "consider all files" affordance.



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


Re: [PR] fix: validate retention durations [iceberg-go]

Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #1443:
URL: https://github.com/apache/iceberg-go/pull/1443


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