laskoviymishka commented on PR #17791: URL: https://github.com/apache/iceberg/pull/17791#issuecomment-6077453258
@xanderbailey the code side is in good shape from my end. What's blocking is the semantic question @amogh-jahagirdar raised, and I don't think we can settle that in PR review. gc.enabled isn't in the spec, but it's become a de-facto cross-client contract: Java, Spark, py, rust, go and cpp all treat false as "no expiry at all". Some catalogs and users may also rely on it to block snapshot removal entirely. I think the way forward is a [DISCUSS] thread on dev@ that settles: 1. What gc.enabled=false means: "never delete files" (what this PR assumes) or "no snapshot removal at all" (what every client enforces today). 2. Whether a boolean is enough, or whether we'd rather have a finer-grained property/level, as @amogh-jahagirdar suggested, instead of overloading gc.enabled. 3. How Spark/Flink get the same behaviour (#17929), so core and the engine actions don't disagree. 4. Whether the other clients should follow, and whether the meaning should be written into the spec or docs. Once there's rough consensus, this PR plus #17929 should be easy to land. If need a help with this thread - ping me, i'll help you there. -- 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]
