dekimir commented on code in PR #16668:
URL: https://github.com/apache/iceberg/pull/16668#discussion_r3348513804


##########
core/src/test/java/org/apache/iceberg/DeleteFileIndexTestBase.java:
##########
@@ -480,6 +480,33 @@ public void 
testPartitionedTableScanWithGlobalAndPartitionDeletes() {
         .isEqualTo(Sets.newHashSet(unpartitionedEqDeletes.location(), 
FILE_A_EQ_1.location()));
   }
 
+  @TestTemplate
+  public void testGlobalEqDeleteWithSpecIdCollision() {
+    // PartitionSpec.unpartitioned() has specId 0, but does not reserve it so 
we must handle
+    // a collision where SpecId 0 is a partitioned spec.
+    
assertThat(SPEC.specId()).isEqualTo(PartitionSpec.unpartitioned().specId());
+
+    // Global equality delete written with PartitionSpec.unpartitioned() 
(specId = 0)
+    DeleteFile globalEqDelete =
+        withDataSequenceNumber(4, 
unpartitionedEqDeletes(PartitionSpec.unpartitioned()));
+
+    // specsById contains only the partitioned SPEC at ID 0, simulating a 
table that was created
+    // with a partitioned spec as its first (and only) spec. 
PartitionSpec.unpartitioned() is not
+    // in the table's spec history, but shares specId = 0 with the table's 
partitioned spec.

Review Comment:
   I'm not sure how big those two example problems are.  There is no external 
spec that reserves 0 for an unpartitioned spec.  And I'd expect callers to be 
much more likely to call `isPartitioned() / isUnpartitioned()` than to compare 
spec to 0.  After all, there can be multiple unpartitioned spec IDs -- schema 
evolution may make unpartitioned table partitioned and then unpartitioned again.



-- 
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: [email protected]

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