raunaqmorarka commented on PR #18074: URL: https://github.com/apache/iceberg/pull/18074#issuecomment-5666637842
Good catch on the wrapper, and I agree the cached objects were a trap. `ManifestListWriter.prepare()` substitutes values as it writes: the sequence number and min sequence number when they are still unassigned, and first-row-id for data manifests in v3 and v4. So a cached `ManifestFile` differs from the record the parser reads back, even though it compares equal. Taken the suggestion. `cleanUncommitted(Set<ManifestFile>)` and `deleteUncommitted` now take a set of manifest paths, and only paths are cached. Every implementation was already doing a contains check and then reading `path()`, and the javadoc on the abstract method already described the parameter as manifest paths, so the signature now matches the docs. The transaction path builds a set of paths anyway, so it loses a mapping step. Rebased and force-pushed. Core append, merge, transaction, rewrite, cleanup and expiry suites pass. -- 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]
