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


##########
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:
   If we change PartitionSpec.unpartitioned() itself to return some reserved id 
(like -1), we would break code which tests existing tables against the current 
value of that which expects specId of 0... changing unpartitioned() itself 
would similarly break code on existing tables made before the change 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.

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