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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]