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/30 11:40:18 UTC
[GitHub] [iceberg] chenjunjiedada opened a new pull request #1273: Generics: Refactor TestLocalScan
chenjunjiedada opened a new pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273
This fixes #https://github.com/apache/iceberg/issues/1262.
----------------------------------------------------------------
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 #1273: Generics: Refactor TestLocalScan
Posted by GitBox <gi...@apache.org>.
chenjunjiedada commented on pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273#issuecomment-666315326
Hi @rdsr, would you mind to take a look?
----------------------------------------------------------------
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 #1273: Generics: Refactor TestLocalScan
Posted by GitBox <gi...@apache.org>.
chenjunjiedada commented on pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273#issuecomment-668342416
@edgarRd , Thanks for your comments! @rdsr, would you please help to take a look as well?
----------------------------------------------------------------
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 #1273: Generics: Refactor TestLocalScan
Posted by GitBox <gi...@apache.org>.
rdsr commented on pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273#issuecomment-668960825
Thanks @chenjunjiedada . Looking into it now.
----------------------------------------------------------------
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 #1273: Generics: Refactor TestLocalScan
Posted by GitBox <gi...@apache.org>.
rdsr merged pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273
----------------------------------------------------------------
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 #1273: Generics: Refactor TestLocalScan
Posted by GitBox <gi...@apache.org>.
chenjunjiedada commented on a change in pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273#discussion_r464756257
##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -103,132 +103,100 @@ public TestLocalScan(String format) {
private String sharedTableLocation = null;
private Table sharedTable = null;
- private List<Record> file1Records = null;
- private List<Record> file2Records = null;
- private List<Record> file3Records = null;
- private List<Record> file1FirstSnapshotRecords = null;
- private List<Record> file2FirstSnapshotRecords = null;
- private List<Record> file3FirstSnapshotRecords = null;
- private List<Record> file1SecondSnapshotRecords = null;
- private List<Record> file2SecondSnapshotRecords = null;
- private List<Record> file3SecondSnapshotRecords = null;
+ private final Record genericRecord = GenericRecord.create(SCHEMA);
+
+ private final List<Record> file1FirstRecords = Lists.newArrayList(
Review comment:
Good idea. Applied.
----------------------------------------------------------------
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] edgarRd commented on a change in pull request #1273: Generics: Refactor TestLocalScan
Posted by GitBox <gi...@apache.org>.
edgarRd commented on a change in pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273#discussion_r464557139
##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -103,132 +103,100 @@ public TestLocalScan(String format) {
private String sharedTableLocation = null;
private Table sharedTable = null;
- private List<Record> file1Records = null;
- private List<Record> file2Records = null;
- private List<Record> file3Records = null;
- private List<Record> file1FirstSnapshotRecords = null;
- private List<Record> file2FirstSnapshotRecords = null;
- private List<Record> file3FirstSnapshotRecords = null;
- private List<Record> file1SecondSnapshotRecords = null;
- private List<Record> file2SecondSnapshotRecords = null;
- private List<Record> file3SecondSnapshotRecords = null;
+ private final Record genericRecord = GenericRecord.create(SCHEMA);
+
+ private final List<Record> file1FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 0L, "data", "clarification")),
+ genericRecord.copy(ImmutableMap.of("id", 1L, "data", "risky")),
+ genericRecord.copy(ImmutableMap.of("id", 2L, "data", "falafel"))
+ );
+ private final List<Record> file2FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 10L, "data", "clammy")),
+ genericRecord.copy(ImmutableMap.of("id", 11L, "data", "evacuate")),
+ genericRecord.copy(ImmutableMap.of("id", 12L, "data", "tissue"))
+ );
+ private final List<Record> file3FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 20L, "data", "ocean")),
+ genericRecord.copy(ImmutableMap.of("id", 21L, "data", "holistic")),
+ genericRecord.copy(ImmutableMap.of("id", 22L, "data", "preventative"))
+ );
+
+ private final List<Record> file1SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 4L, "data", "obscure")),
+ genericRecord.copy(ImmutableMap.of("id", 5L, "data", "secure")),
+ genericRecord.copy(ImmutableMap.of("id", 6L, "data", "fetta"))
+ );
+ private final List<Record> file2SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 14L, "data", "radical")),
+ genericRecord.copy(ImmutableMap.of("id", 15L, "data", "collocation")),
+ genericRecord.copy(ImmutableMap.of("id", 16L, "data", "book"))
+ );
+ private final List<Record> file3SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 24L, "data", "cloud")),
+ genericRecord.copy(ImmutableMap.of("id", 25L, "data", "zen")),
+ genericRecord.copy(ImmutableMap.of("id", 26L, "data", "sky"))
+ );
+
+ private final List<Record> file1ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 6L, "data", "brainy")),
+ genericRecord.copy(ImmutableMap.of("id", 7L, "data", "film")),
+ genericRecord.copy(ImmutableMap.of("id", 8L, "data", "fetta"))
+ );
+ private final List<Record> file2ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 16L, "data", "cake")),
+ genericRecord.copy(ImmutableMap.of("id", 17L, "data", "intrinsic")),
+ genericRecord.copy(ImmutableMap.of("id", 18L, "data", "paper"))
+ );
+ private final List<Record> file3ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 26L, "data", "belleview")),
+ genericRecord.copy(ImmutableMap.of("id", 27L, "data", "overview")),
+ genericRecord.copy(ImmutableMap.of("id", 28L, "data", "tender"))
+ );
private void overwriteExistingData() throws IOException {
- Record record = GenericRecord.create(SCHEMA);
-
- this.file1FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 4L, "data", "obscure")),
- record.copy(ImmutableMap.of("id", 5L, "data", "secure")),
- record.copy(ImmutableMap.of("id", 6L, "data", "fetta"))
- );
- DataFile file11 = writeFile(sharedTableLocation, format.addExtension("file-11"), file1FirstSnapshotRecords);
-
- this.file2FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 14L, "data", "radical")),
- record.copy(ImmutableMap.of("id", 15L, "data", "collocation")),
- record.copy(ImmutableMap.of("id", 16L, "data", "book"))
- );
- DataFile file21 = writeFile(sharedTableLocation, format.addExtension("file-21"), file2FirstSnapshotRecords);
-
- this.file3FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 24L, "data", "cloud")),
- record.copy(ImmutableMap.of("id", 25L, "data", "zen")),
- record.copy(ImmutableMap.of("id", 26L, "data", "sky"))
- );
- DataFile file31 = writeFile(sharedTableLocation, format.addExtension("file-31"), file3FirstSnapshotRecords);
+ DataFile file12 = writeFile(sharedTableLocation, format.addExtension("file-11"), file1SecondSnapshotRecords);
Review comment:
It's a bit confusing that the name of the `DataFile` variable is different from the extension name, e.g. in this case `DataFile` is `file12` and extension `file-11`. Is there a reason for the rename from how it was before?
##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -103,132 +103,100 @@ public TestLocalScan(String format) {
private String sharedTableLocation = null;
private Table sharedTable = null;
- private List<Record> file1Records = null;
- private List<Record> file2Records = null;
- private List<Record> file3Records = null;
- private List<Record> file1FirstSnapshotRecords = null;
- private List<Record> file2FirstSnapshotRecords = null;
- private List<Record> file3FirstSnapshotRecords = null;
- private List<Record> file1SecondSnapshotRecords = null;
- private List<Record> file2SecondSnapshotRecords = null;
- private List<Record> file3SecondSnapshotRecords = null;
+ private final Record genericRecord = GenericRecord.create(SCHEMA);
+
+ private final List<Record> file1FirstRecords = Lists.newArrayList(
Review comment:
Looks like the goal is to make these immutable, have you considered using `ImmutableList` too?
##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -103,132 +103,100 @@ public TestLocalScan(String format) {
private String sharedTableLocation = null;
private Table sharedTable = null;
- private List<Record> file1Records = null;
- private List<Record> file2Records = null;
- private List<Record> file3Records = null;
- private List<Record> file1FirstSnapshotRecords = null;
- private List<Record> file2FirstSnapshotRecords = null;
- private List<Record> file3FirstSnapshotRecords = null;
- private List<Record> file1SecondSnapshotRecords = null;
- private List<Record> file2SecondSnapshotRecords = null;
- private List<Record> file3SecondSnapshotRecords = null;
+ private final Record genericRecord = GenericRecord.create(SCHEMA);
+
+ private final List<Record> file1FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 0L, "data", "clarification")),
+ genericRecord.copy(ImmutableMap.of("id", 1L, "data", "risky")),
+ genericRecord.copy(ImmutableMap.of("id", 2L, "data", "falafel"))
+ );
+ private final List<Record> file2FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 10L, "data", "clammy")),
+ genericRecord.copy(ImmutableMap.of("id", 11L, "data", "evacuate")),
+ genericRecord.copy(ImmutableMap.of("id", 12L, "data", "tissue"))
+ );
+ private final List<Record> file3FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 20L, "data", "ocean")),
+ genericRecord.copy(ImmutableMap.of("id", 21L, "data", "holistic")),
+ genericRecord.copy(ImmutableMap.of("id", 22L, "data", "preventative"))
+ );
+
+ private final List<Record> file1SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 4L, "data", "obscure")),
+ genericRecord.copy(ImmutableMap.of("id", 5L, "data", "secure")),
+ genericRecord.copy(ImmutableMap.of("id", 6L, "data", "fetta"))
+ );
+ private final List<Record> file2SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 14L, "data", "radical")),
+ genericRecord.copy(ImmutableMap.of("id", 15L, "data", "collocation")),
+ genericRecord.copy(ImmutableMap.of("id", 16L, "data", "book"))
+ );
+ private final List<Record> file3SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 24L, "data", "cloud")),
+ genericRecord.copy(ImmutableMap.of("id", 25L, "data", "zen")),
+ genericRecord.copy(ImmutableMap.of("id", 26L, "data", "sky"))
+ );
+
+ private final List<Record> file1ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 6L, "data", "brainy")),
+ genericRecord.copy(ImmutableMap.of("id", 7L, "data", "film")),
+ genericRecord.copy(ImmutableMap.of("id", 8L, "data", "fetta"))
+ );
+ private final List<Record> file2ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 16L, "data", "cake")),
+ genericRecord.copy(ImmutableMap.of("id", 17L, "data", "intrinsic")),
+ genericRecord.copy(ImmutableMap.of("id", 18L, "data", "paper"))
+ );
+ private final List<Record> file3ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 26L, "data", "belleview")),
+ genericRecord.copy(ImmutableMap.of("id", 27L, "data", "overview")),
+ genericRecord.copy(ImmutableMap.of("id", 28L, "data", "tender"))
+ );
private void overwriteExistingData() throws IOException {
- Record record = GenericRecord.create(SCHEMA);
-
- this.file1FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 4L, "data", "obscure")),
- record.copy(ImmutableMap.of("id", 5L, "data", "secure")),
- record.copy(ImmutableMap.of("id", 6L, "data", "fetta"))
- );
- DataFile file11 = writeFile(sharedTableLocation, format.addExtension("file-11"), file1FirstSnapshotRecords);
-
- this.file2FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 14L, "data", "radical")),
- record.copy(ImmutableMap.of("id", 15L, "data", "collocation")),
- record.copy(ImmutableMap.of("id", 16L, "data", "book"))
- );
- DataFile file21 = writeFile(sharedTableLocation, format.addExtension("file-21"), file2FirstSnapshotRecords);
-
- this.file3FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 24L, "data", "cloud")),
- record.copy(ImmutableMap.of("id", 25L, "data", "zen")),
- record.copy(ImmutableMap.of("id", 26L, "data", "sky"))
- );
- DataFile file31 = writeFile(sharedTableLocation, format.addExtension("file-31"), file3FirstSnapshotRecords);
+ DataFile file12 = writeFile(sharedTableLocation, format.addExtension("file-11"), file1SecondSnapshotRecords);
Review comment:
I think this renaming produces some changes down below too.
----------------------------------------------------------------
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 #1273: Generics: Refactor TestLocalScan
Posted by GitBox <gi...@apache.org>.
chenjunjiedada commented on a change in pull request #1273:
URL: https://github.com/apache/iceberg/pull/1273#discussion_r464756103
##########
File path: data/src/test/java/org/apache/iceberg/data/TestLocalScan.java
##########
@@ -103,132 +103,100 @@ public TestLocalScan(String format) {
private String sharedTableLocation = null;
private Table sharedTable = null;
- private List<Record> file1Records = null;
- private List<Record> file2Records = null;
- private List<Record> file3Records = null;
- private List<Record> file1FirstSnapshotRecords = null;
- private List<Record> file2FirstSnapshotRecords = null;
- private List<Record> file3FirstSnapshotRecords = null;
- private List<Record> file1SecondSnapshotRecords = null;
- private List<Record> file2SecondSnapshotRecords = null;
- private List<Record> file3SecondSnapshotRecords = null;
+ private final Record genericRecord = GenericRecord.create(SCHEMA);
+
+ private final List<Record> file1FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 0L, "data", "clarification")),
+ genericRecord.copy(ImmutableMap.of("id", 1L, "data", "risky")),
+ genericRecord.copy(ImmutableMap.of("id", 2L, "data", "falafel"))
+ );
+ private final List<Record> file2FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 10L, "data", "clammy")),
+ genericRecord.copy(ImmutableMap.of("id", 11L, "data", "evacuate")),
+ genericRecord.copy(ImmutableMap.of("id", 12L, "data", "tissue"))
+ );
+ private final List<Record> file3FirstRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 20L, "data", "ocean")),
+ genericRecord.copy(ImmutableMap.of("id", 21L, "data", "holistic")),
+ genericRecord.copy(ImmutableMap.of("id", 22L, "data", "preventative"))
+ );
+
+ private final List<Record> file1SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 4L, "data", "obscure")),
+ genericRecord.copy(ImmutableMap.of("id", 5L, "data", "secure")),
+ genericRecord.copy(ImmutableMap.of("id", 6L, "data", "fetta"))
+ );
+ private final List<Record> file2SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 14L, "data", "radical")),
+ genericRecord.copy(ImmutableMap.of("id", 15L, "data", "collocation")),
+ genericRecord.copy(ImmutableMap.of("id", 16L, "data", "book"))
+ );
+ private final List<Record> file3SecondSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 24L, "data", "cloud")),
+ genericRecord.copy(ImmutableMap.of("id", 25L, "data", "zen")),
+ genericRecord.copy(ImmutableMap.of("id", 26L, "data", "sky"))
+ );
+
+ private final List<Record> file1ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 6L, "data", "brainy")),
+ genericRecord.copy(ImmutableMap.of("id", 7L, "data", "film")),
+ genericRecord.copy(ImmutableMap.of("id", 8L, "data", "fetta"))
+ );
+ private final List<Record> file2ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 16L, "data", "cake")),
+ genericRecord.copy(ImmutableMap.of("id", 17L, "data", "intrinsic")),
+ genericRecord.copy(ImmutableMap.of("id", 18L, "data", "paper"))
+ );
+ private final List<Record> file3ThirdSnapshotRecords = Lists.newArrayList(
+ genericRecord.copy(ImmutableMap.of("id", 26L, "data", "belleview")),
+ genericRecord.copy(ImmutableMap.of("id", 27L, "data", "overview")),
+ genericRecord.copy(ImmutableMap.of("id", 28L, "data", "tender"))
+ );
private void overwriteExistingData() throws IOException {
- Record record = GenericRecord.create(SCHEMA);
-
- this.file1FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 4L, "data", "obscure")),
- record.copy(ImmutableMap.of("id", 5L, "data", "secure")),
- record.copy(ImmutableMap.of("id", 6L, "data", "fetta"))
- );
- DataFile file11 = writeFile(sharedTableLocation, format.addExtension("file-11"), file1FirstSnapshotRecords);
-
- this.file2FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 14L, "data", "radical")),
- record.copy(ImmutableMap.of("id", 15L, "data", "collocation")),
- record.copy(ImmutableMap.of("id", 16L, "data", "book"))
- );
- DataFile file21 = writeFile(sharedTableLocation, format.addExtension("file-21"), file2FirstSnapshotRecords);
-
- this.file3FirstSnapshotRecords = Lists.newArrayList(
- record.copy(ImmutableMap.of("id", 24L, "data", "cloud")),
- record.copy(ImmutableMap.of("id", 25L, "data", "zen")),
- record.copy(ImmutableMap.of("id", 26L, "data", "sky"))
- );
- DataFile file31 = writeFile(sharedTableLocation, format.addExtension("file-31"), file3FirstSnapshotRecords);
+ DataFile file12 = writeFile(sharedTableLocation, format.addExtension("file-11"), file1SecondSnapshotRecords);
Review comment:
Nice catch, fixed the extension.
The first snapshot is actually consisted of `file1FirstRecords`, `file2FirstRecords` and `file3FirstRecords`, so I renamed this to match the actual snapshot order.
----------------------------------------------------------------
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