Hey Christian, Great catch, I would also be in favor of removing the outer one. I don't see any value in having them both.
Kind regards, Fokko Op ma 16 dec 2024 om 14:26 schreef Jean-Baptiste Onofré <j...@nanthrax.net>: > Hi, > > I saw the discussion on Slack. Yeah, it's redundant. > I know some catalogs only consider the snapshot id in SetStatisticsUpdate. > > Regards > JB > > On Fri, Dec 13, 2024 at 8:03 PM Christian <christian.t.b...@gmail.com> > wrote: > > > > Dear all, > > > > I believe we currently have a redundancy in the IRC SetStatisticsUpdate > [1]. > > SetStatisticsUpdate has a required field `snapshot-id` but also a > `StatisticsFile` which in turn contains the `snapshot-id` as required. The > redundant information is used in Java only for an assertion to check if the > ids are identical [2]. > > > > Are there any good reasons to keep both `snapshot-id`s? If not I would > propose to deprecate the outer `snapshot-id`. > > To remove redundancy in libraries, I am using a custom serializer / > deserializer to handle this in Rust [3]. > > > > Let me know what you think! > > > > Thanks, > > Christian > > > > [1]: > https://github.com/apache/iceberg/blob/540d6a6251e31b232fe6ed2413680621454d107a/open-api/rest-catalog-open-api.yaml#L2902 > > [2]: > https://github.com/apache/iceberg/blob/540d6a6251e31b232fe6ed2413680621454d107a/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1314 > > [3]: https://github.com/apache/iceberg-rust/pull/799 > > > > >
