alliasgher commented on PR #875: URL: https://github.com/apache/iceberg-go/pull/875#issuecomment-4238079964
Thanks for the review — pushed the extra tests: - `TestRollingDataWriter/TestWriterFactoryPropagatesSortOrderID` covers the standard data write path (writerFactory + rolling data writer) with a non-zero default sort order. - `TestPositionDeleteUnpartitionedSortOrderID` and `TestEqualityDeleteUnpartitionedSortOrderID` cover the unpartitioned branches of `positionDeleteRecordsToDataFiles` and `equalityDeleteRecordsToDataFiles`. - `TestManifests/TestManifestRoundTripSortOrderID` writes a data file with a sort_order_id through `WriteManifest`, reads it back via `ReadManifest`, and verifies the value round-trips. One clarification question on the first point — could you take another look at [`equality_delete_writer.go` line 208](https://github.com/apache/iceberg-go/pull/875/files#diff-a041da9e1b29acca53a8b8d8b5fca7be8b16c3bce3f3bb8fcfd7c2c90b063a72) and [`arrow_utils.go` line 1620](https://github.com/apache/iceberg-go/pull/875/files#diff-38f706ad9be1ff50afebbbfe2050ed3be8090a9bdc3ae08b74a08b4769e9fdfc)? I believe both unpartitioned `WriteTask{}` constructions already set `SortOrderID: meta.defaultSortOrderID` in the diff, but let me know if I'm missing something or if you were pointing at a different site. -- 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]
