Hey Christian, Do you want to follow up with a PR? I was just reviewing a PR on PyIceberg <https://github.com/apache/iceberg-python/pull/1285> and came to the same conclusion.
Kind regards, Fokko Op di 17 dec 2024 om 23:36 schreef Marc Cenac <marc.ce...@datadoghq.com.invalid>: > +1 to removing this redundancy in the REST spec and Java implementation > > On Tue, Dec 17, 2024 at 12:10 PM Kevin Liu <kevinjq...@apache.org> wrote: > >> Hey Christian, >> >> Thanks for bringing this up! We also noticed this issue while >> implementing table statistics in Python [1]. >> I'm in favor of removing the outer field. Since this is part of the spec >> change, we would need to follow the proper deprecation and removal path, >> similar to what we did for `last-column-id` in #11514 >> <https://github.com/apache/iceberg/pull/11514>. >> >> Best, >> Kevin Liu >> >> [1] https://github.com/apache/iceberg-python/pull/1285 >> >> On Mon, Dec 16, 2024 at 5:30 AM Fokko Driesprong <fo...@apache.org> >> wrote: >> >>> 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 >>>> > >>>> > >>>> >>>
