ArulJerald commented on PR #3991: URL: https://github.com/apache/iceberg-python/pull/3991#issuecomment-5723438389
@Fokko to clarify — I'm not claiming a manifest mutates in place; that would indeed violate the spec, and nothing here contradicts it. The scenario is a process-wide cache with no table/catalog scoping: two unrelated ManifestFile objects (e.g. from different tables, or even two unrelated test fixtures that happen to reuse the same placeholder path) hitting the same cache key. Nothing was mutated — the cache just has no way to tell "same manifest, reused correctly" apart from "different manifest, coincidentally same path string," because the key is only ever a bare path. @rambleraptor your point on the cost is fair, and thinking about it more, scoping the cache key by catalog, table (or table UUID/location) is a better fix than the content-comparison approach in this PR — it prevents the collision structurally instead of detecting it after the fact, and it doesn't add cost to the common hit path. I'll rework the PR to key on (table_identifier, manifest_path) instead and drop the equality check unless there's a reason to keep it as a cheap defensive assertion. Let me know if that direction sounds right before I push the change. -- 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]
