+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 >>> > >>> > >>> >>
