yyanyy commented on issue #2405: URL: https://github.com/apache/iceberg/issues/2405#issuecomment-827171072
I think the `required` keyword in manifests table @RussellSpitzer pointed out is the culprit, I think `StaticDataTask.Row` should be able to handle null but since we marked `containsNan` as required in manifests table, it then throws the exception. And I agree with Russell that at least for showing manifest table, we probably don't want to mark `containsNan` as true if originally it's null/missing as it could be misleading. If we indeed want to make `containsNan` a primitive everywhere, there would be other places to change including a [public facing interface](https://github.com/apache/iceberg/blob/master/api/src/main/java/org/apache/iceberg/ManifestFile.java#L209-L211) that are introduced in #1872 and [a case](https://github.com/apache/iceberg/blob/master/api/src/main/java/org/apache/iceberg/expressions/ManifestEvaluator.java#L164) that will produce less accurate result. However we haven't specify if we want `contains_nan` to be nullable or not [in spec](https://iceberg.apache.org/spec/#manifest-lists) so this decision might still be revertible. I'll submit a PR to fix `required` field in manifests table and a separate PR to update spec so that we can have a wider audience and make a decision there. Meanwhile @RussellSpitzer @chenjunjiedada please let me know if you have further comments/concerns, and sorry for causing the issue! -- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
