szehon-ho commented on code in PR #12334: URL: https://github.com/apache/iceberg/pull/12334#discussion_r1962283684
########## format/spec.md: ########## @@ -1754,6 +1754,10 @@ Snapshot summary can include metrics fields to track numeric stats of the snapsh | **`engine-name`** | "spark" | Name of the engine that created the snapshot | | **`engine-version`** | "3.5.4" | Version of the engine that created the snapshot | +### Encoding of `current-snapshot-id` + +The Java implementation writes `-1` for "no current snapshot" with V1 and V2 tables and considers this equivalent to missing/null. This has never been formalized in the spec but for compatibility other implementations can accept `-1` as `null`. The Java implementation will no longer write `-1` and will use null for "no current snapshot for all tables with a version greater than or equal to V3 Review Comment: Puncutation nits: - remove extra space between `for "no current snapshot"` - need comma before 'but for compatibility .. ' (second is complete sentence) - need period at end - should we be consistent whether `null` is code-denoted or not? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org