danielcweeks commented on code in PR #12250:
URL: https://github.com/apache/iceberg/pull/12250#discussion_r1974017826


##########
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:
   I would think that if you capture the format version when you create the 
SerializableMetadataTable [like we do with name 
here](https://github.com/apache/iceberg/blob/22d14d5407356989415ccabd50071392c3c1076c/core/src/main/java/org/apache/iceberg/SerializableTable.java#L445)
 and it would be available for all metadata tables (not just positional deletes)
   
   I believe @aokolnychyi had a [similar 
comment](https://github.com/apache/iceberg/pull/11620#discussion_r1859414335) 
and I'm not sure I follow @RussellSpitzer concern (other than we already have 
multiple places to check to find what format version we're working with).
   
   In general, it would be nice if a table acts like a table and we can get the 
format version we're working against.  This just feels weird because we're 
updating the methods to pass through info we should have from the table.



-- 
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

Reply via email to