The origins of 'gc.enabled' go back quite far and fall more into the category of convention rather than specification. The surrounding ecosystem has changed significantly and while I think clients should generally abide by the convention, I don't think codifying this is the right approach. There are also FileIO flags that disable deletes, but similarly, we wouldn't want to codify those behaviors.
I'd rather rely on catalog implementations to enforce requirements when necessary; not on clients to do the right thing. -Dan On Fri, Sep 11, 2026 at 8:33 AM Russell Spitzer <[email protected]> wrote: > > We added this when we started snapshotting existing data: an Iceberg table > that only represents files owned by another system. The metadata would be > all new and owned by the Iceberg Table but the data files would still be > owned by another system. We did not want Iceberg maintenance to touch those > files. I have no problem writing that down. I don't think it matters much > how an implementation enforces it, as long as it does not delete data > files. We could put this in the spec appendix; when we added the property > we simply were not putting table properties in the spec. > > I don't really see this as ambiguous in the Java Repo. The operations that > fail (or skip work) under gc.enabled=false are the ones *that might > delete a data file*. How that is enforced differs but that doesn't really > bother me. It is true that in some cases you could run the same API without > any data-file risk if we checked the cleanup mode instead of failing in the > constructor but we just haven't done that yet. > > So following for q4, the behavior is fine > And Q5 - It would be too late, if a client already removed the files, it > doesn't matter if the catalog rejects the commit removing those snapshots. > If the Catalog is removing data files (through a purge or it's own > maintenance) then it should respect the GC flag. > > TL;DR - The invariant here is "*Don't remove this table's data files"* and > I don't think anything needs to change about that. > > On Fri, Sep 11, 2026 at 5:20 AM Alexander Bailey <[email protected]> > wrote: > >> Hi all, >> >> I have a PR open that relaxes `gc.enabled` for metadata-only snapshot >> expiry, and review feedback has convinced me the underlying problem is >> that we've never defined what `gc.enabled` means in full. I'd like to try >> and settle the >> definition on-list before the behaviour change goes anywhere. >> >> https://github.com/apache/iceberg/pull/17791 >> >> The property is not in the spec. The only normative-ish text is one line >> in docs/docs/configuration.md: >> >> > gc.enabled | true | Allows garbage collection operations such as >> > expiring snapshots and removing orphan files >> >> That wording arguably supports the strictest reading ("expiry is a GC >> operation, so it is blocked"), which is why I don't think of this as a >> bug fix -- it's a semantic decision the community should make. >> >> Java implements at least three different meanings today: >> >> 1. `CatalogUtil.dropTableData` proceeds when `gc.enabled=false`: it >> skips data files but still deletes manifests, manifest lists, >> statistics, previous metadata and the metadata file. No error. That >> is exactly the `METADATA_ONLY` semantic we just added to >> `ExpireSnapshots.CleanupLevel`. >> >> 2. `RemoveSnapshots` (before this PR) hard-failed in its constructor, >> even for an expiry configured to delete nothing at all. >> >> 3. `SparkCatalog.purgeTable`, `ExpireSnapshotsSparkAction`, >> `DeleteOrphanFilesSparkAction` and `DeleteReachableFilesSparkAction` >> all hard-fail on construction. >> >> 4. `HMSTablePropertyHelper` maps `gc.enabled` to Hive's >> `external.table.purge`, i.e. treats it as a drop-purge flag. Its own >> javadoc already concedes the two flags can disagree and produce >> surprising behaviour. >> >> So within one repository the same flag means "skip data files, delete >> metadata", "refuse the whole operation", and "control DROP TABLE purge". >> >> The clients have inherited the ambiguity, not resolved it: >> >> - iceberg-rust rejects expiry at commit time in >> crates/iceberg/src/transaction/expire_snapshots.rs, with the comment >> "Expiring metadata defeats a user's explicit decision to disable GC >> (Java refuses too)" -- citing Java as the authority. Meanwhile >> table_properties.rs documents the same key as "enabling garbage >> collection on drop... data files will not be deleted when a table is >> dropped". Two definitions in one crate. >> >> - iceberg-go documents it as "Snapshot expiration is rejected when >> false, and data-file cleanup skips deletion." >> >> - iceberg-cpp rejects expiry in the `ExpireSnapshots` constructor, with >> the identical error string to Java's pre-PR check ("Cannot expire >> snapshots: GC is disabled (deleting files may corrupt other tables)") >> in the same construction-time position. It is a direct port of the >> behaviour this PR removes. >> src/iceberg/update/expire_snapshots.cc >> >> - iceberg-python has a full `expire_snapshots` implementation and no >> `gc.enabled` check that I can find under any spelling, so it already >> permits what Rust, Go and C++ reject. >> >> So two clients have hard-coded Java's current behaviour -- one citing >> Java in a comment, the other copying its error message verbatim -- while >> a third ignores the property for this operation entirely. Whichever >> definition we pick, the worst outcome for those implementations is Java >> changing quietly, which is why I'd rather settle it here than in a Java >> PR. >> >> Proposal. Define `gc.enabled=false` as: >> >> > Physical file deletion is disabled for this table. No client may >> > delete a file from the table's storage, whether or not the table's >> > metadata still references it. Operations that only rewrite table >> > metadata are unaffected. >> >> Consequences: >> >> - Snapshot expiry that deletes no files is permitted; expiry that would >> delete files is rejected. >> >> - `remove_orphan_files` and drop-with-purge stay blocked, as today. >> >> - `dropTableData` currently proceeds under `gc.enabled=false` and >> deletes manifests, manifest lists, statistics and metadata files, >> skipping only data files. Under this definition that becomes >> non-conformant. As far as I can tell nothing in-tree depends on it -- >> Nessie ignores `purge` entirely -- but it is a behaviour change well >> outside the scope of the PR above and would need to be its own >> discussion. >> >> - The rule is stated once, in the spec's table-property appendix, so >> clients implement one thing. >> >> The alternative, raised by Amogh in review and worth taking seriously: >> keep `gc.enabled` as a blunt "no snapshot removal at all" switch, on the >> grounds that catalogs and platform operators may already rely on it that >> way, and introduce a separate, finer-grained property for cleanup >> control. I lean against adding a property when `CleanupLevel` already >> expresses the granularity, but I don't think the concern about existing >> opinionated deployments is wrong. >> >> Questions to answer: >> >> 1. Which definition do we want -- "no physical deletion" or "no snapshot >> removal"? >> >> 2. Should this land in the spec (table-property appendix) or stay >> documentation-only? >> >> 3. Error or silent skip when a cleanup operation is blocked? Java >> currently does both. >> >> 4. Should the strict reading extend to metadata files on drop, making >> `dropTableData` skip manifests and metadata too, or do we grandfather >> its current behaviour? >> >> 5. Does anything need saying about REST catalogs, which can reject the >> remove-snapshot update server-side regardless of what clients do? >> >> Happy to write the spec PR and open tracking issues in the Python, Rust, >> Go and C++ repos once there's a direction. >> >> Thanks, >> Xander >> >
