(iceberg) branch main updated: Core: Support merging in PositionDeleteIndex (#11208)

[aokolnychyi]

This is an automated email from the ASF dual-hosted git repository.

aokolnychyi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/main by this push:
     new 1e5dcb1f6b Core: Support merging in PositionDeleteIndex (#11208)
1e5dcb1f6b is described below

commit 1e5dcb1f6b01128032a704258e1730c9936c9a07
Author: Anton Okolnychyi <aokolnyc...@apache.org>
AuthorDate: Wed Sep 25 16:22:42 2024 -0700

    Core: Support merging in PositionDeleteIndex (#11208)
---
 .../iceberg/deletes/BitmapPositionDeleteIndex.java |  9 +++++
 .../iceberg/deletes/PositionDeleteIndex.java       |  9 +++++
 .../iceberg/deletes/PositionDeleteIndexUtil.java   | 14 +-------
 .../deletes/TestBitmapPositionDeleteIndex.java     | 41 ++++++++++++++++++++++
 4 files changed, 60 insertions(+), 13 deletions(-)

diff --git 
a/core/src/main/java/org/apache/iceberg/deletes/BitmapPositionDeleteIndex.java 
b/core/src/main/java/org/apache/iceberg/deletes/BitmapPositionDeleteIndex.java
index 7503d0d83f..3a04487856 100644
--- 
a/core/src/main/java/org/apache/iceberg/deletes/BitmapPositionDeleteIndex.java
+++ 
b/core/src/main/java/org/apache/iceberg/deletes/BitmapPositionDeleteIndex.java
@@ -42,6 +42,15 @@ class BitmapPositionDeleteIndex implements 
PositionDeleteIndex {
     roaring64Bitmap.addRange(posStart, posEnd);
   }
 
+  @Override
+  public void merge(PositionDeleteIndex that) {
+    if (that instanceof BitmapPositionDeleteIndex) {
+      merge((BitmapPositionDeleteIndex) that);
+    } else {
+      that.forEach(this::delete);
+    }
+  }
+
   @Override
   public boolean isDeleted(long position) {
     return roaring64Bitmap.contains(position);
diff --git 
a/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndex.java 
b/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndex.java
index 27c15749ad..c0086fe6aa 100644
--- a/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndex.java
+++ b/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndex.java
@@ -36,6 +36,15 @@ public interface PositionDeleteIndex {
    */
   void delete(long posStart, long posEnd);
 
+  /**
+   * Adds positions from the other index, modifying this index in place.
+   *
+   * @param that the other index to merge
+   */
+  default void merge(PositionDeleteIndex that) {
+    that.forEach(this::delete);
+  }
+
   /**
    * Checks whether a row at the position is deleted.
    *
diff --git 
a/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndexUtil.java 
b/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndexUtil.java
index 0c3bff28ee..7601232ad2 100644
--- a/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndexUtil.java
+++ b/core/src/main/java/org/apache/iceberg/deletes/PositionDeleteIndexUtil.java
@@ -18,25 +18,13 @@
  */
 package org.apache.iceberg.deletes;
 
-import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
-
 public class PositionDeleteIndexUtil {
 
   private PositionDeleteIndexUtil() {}
 
   public static PositionDeleteIndex merge(Iterable<? extends 
PositionDeleteIndex> indexes) {
     BitmapPositionDeleteIndex result = new BitmapPositionDeleteIndex();
-
-    for (PositionDeleteIndex index : indexes) {
-      if (index.isNotEmpty()) {
-        Preconditions.checkArgument(
-            index instanceof BitmapPositionDeleteIndex,
-            "Can merge only bitmap-based indexes, got %s",
-            index.getClass().getName());
-        result.merge((BitmapPositionDeleteIndex) index);
-      }
-    }
-
+    indexes.forEach(result::merge);
     return result;
   }
 }
diff --git 
a/core/src/test/java/org/apache/iceberg/deletes/TestBitmapPositionDeleteIndex.java
 
b/core/src/test/java/org/apache/iceberg/deletes/TestBitmapPositionDeleteIndex.java
index 279c5b8d16..c8fc723deb 100644
--- 
a/core/src/test/java/org/apache/iceberg/deletes/TestBitmapPositionDeleteIndex.java
+++ 
b/core/src/test/java/org/apache/iceberg/deletes/TestBitmapPositionDeleteIndex.java
@@ -64,6 +64,47 @@ public class TestBitmapPositionDeleteIndex {
     assertThat(positions).isEmpty();
   }
 
+  @Test
+  public void testMergeBitmapIndexWithNonEmpty() {
+    long pos1 = 10L; // Container 0 (high bits = 0)
+    long pos2 = 1L << 33; // Container 1 (high bits = 1)
+    long pos3 = pos2 + 1; // Container 1 (high bits = 1)
+    long pos4 = 2L << 33; // Container 2 (high bits = 2)
+
+    BitmapPositionDeleteIndex index1 = new BitmapPositionDeleteIndex();
+    index1.delete(pos2);
+    index1.delete(pos1);
+
+    BitmapPositionDeleteIndex index2 = new BitmapPositionDeleteIndex();
+    index2.delete(pos4);
+    index2.delete(pos3);
+
+    index1.merge(index2);
+
+    // output must be sorted in ascending order across containers
+    List<Long> positions = collect(index1);
+    assertThat(positions).containsExactly(pos1, pos2, pos3, pos4);
+  }
+
+  @Test
+  public void testMergeBitmapIndexWithEmpty() {
+    long pos1 = 10L; // Container 0 (high bits = 0)
+    long pos2 = 1L << 33; // Container 1 (high bits = 1)
+    long pos3 = pos2 + 1; // Container 1 (high bits = 1)
+    long pos4 = 2L << 33; // Container 2 (high bits = 2)
+
+    BitmapPositionDeleteIndex index = new BitmapPositionDeleteIndex();
+    index.delete(pos2);
+    index.delete(pos1);
+    index.delete(pos3);
+    index.delete(pos4);
+    index.merge(PositionDeleteIndex.empty());
+
+    // output must be sorted in ascending order across containers
+    List<Long> positions = collect(index);
+    assertThat(positions).containsExactly(pos1, pos2, pos3, pos4);
+  }
+
   private List<Long> collect(PositionDeleteIndex index) {
     List<Long> positions = Lists.newArrayList();
     index.forEach(positions::add);