RussellSpitzer edited a comment on pull request #3273: URL: https://github.com/apache/iceberg/pull/3273#issuecomment-940376420
Checking the evaluators it looks like we really do need a negative 1 here see https://github.com/apache/iceberg/blob/7aef02b9a192f62740f16a883a6ec955cdcd7aff/api/src/main/java/org/apache/iceberg/expressions/InclusiveMetricsEvaluator.java#L90-L99 If we set it to 0 then files will always be ignored, if we set it to -1 then that is currently used as a signal that the file has no metric information and must be scanned. This has been broken ever since we switched from using a direct DataFile constructor So I think we only have a few options 1. Change the builder so that it allows -1 2. Go back to using the raw datafile constructor I think approach 1. is the safest thing to do here for future proofing behavior. Specifically I think we should allow for explicitly setting the rowCount to -1 if and only if the rest of the metrics are empty. This should preserve the behavior of allowing a file without metrics but also insure that we don't have rowcount as -1 when the metrics are set. -- 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]
