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 2020/07/24 10:03:38 UTC

[GitHub] [iceberg] chenjunjiedada opened a new pull request #1239: Add incremental scan for iceberg generics scan builder

chenjunjiedada opened a new pull request #1239:
URL: https://github.com/apache/iceberg/pull/1239


   Currently, `IcebergGenerics.ScanBuilder` doesn't support incremental scan,  this adds the incremental scan support to the iceberg generic scan builder.


----------------------------------------------------------------
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:
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] chenjunjiedada commented on pull request #1239: Add incremental scan for iceberg generics scan builder

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


   @rdsr , Thanks for your review! I used this when building unit tests for Flink table source since we are using Iceberg generics for Flink. This will be updated if we changed to use Flink internal data model later.


----------------------------------------------------------------
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:
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] chenjunjiedada commented on a change in pull request #1239: Add incremental scan for iceberg generics scan builder

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



##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -174,6 +174,64 @@ private void overwriteExistingData() throws IOException {
         .commit();
   }
 
+  private void appendData() throws IOException {

Review comment:
       Agree with you. The test data starts to become a bit hard to use if we add more tests in future. I could try to clean this up, let me create an issue in case we forget it.

##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -174,6 +174,64 @@ private void overwriteExistingData() throws IOException {
         .commit();
   }
 
+  private void appendData() throws IOException {

Review comment:
       Agree with you. The test data starts to become a bit hard to use if we add more tests in future. I could try to clean this up, let me create an [issue ](https://github.com/apache/iceberg/issues/1262)in case we forget it.




----------------------------------------------------------------
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:
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] rdsr merged pull request #1239: Add incremental scan for iceberg generics scan builder

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


   


----------------------------------------------------------------
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:
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] chenjunjiedada closed pull request #1239: Add incremental scan for iceberg generics scan builder

Posted by GitBox <gi...@apache.org>.
chenjunjiedada closed pull request #1239:
URL: https://github.com/apache/iceberg/pull/1239


   


----------------------------------------------------------------
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:
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] rdsr commented on a change in pull request #1239: Add incremental scan for iceberg generics scan builder

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



##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -174,6 +174,64 @@ private void overwriteExistingData() throws IOException {
         .commit();
   }
 
+  private void appendData() throws IOException {

Review comment:
       I generally found the tests in TestLocalScan to be a little hard to read. It seems that the sharedTable being used in each test can have a different state depending upon the order of execution of the tests as it is initialized only once before all tests run. Also,  the data records `file1FirstSnapshotRecords` etc are being initialized by each test on their own, maybe we could make them constants. I don't think we need to take action on this though, if u all agree we can take it up as a separate task.

##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -174,6 +174,64 @@ private void overwriteExistingData() throws IOException {
         .commit();
   }
 
+  private void appendData() throws IOException {

Review comment:
       nit: I generally found the tests in TestLocalScan to be a little hard to read. It seems that the sharedTable being used in each test can have a different state depending upon the order of execution of the tests as it is initialized only once before all tests run. Also,  the data records `file1FirstSnapshotRecords` etc are being initialized by each test on their own, maybe we could make them constants. I don't think we need to take action on this though, if u all agree we can take it up as a separate task.




----------------------------------------------------------------
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:
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] rdsr commented on pull request #1239: Add incremental scan for iceberg generics scan builder

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


   I merged this. Thanks @chenjunjiedada!


----------------------------------------------------------------
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:
users@infra.apache.org



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