yyanyy commented on a change in pull request #1820:
URL: https://github.com/apache/iceberg/pull/1820#discussion_r552306700
##########
File path: core/src/test/java/org/apache/iceberg/TestManifestReaderStats.java
##########
@@ -74,7 +74,7 @@ public void testReadIncludesFullStats() throws IOException {
}
@Test
- public void testReadWithFilterIncludesFullStats() throws IOException {
+ public void testReadEntriesWithFilterIncludesFullStats() throws IOException {
Review comment:
I think the tests won't be able to verify this with calling
`ManifestReader`, since the only way to invoke `copyWithoutStats` within
`ManifestReader` is by calling `iterator()` which checks if stats should be
dropped first (from `dropStats`), and stats won't be dropped if any stat column
is selected. To test this I [added a
line](https://github.com/apache/iceberg/pull/1820/files#diff-4065997b5d6f2438a6d3993aff519e2fec33b9309c2013cea03ca7410adfd9f7R141)
to explicitly call `copyWithoutStats`. Please let me know if you have better
suggestion!
----------------------------------------------------------------
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]
