badalprasadsingh commented on PR #1214: URL: https://github.com/apache/iceberg-go/pull/1214#issuecomment-4799537092
Hey @laskoviymishka , @zeroshade thank you so much for the review. I have addressed all of the above mentioned comments. The two major _blockers_: 1. **Panic -> errors**: Added an up-front _comma-ok_ assertion on `Metadata`, plus a deferred `recover()` in `DataFileFromMetadata`. 2. **V3 tables**: Added `FirstRowID *int64` to `DataFileArgs` and plumbed it through `ToDataFile`. The other smaller comments, all in: 1. Positional deletes now reject a nonzero `SortOrderID`, matching the writer. 2. Field-ID validation: `EqualityFieldIDs` must exist in the schema for _eq-deletes_, and the schema must be compatible with `PositionalDeleteSchema` ( _reserved IDs_ present and correctly typed ) for _pos-deletes_. 3. Partition values are validated against the _spec_ up front ( missing / extra field IDs ) instead of failing at commit or panicking. 4. Tests are now _table-driven_, decode and assert real _min/max_ bounds (not just map lengths), and cover the error cases — including the recover path and the validation failures above. 5. Added `validatePartitionValues` so a _Spec/PartitionValues_ mismatch fails at the boundary instead of at commit, and the one mismatch that depends on file stats (multi-value partition) is now caught by a _deferred_ `recover()` and returned as an error. <br></br> Added partitioned-spec tests covering the happy path, missing value, unknown field id, and the multi-value panic path. cc: @zeroshade On `Metadata`: I have kept it as `any` but made the requirement explicit and fail-fast via the comma-ok assertion. Reason: the entry point routes through the internal FileFormat seam (`DataFileStatsFromMeta(meta Metadata, …)`), which is intentionally format-agnostic. Ready for another _pass_ whenever you have a moment — thanks again! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
