Hey Robert, The thing is, that -1 cannot "go away".
Yes, I agree, but that's also the case for null, as the field is optional in the spec <https://iceberg.apache.org/spec/?column-projection#table-metadata-fields>. Therefore we support both in PyIceberg <https://github.com/apache/iceberg-python/blob/300b8405a0fe7d0111321e5644d704026af9266b/pyiceberg/table/metadata.py#L71-L77>, Iceberg-Rust <https://github.com/apache/iceberg-rust/blob/752d69041e0461989c48dd1ca79bcff577776f5d/crates/iceberg/src/spec/table_metadata.rs#L500>, Iceberg-Java <https://github.com/apache/iceberg/blob/bcbbd0344623ffea5b092e2de5debb0bc12892a1/core/src/main/java/org/apache/iceberg/TableMetadataParser.java#L458-L462>, and Iceberg-Go <https://github.com/apache/iceberg-go/blob/ada5480954d9b41d2f8eb4c765523614fad65e1a/table/metadata.go#L837-L841>. On the write side, they all produce null instead of -1. Therefore, I was surprised that it comes up now, and not earlier. I'd prefer to keep the previous behavior - otherwise implementations may > fall back to 0, which is definitely wrong. I'm not seeing why it would fall back to 0, and I agree, that's wrong. Would be better IMHO not to break existing implementations / render > existing setups incompatible with Iceberg 1.8. In my opinion, if this had been caught in an RC, it would be open for discussion, but that ship has sailed. Let's hear what others think. Kind regards, Fokko Op ma 17 feb 2025 om 18:16 schreef Robert Stupp <sn...@snazy.de>: > Hi Fokko, > > sure, in general "absent" or "null" would be cleaner. But now we have two > representations for the same case - I suspect most went with the "reference > behavior". > > The thing is, that -1 cannot "go away". > > I'd prefer to keep the previous behavior - otherwise implementations may > fall back to 0, which is definitely wrong. Would be better IMHO not to > break existing implementations / render existing setups incompatible with > Iceberg 1.8. > > > On 17.02.25 15:49, Fokko Driesprong wrote: > > Hey Robert, > > Thanks for raising this. > > snapshot-ID -1 isn't per-se invalid, because the valid values are not >> defined in the spec. > > > For me, this is invalid, since there is no snapshot with -1 in the > snapshots property. In the tests with the PR, you can see that there are > no snapshots > <https://github.com/apache/iceberg/pull/11560/files#diff-41bdfb6698d2aa7b47ff7d5fabc558a5a64f8b7496fe1bcd8f8ecb69b2afc128R112>. > A year ago we had a similar discussion on PyIceberg > <https://py.iceberg.apache.org/configuration/#backward-compatibility> > around this and this ended up in adding a flag to fall back to the old > behavior. I do agree that we should have communicated this more clearly > with the release. > > Kind regards, > Fokko > > > > > Op ma 17 feb 2025 om 12:25 schreef Robert Stupp <sn...@snazy.de>: > >> Feels like https://github.com/apache/iceberg/pull/11560 introduced a >> behavior change. >> >> snapshot-ID -1 isn't per-se invalid, because the valid values are not >> defined in the spec. >> >> Previous Iceberg-Java versions always produced -1 if there's no current >> snapshot - 1.8 produces `null` in that case. So there are now two >> _different_ values for "no current snapshot". >> >> Implementations that rely on the behavior of the "reference >> implementation" (Iceberg-Java) do now fail in case there's no current >> snapshot. >> On 13.02.25 10:09, Amogh Jahagirdar wrote: >> >> I'm pleased to announce the release of Apache Iceberg 1.8.0! >> >> Apache Iceberg is an open table format for huge analytic datasets. Iceberg >> delivers high query performance for tables with tens of petabytes of data, >> along with atomic commits, concurrent writes, and SQL-compatible table >> evolution. >> >> This release can be downloaded from: >> https://www.apache.org/dyn/closer.cgi/iceberg/apache-iceberg-1.8.0/apache-iceberg-1.8.0.tar.gz >> >> >> Release notes: https://iceberg.apache.org/releases/#180-release >> <https://iceberg.apache.org/releases/180-release> >> >> Java artifacts are available from Maven Central. >> >> Thanks to everyone for contributing! >> >> -- >> Robert Stupp >> @snazy >> >> -- > Robert Stupp > @snazy > >
