RussellSpitzer edited a comment on pull request #3059: URL: https://github.com/apache/iceberg/pull/3059#issuecomment-966521072
Ok so the test failure is real and is caused by the check here https://github.com/apache/iceberg/blob/f5a753791f4dc6aca78569a14f731feda9edf462/spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/source/SparkWrite.java#L543-L544 Which uses "isUnpartitioned" to check whether or not to use UnpartitionedDataWriter. UnpartitionedWriter passes through "null" as the value of the Spec which causes an NPE https://github.com/apache/iceberg/blame/9f07b83725e05206219368c020c3c772771a63d0/core/src/main/java/org/apache/iceberg/DataFiles.java#L50 Where we attempt to copy through the values into the spec (which is all void transforms). Before this patch V1 Tables with only void transforms would be considered "partitioned" by this code and deal with this correctly (but perhaps more confusingly.) As far as I can tell we have a few ways to go forward here. 1. The Unpartitioned Writer can get an empty version of the PartitionSpec passed through as the value, basically a struct like where all the values are empty. 2. The if statement between Unpartitioned and Partitioned can check whether there are no fields in the spec rather than whether the spec is unpartitioned 3. We change the copy method to always return early if the spec is unpartitioned, rather than trying to reset partition values I think I am leaning towards 1 or 3. I feel like we are dealing with an unpartitioned spec and because of V1 this means we have to be a bit careful about how we deal with that. -- 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]
