rdblue commented on a change in pull request #2358:
URL: https://github.com/apache/iceberg/pull/2358#discussion_r639160362



##########
File path: core/src/test/java/org/apache/iceberg/TestMetadataTableScans.java
##########
@@ -159,6 +162,105 @@ public void testAllManifestsTableHonorsIgnoreResiduals() 
throws IOException {
     validateTaskScanResiduals(scan2, true);
   }
 
+  @Test
+  public void testPartitionsTableScan() throws IOException {
+
+    // Make 4 Manifests
+    table.newFastAppend()
+        .appendFile(FILE_PARTITION_0)
+        .commit();
+    table.newFastAppend()
+        .appendFile(FILE_PARTITION_1)
+        .commit();
+    table.newFastAppend()
+        .appendFile(FILE_PARTITION_2)
+        .commit();
+    table.newFastAppend()
+        .appendFile(FILE_PARTITION_3)
+        .commit();
+
+    Table partitionsTable = new PartitionsTable(table.ops(), table);
+    Types.StructType expected = new Schema(
+        required(1, "partition", Types.StructType.of(
+            optional(1000, "data_bucket", 
Types.IntegerType.get())))).asStruct();
+
+    // Sanity check a variety of partition predicates.
+    TableScan scanNoFilter = partitionsTable.newScan()
+        .select("partition.data_bucket");
+    Assert.assertEquals(expected, scanNoFilter.schema().asStruct());
+    CloseableIterable<FileScanTask> tasksNoFilter = ((PartitionsTable) 
partitionsTable)
+        .planTasks((StaticTableScan) scanNoFilter);
+    Assert.assertEquals(4, Iterators.size(tasksNoFilter.iterator()));
+    validateIncludes(tasksNoFilter, 0);
+    validateIncludes(tasksNoFilter, 1);
+    validateIncludes(tasksNoFilter, 2);
+    validateIncludes(tasksNoFilter, 3);
+
+    Expression andEquals = Expressions.and(
+        Expressions.equal("partition.data_bucket", 0),
+        Expressions.greaterThan("record_count", 0));
+    TableScan scanAndEq = partitionsTable.newScan()
+        .filter(andEquals);
+    CloseableIterable<FileScanTask> tasksAndEq = ((PartitionsTable) 
partitionsTable)
+        .planTasks((StaticTableScan) scanAndEq);
+    Assert.assertEquals(1, Iterators.size(tasksAndEq.iterator()));
+    validateIncludes(tasksAndEq, 0);
+
+    Expression ltAnd = Expressions.and(
+        Expressions.lessThan("partition.data_bucket", 2),
+        Expressions.greaterThan("record_count", 0));
+    TableScan scanLtAnd = partitionsTable.newScan()
+        .filter(ltAnd);
+    CloseableIterable<FileScanTask> tasksLtAnd = ((PartitionsTable) 
partitionsTable)
+        .planTasks((StaticTableScan) scanLtAnd);
+    Assert.assertEquals(2, Iterators.size(tasksLtAnd.iterator()));
+    validateIncludes(tasksLtAnd, 0);
+    validateIncludes(tasksLtAnd, 1);
+
+    Expression or = Expressions.or(
+        Expressions.equal("partition.data_bucket", 2),
+        Expressions.greaterThan("record_count", 0));
+    TableScan scanOr = partitionsTable.newScan()
+        .filter(or);
+    CloseableIterable<FileScanTask> tasksOr = ((PartitionsTable) 
partitionsTable)
+        .planTasks((StaticTableScan) scanOr);
+    Assert.assertEquals(4, Iterators.size(tasksOr.iterator()));
+    validateIncludes(tasksOr, 0);
+    validateIncludes(tasksOr, 1);
+    validateIncludes(tasksOr, 2);
+    validateIncludes(tasksOr, 3);
+
+    Expression not = Expressions.not(
+        Expressions.lessThan("partition.data_bucket", 2));
+    TableScan scanNot = partitionsTable.newScan()
+        .filter(not);
+    CloseableIterable<FileScanTask> tasksNot = ((PartitionsTable) 
partitionsTable)
+        .planTasks((StaticTableScan) scanNot);
+    Assert.assertEquals(2, Iterators.size(tasksNot.iterator()));
+    validateIncludes(tasksNot, 2);
+    validateIncludes(tasksNot, 3);
+
+    Expression set = Expressions.in("partition.data_bucket", 2, 3);
+    TableScan scanSet = partitionsTable.newScan()
+        .filter(set);
+    CloseableIterable<FileScanTask> tasksSet = ((PartitionsTable) 
partitionsTable)
+        .planTasks((StaticTableScan) scanSet);
+    Assert.assertEquals(2, Iterators.size(tasksSet.iterator()));
+    validateIncludes(tasksSet, 2);
+    validateIncludes(tasksSet, 3);
+
+    Expression unary = Expressions.notNull("partition.data_bucket");
+    TableScan scanUnary = partitionsTable.newScan()
+        .filter(unary);
+    CloseableIterable<FileScanTask> tasksUnary = ((PartitionsTable) 
partitionsTable)
+        .planTasks((StaticTableScan) scanUnary);
+    Assert.assertEquals(4, Iterators.size(tasksUnary.iterator()));
+    validateIncludes(tasksUnary, 0);
+    validateIncludes(tasksUnary, 1);
+    validateIncludes(tasksUnary, 2);
+    validateIncludes(tasksUnary, 3);

Review comment:
       I haven't looked very closely, but this appears to be a really long 
test. My guess is that you're mixing test cases that should be in separate 
methods. Can you clean it up so that you're testing cases individually?




-- 
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]

Reply via email to