nastra commented on code in PR #12250:
URL: https://github.com/apache/iceberg/pull/12250#discussion_r1973052621
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkPositionDeletesRewrite.java:
##########
@@ -200,7 +207,8 @@ static class PositionDeletesWriterFactory implements
DataWriterFactory {
StructType dsSchema,
int specId,
StructLike partition,
- Map<String, String> writeProperties) {
+ Map<String, String> writeProperties,
+ int formatVersion) {
Review Comment:
we already have the format version in `SerializableTable` but in
https://github.com/apache/iceberg/pull/11620 we concluded that we don't want to
support fetching a format version from a `SerializableMetadataTable`.
There was also some discussion in
https://github.com/apache/iceberg/pull/11620#discussion_r1852542188 where we
basically concluded that it's probably better to let the calling site fetch the
underlying table from `PositionDeletesTable` instead of only supporting the
retrieval of a format version from the `PositionDeletesTable` and not all the
other metadata tables in `TableUtil`.
Also the reason why we're passing the `formatVersion` here is so that we
avoid doing a distributed table metadata load on worker nodes.
Does that make sense or do you think we should still make some changes to
properly support fetching the format version from the `PositionDeletesTable` in
`TableUtil`/`SerializableMetadataTable`?
--
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]
