You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/07/27 15:20:22 UTC

[GitHub] [iceberg] Flyangz opened a new pull request #2875: added includeColumnStats option in FindFiles API

Flyangz opened a new pull request #2875:
URL: https://github.com/apache/iceberg/pull/2875


   This PR  added includeColumnStats option in FindFiles API so that we can choose to remain column stat when select data files and commit them to other iceberg table.
   Reference #2870  
   @openinx 


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] openinx commented on pull request #2875: Core: added includeColumnStats option in FindFiles API

Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #2875:
URL: https://github.com/apache/iceberg/pull/2875#issuecomment-889007987


   LGTM,  thanks @Flyangz for contributing !


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] openinx commented on a change in pull request #2875: Core: added includeColumnStats option in FindFiles API

Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #2875:
URL: https://github.com/apache/iceberg/pull/2875#discussion_r678030557



##########
File path: core/src/main/java/org/apache/iceberg/FindFiles.java
##########
@@ -206,7 +212,8 @@ public Builder inPartitions(PartitionSpec spec, List<StructLike> partitions) {
           .caseSensitive(caseSensitive)
           .entries();
 
-      return CloseableIterable.transform(entries, entry -> entry.file().copyWithoutStats());
+      return CloseableIterable.transform(entries,
+          entry -> includeColumnStats ? entry.file().copy() : entry.file().copyWithoutStats());

Review comment:
       Thanks @Flyangz for contributing,  but I don't think adding an option without any test case verification is the correct practice.  I think you can read this [PR](https://github.com/apache/iceberg/pull/206/files) to add the `includeColumnStats` in FindFiles API.




-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] openinx merged pull request #2875: Core: added includeColumnStats option in FindFiles API

Posted by GitBox <gi...@apache.org>.
openinx merged pull request #2875:
URL: https://github.com/apache/iceberg/pull/2875


   


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

[GitHub] [iceberg] Flyangz commented on a change in pull request #2875: Core: added includeColumnStats option in FindFiles API

Posted by GitBox <gi...@apache.org>.
Flyangz commented on a change in pull request #2875:
URL: https://github.com/apache/iceberg/pull/2875#discussion_r678469274



##########
File path: core/src/main/java/org/apache/iceberg/FindFiles.java
##########
@@ -206,7 +212,8 @@ public Builder inPartitions(PartitionSpec spec, List<StructLike> partitions) {
           .caseSensitive(caseSensitive)
           .entries();
 
-      return CloseableIterable.transform(entries, entry -> entry.file().copyWithoutStats());
+      return CloseableIterable.transform(entries,
+          entry -> includeColumnStats ? entry.file().copy() : entry.file().copyWithoutStats());

Review comment:
       Thanks reply. I have added the unit test, please check it out.




-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org