Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-19 Thread Jean-Baptiste Onofré
Hi, I agree with Fokko's sum-up and call for action. Regards JB On Wed, Feb 19, 2025 at 11:34 AM Fokko Driesprong wrote: > > Thanks for the historical context here. I wasn't aware of the equivalent > between -1 and null. > > It looks like we're all aligned on not retroactively changing the spe

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-19 Thread Fokko Driesprong
Thanks for the historical context here. I wasn't aware of the equivalent between -1 and null. It looks like we're all aligned on not retroactively changing the spec. Let's wrap this up with some actions: - To revert the changes, I've raised the PRs (1.8.x

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Daniel Weeks
I agree that we shouldn't retroactively change the spec to align with the implementation. We're trying to strike a balance here and I think the notes are an effective way to convey that while omitting or producing null with v1/v2 is technically to spec, it's going to be incompatible with most impl

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Russell Spitzer
`For compatibility with existing libraries, we should maintain that `-1` is equivalent to no snapshot and it should be written for v1/v2.` The only issue I have with this is are we saying that for v1 and v2 we are changing the spec to say that current-snapshot-id is required? Or are we adding an i

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Daniel Weeks
I would agree the best path forward is to note the current behavior for v1/v2 since that's well established and address the behavior in v3. For compatibility with existing libraries, we should maintain that `-1` is equivalent to no snapshot and it should be written for v1/v2. With V3 we should su

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread rdb...@gmail.com
+1 to reverting PT 11560 in main and 1.8.1. That avoids unnecessary incompatibility with older readers. I also agree that we should update the spec to say what Russell suggests: > that -1 has meant "no current snapshot" in the past and is equivalent to missing/null. That's a correct description o

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Bryan Keller
I had a couple of small fixes that would be great to get into 1.8.1: https://github.com/apache/iceberg/pull/12305 https://github.com/apache/iceberg/pull/12224 I added those to the GitHub 1.8.1 milestone in case this is possible. Thanks, Bryan > On Feb 18, 2025, at 8:53 AM, Robert Stupp wrote: >

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Robert Stupp
On 18.02.25 17:10, Fokko Driesprong wrote: Reality is that Iceberg did write '-1' into current-snapshot-id (and other "non-exist" marker values for schema/spec/sort) instead of omitting the field. Yes, but this is wrong. The spec dictates

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Fokko Driesprong
> > Reality is that Iceberg did write '-1' into current-snapshot-id (and other > "non-exist" marker values for schema/spec/sort) instead of omitting the > field. Yes, but this is wrong. The spec dictates under current-snapshot-id: long ID

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Russell Spitzer
The only thing I think I agree with is defining that -1 has meant "no current snapshot" in the past and is equivalent to a missing/nuil (if we have to specify that) . I don't think there is any reason to change the behavior of writing null / missing unless that's really a point of confusion for fo

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Robert Stupp
Correcting myself: schema/spec/sort seem to be always present - please ignore that part in my previous email. The valid values for those fields however should be defined. On 18.02.25 14:29, Robert Stupp wrote: Reality is that Iceberg did write '-1' into current-snapshot-id (and other "non-ex

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-18 Thread Robert Stupp
Reality is that Iceberg did write '-1' into current-snapshot-id (and other "non-exist" marker values for schema/spec/sort) instead of omitting the field. Side note: the table-spec says that these fields are optional, but nothing about whether it is nullable. The spec should at least be amend

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-17 Thread Russell Spitzer
It sounds like the argument here is that we should change the Spec for V1, V2, and V3 to mark current-snapshot-id as required. Then we should change all other implementations to follow this new standard. I'm not sure that is a good solution going forwards but I'm not sure of how we can support cata

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-17 Thread Fokko Driesprong
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 . Therefore we support both in PyIceberg

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-17 Thread Robert Stupp
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 ba

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-17 Thread Fokko Driesprong
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

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-17 Thread Robert Stupp
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

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-13 Thread Daniel Weeks
I think we wanted to perform a check against the endpoint support like other view operations since they aren't expected to be implemented. PR here is what I believe we want. -Dan On Thu, Feb 13, 2025 at 7:37 AM Eduard Tudenhöfner wrote: > This is

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-13 Thread Eduard Tudenhöfner
This is most likely caused by the additional checks that were added in https://github.com/apache/iceberg/pull/11756. On Thu, Feb 13, 2025 at 4:16 PM Gabor Kaszab wrote: > Hi, > > I think it's because of this patch: > https://github.com/apache/iceberg/commit/a2b8008da7bc26e03248a3560d1cc7e849

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-13 Thread Gabor Kaszab
Hi, I think it's because of this patch: https://github.com/apache/iceberg/commit/a2b8008da7bc26e03248a3560d1cc7e8499d Before that replaceTransaction() didn't fail if the server didn't support views, with this change it does. Anyway, I see this was merged into 1.7 too. Didn't you see the same i

Re: [ANNOUNCE] Apache Iceberg release 1.8.0

2025-02-13 Thread Robert Stupp
Hi, There is a breaking change in 1.8.0 that prevents Iceberg 1.8 clients to talk to pre-1.8 Iceberg REST services. The errors manifest as something like: java.lang.UnsupportedOperationException: Server does not support endpoint: HEAD /v1/{prefix}/namespaces/{namespace}/views/{view} at