mxm commented on code in PR #15996:
URL: https://github.com/apache/iceberg/pull/15996#discussion_r3155896445


##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertDVResolver.java:
##########
@@ -0,0 +1,249 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.flink.maintenance.operator;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.streaming.api.operators.AbstractStreamOperator;
+import org.apache.flink.streaming.api.operators.TwoInputStreamOperator;
+import org.apache.flink.streaming.api.watermark.Watermark;
+import org.apache.flink.streaming.runtime.streamrecord.StreamRecord;
+import org.apache.iceberg.DataFile;
+import org.apache.iceberg.DeleteFile;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.ManifestFiles;
+import org.apache.iceberg.ManifestReader;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Snapshot;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.flink.TableLoader;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.util.ContentFileUtil;
+import org.apache.iceberg.util.StructLikeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Collects {@link DVPosition}s from {@link EqualityConvertWorker} instances 
(input 1) and an {@link
+ * EqualityConvertPlanResult} from the planner (input 2). On watermark 
advancement, groups DVs, and
+ * emits {@link DVMergeCommand}s for parallel {@link EqualityConvertDVMerger} 
instances.
+ */
+@Internal
+public class EqualityConvertDVResolver extends 
AbstractStreamOperator<DVMergeCommand>
+    implements TwoInputStreamOperator<DVPosition, EqualityConvertPlanResult, 
DVMergeCommand> {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(EqualityConvertDVResolver.class);
+
+  private final String tableName;
+  private final String taskName;
+  private final int taskIndex;
+  private final TableLoader tableLoader;
+  private final String targetBranch;
+
+  private transient Table table;
+  private transient List<DVPosition> bufferedPositions;
+  private transient EqualityConvertPlanResult planResult;
+  private transient boolean hasUpstreamError;
+
+  public EqualityConvertDVResolver(
+      String tableName,
+      String taskName,
+      int taskIndex,
+      TableLoader tableLoader,
+      String targetBranch) {
+    this.tableName = tableName;
+    this.taskName = taskName;
+    this.taskIndex = taskIndex;
+    this.tableLoader = tableLoader;
+    this.targetBranch = targetBranch;
+  }
+
+  @Override
+  public void open() throws Exception {
+    super.open();
+    if (!tableLoader.isOpen()) {
+      tableLoader.open();
+    }
+
+    table = tableLoader.loadTable();
+    bufferedPositions = Lists.newArrayList();
+  }
+
+  @Override
+  public void processElement1(StreamRecord<DVPosition> record) {
+    if (record.getValue().isAbort()) {
+      hasUpstreamError = true;
+    } else {
+      bufferedPositions.add(record.getValue());
+    }
+  }
+
+  @Override
+  public void processElement2(StreamRecord<EqualityConvertPlanResult> record) {
+    planResult = record.getValue();
+  }
+
+  @Override
+  public void processWatermark(Watermark mark) throws Exception {
+    if (planResult != null && mark.getTimestamp() >= 
planResult.doneTimestamp()) {
+      if (hasUpstreamError) {
+        output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+      } else {
+        try {
+          resolveAndEmit();
+        } catch (Exception e) {
+          LOG.error(
+              "Error resolving DVs for table {} task {}[{}]", tableName, 
taskName, taskIndex, e);
+          output.collect(TaskResultAggregator.ERROR_STREAM, new 
StreamRecord<>(e));
+          output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+        }
+      }
+
+      bufferedPositions.clear();
+      hasUpstreamError = false;
+      planResult = null;
+    }
+
+    super.processWatermark(mark);
+  }
+
+  private void resolveAndEmit() {
+    if (bufferedPositions.isEmpty()) {
+      return;
+    }
+
+    table.refresh();
+
+    Snapshot mainSnapshot = table.snapshot(targetBranch);
+    Map<String, PartitionAndSpec> partitions = 
collectDataFilePartitions(mainSnapshot);
+    Map<String, DeleteFile> dvs = collectExistingDVs(mainSnapshot);
+
+    for (DataFile df : planResult.dataFiles()) {
+      partitions.put(
+          df.location(),
+          new PartitionAndSpec(
+              table.specs().get(df.specId()), 
StructLikeUtil.copy(df.partition())));
+    }
+
+    // Include staging DVs so the merger folds them in; prevents two DVs per 
data file (V3
+    // violation).
+    for (DeleteFile sd : planResult.stagingDVFiles()) {
+      if (ContentFileUtil.isDV(sd) && sd.referencedDataFile() != null) {
+        dvs.put(sd.referencedDataFile(), sd.copy());

Review Comment:
   The JavaDoc of the ContentFile says: 
   
   ```java
     /**
      * Copies this file. Manifest readers can reuse file instances; use this 
method to copy data when
      * collecting files from tasks.
      *
      * @return a copy of this data file
      */
     F copy();
   ```



##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertDVResolver.java:
##########
@@ -0,0 +1,249 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.flink.maintenance.operator;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.streaming.api.operators.AbstractStreamOperator;
+import org.apache.flink.streaming.api.operators.TwoInputStreamOperator;
+import org.apache.flink.streaming.api.watermark.Watermark;
+import org.apache.flink.streaming.runtime.streamrecord.StreamRecord;
+import org.apache.iceberg.DataFile;
+import org.apache.iceberg.DeleteFile;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.ManifestFiles;
+import org.apache.iceberg.ManifestReader;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Snapshot;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.flink.TableLoader;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.util.ContentFileUtil;
+import org.apache.iceberg.util.StructLikeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Collects {@link DVPosition}s from {@link EqualityConvertWorker} instances 
(input 1) and an {@link
+ * EqualityConvertPlanResult} from the planner (input 2). On watermark 
advancement, groups DVs, and
+ * emits {@link DVMergeCommand}s for parallel {@link EqualityConvertDVMerger} 
instances.
+ */
+@Internal
+public class EqualityConvertDVResolver extends 
AbstractStreamOperator<DVMergeCommand>
+    implements TwoInputStreamOperator<DVPosition, EqualityConvertPlanResult, 
DVMergeCommand> {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(EqualityConvertDVResolver.class);
+
+  private final String tableName;
+  private final String taskName;
+  private final int taskIndex;
+  private final TableLoader tableLoader;
+  private final String targetBranch;
+
+  private transient Table table;
+  private transient List<DVPosition> bufferedPositions;
+  private transient EqualityConvertPlanResult planResult;
+  private transient boolean hasUpstreamError;
+
+  public EqualityConvertDVResolver(
+      String tableName,
+      String taskName,
+      int taskIndex,
+      TableLoader tableLoader,
+      String targetBranch) {
+    this.tableName = tableName;
+    this.taskName = taskName;
+    this.taskIndex = taskIndex;
+    this.tableLoader = tableLoader;
+    this.targetBranch = targetBranch;
+  }
+
+  @Override
+  public void open() throws Exception {
+    super.open();
+    if (!tableLoader.isOpen()) {
+      tableLoader.open();
+    }
+
+    table = tableLoader.loadTable();
+    bufferedPositions = Lists.newArrayList();
+  }
+
+  @Override
+  public void processElement1(StreamRecord<DVPosition> record) {
+    if (record.getValue().isAbort()) {
+      hasUpstreamError = true;
+    } else {
+      bufferedPositions.add(record.getValue());
+    }
+  }
+
+  @Override
+  public void processElement2(StreamRecord<EqualityConvertPlanResult> record) {
+    planResult = record.getValue();
+  }
+
+  @Override
+  public void processWatermark(Watermark mark) throws Exception {
+    if (planResult != null && mark.getTimestamp() >= 
planResult.doneTimestamp()) {
+      if (hasUpstreamError) {
+        output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+      } else {
+        try {
+          resolveAndEmit();
+        } catch (Exception e) {
+          LOG.error(
+              "Error resolving DVs for table {} task {}[{}]", tableName, 
taskName, taskIndex, e);
+          output.collect(TaskResultAggregator.ERROR_STREAM, new 
StreamRecord<>(e));
+          output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+        }
+      }
+
+      bufferedPositions.clear();
+      hasUpstreamError = false;
+      planResult = null;
+    }
+
+    super.processWatermark(mark);
+  }
+
+  private void resolveAndEmit() {
+    if (bufferedPositions.isEmpty()) {
+      return;
+    }
+
+    table.refresh();
+
+    Snapshot mainSnapshot = table.snapshot(targetBranch);
+    Map<String, PartitionAndSpec> partitions = 
collectDataFilePartitions(mainSnapshot);
+    Map<String, DeleteFile> dvs = collectExistingDVs(mainSnapshot);
+
+    for (DataFile df : planResult.dataFiles()) {
+      partitions.put(
+          df.location(),
+          new PartitionAndSpec(
+              table.specs().get(df.specId()), 
StructLikeUtil.copy(df.partition())));
+    }
+
+    // Include staging DVs so the merger folds them in; prevents two DVs per 
data file (V3
+    // violation).
+    for (DeleteFile sd : planResult.stagingDVFiles()) {
+      if (ContentFileUtil.isDV(sd) && sd.referencedDataFile() != null) {
+        dvs.put(sd.referencedDataFile(), sd.copy());
+      }
+    }
+
+    Map<String, List<Long>> positionsByFile = Maps.newHashMap();

Review Comment:
   We now store the positions by file as we receive them.



##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertDVResolver.java:
##########
@@ -0,0 +1,249 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.flink.maintenance.operator;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.streaming.api.operators.AbstractStreamOperator;
+import org.apache.flink.streaming.api.operators.TwoInputStreamOperator;
+import org.apache.flink.streaming.api.watermark.Watermark;
+import org.apache.flink.streaming.runtime.streamrecord.StreamRecord;
+import org.apache.iceberg.DataFile;
+import org.apache.iceberg.DeleteFile;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.ManifestFiles;
+import org.apache.iceberg.ManifestReader;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Snapshot;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.flink.TableLoader;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.util.ContentFileUtil;
+import org.apache.iceberg.util.StructLikeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Collects {@link DVPosition}s from {@link EqualityConvertWorker} instances 
(input 1) and an {@link
+ * EqualityConvertPlanResult} from the planner (input 2). On watermark 
advancement, groups DVs, and
+ * emits {@link DVMergeCommand}s for parallel {@link EqualityConvertDVMerger} 
instances.
+ */
+@Internal
+public class EqualityConvertDVResolver extends 
AbstractStreamOperator<DVMergeCommand>
+    implements TwoInputStreamOperator<DVPosition, EqualityConvertPlanResult, 
DVMergeCommand> {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(EqualityConvertDVResolver.class);
+
+  private final String tableName;
+  private final String taskName;
+  private final int taskIndex;
+  private final TableLoader tableLoader;
+  private final String targetBranch;
+
+  private transient Table table;
+  private transient List<DVPosition> bufferedPositions;
+  private transient EqualityConvertPlanResult planResult;
+  private transient boolean hasUpstreamError;
+
+  public EqualityConvertDVResolver(
+      String tableName,
+      String taskName,
+      int taskIndex,
+      TableLoader tableLoader,
+      String targetBranch) {
+    this.tableName = tableName;
+    this.taskName = taskName;
+    this.taskIndex = taskIndex;
+    this.tableLoader = tableLoader;
+    this.targetBranch = targetBranch;
+  }
+
+  @Override
+  public void open() throws Exception {
+    super.open();
+    if (!tableLoader.isOpen()) {
+      tableLoader.open();
+    }
+
+    table = tableLoader.loadTable();
+    bufferedPositions = Lists.newArrayList();
+  }
+
+  @Override
+  public void processElement1(StreamRecord<DVPosition> record) {
+    if (record.getValue().isAbort()) {
+      hasUpstreamError = true;
+    } else {
+      bufferedPositions.add(record.getValue());
+    }
+  }
+
+  @Override
+  public void processElement2(StreamRecord<EqualityConvertPlanResult> record) {
+    planResult = record.getValue();
+  }
+
+  @Override
+  public void processWatermark(Watermark mark) throws Exception {
+    if (planResult != null && mark.getTimestamp() >= 
planResult.doneTimestamp()) {
+      if (hasUpstreamError) {
+        output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+      } else {
+        try {
+          resolveAndEmit();
+        } catch (Exception e) {
+          LOG.error(
+              "Error resolving DVs for table {} task {}[{}]", tableName, 
taskName, taskIndex, e);
+          output.collect(TaskResultAggregator.ERROR_STREAM, new 
StreamRecord<>(e));
+          output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+        }
+      }
+
+      bufferedPositions.clear();
+      hasUpstreamError = false;
+      planResult = null;
+    }
+
+    super.processWatermark(mark);
+  }
+
+  private void resolveAndEmit() {
+    if (bufferedPositions.isEmpty()) {
+      return;
+    }
+
+    table.refresh();
+
+    Snapshot mainSnapshot = table.snapshot(targetBranch);
+    Map<String, PartitionAndSpec> partitions = 
collectDataFilePartitions(mainSnapshot);
+    Map<String, DeleteFile> dvs = collectExistingDVs(mainSnapshot);
+
+    for (DataFile df : planResult.dataFiles()) {
+      partitions.put(
+          df.location(),
+          new PartitionAndSpec(
+              table.specs().get(df.specId()), 
StructLikeUtil.copy(df.partition())));
+    }
+
+    // Include staging DVs so the merger folds them in; prevents two DVs per 
data file (V3
+    // violation).
+    for (DeleteFile sd : planResult.stagingDVFiles()) {
+      if (ContentFileUtil.isDV(sd) && sd.referencedDataFile() != null) {
+        dvs.put(sd.referencedDataFile(), sd.copy());
+      }
+    }
+
+    Map<String, List<Long>> positionsByFile = Maps.newHashMap();
+    for (DVPosition pos : bufferedPositions) {
+      positionsByFile
+          .computeIfAbsent(pos.dataFilePath(), k -> Lists.newArrayList())
+          .add(pos.position());
+    }
+
+    for (Map.Entry<String, List<Long>> entry : positionsByFile.entrySet()) {
+      String dataFilePath = entry.getKey();
+      PartitionAndSpec partAndSpec =
+          partitions.getOrDefault(
+              dataFilePath, new 
PartitionAndSpec(PartitionSpec.unpartitioned(), null));
+      DeleteFile existingDV = dvs.get(dataFilePath);
+
+      output.collect(
+          new StreamRecord<>(
+              new DVMergeCommand(
+                  dataFilePath,
+                  entry.getValue(),
+                  partAndSpec.spec.specId(),
+                  partAndSpec.partition,
+                  existingDV != null ? existingDV.copy() : null)));
+    }
+
+    LOG.info(
+        "Emitted {} DV merge commands for table {} task {}[{}].",
+        positionsByFile.size(),
+        tableName,
+        taskName,
+        taskIndex);
+  }
+
+  @Override
+  public void close() throws Exception {
+    super.close();
+    tableLoader.close();
+  }
+
+  private Map<String, PartitionAndSpec> collectDataFilePartitions(Snapshot 
mainSnapshot) {

Review Comment:
   That doesn't work, we need the partitions for all data files here.



##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertDVResolver.java:
##########
@@ -0,0 +1,249 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.flink.maintenance.operator;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.streaming.api.operators.AbstractStreamOperator;
+import org.apache.flink.streaming.api.operators.TwoInputStreamOperator;
+import org.apache.flink.streaming.api.watermark.Watermark;
+import org.apache.flink.streaming.runtime.streamrecord.StreamRecord;
+import org.apache.iceberg.DataFile;
+import org.apache.iceberg.DeleteFile;
+import org.apache.iceberg.ManifestFile;
+import org.apache.iceberg.ManifestFiles;
+import org.apache.iceberg.ManifestReader;
+import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.Snapshot;
+import org.apache.iceberg.StructLike;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.flink.TableLoader;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.util.ContentFileUtil;
+import org.apache.iceberg.util.StructLikeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Collects {@link DVPosition}s from {@link EqualityConvertWorker} instances 
(input 1) and an {@link
+ * EqualityConvertPlanResult} from the planner (input 2). On watermark 
advancement, groups DVs, and
+ * emits {@link DVMergeCommand}s for parallel {@link EqualityConvertDVMerger} 
instances.
+ */
+@Internal
+public class EqualityConvertDVResolver extends 
AbstractStreamOperator<DVMergeCommand>
+    implements TwoInputStreamOperator<DVPosition, EqualityConvertPlanResult, 
DVMergeCommand> {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(EqualityConvertDVResolver.class);
+
+  private final String tableName;
+  private final String taskName;
+  private final int taskIndex;
+  private final TableLoader tableLoader;
+  private final String targetBranch;
+
+  private transient Table table;
+  private transient List<DVPosition> bufferedPositions;
+  private transient EqualityConvertPlanResult planResult;
+  private transient boolean hasUpstreamError;
+
+  public EqualityConvertDVResolver(
+      String tableName,
+      String taskName,
+      int taskIndex,
+      TableLoader tableLoader,
+      String targetBranch) {
+    this.tableName = tableName;
+    this.taskName = taskName;
+    this.taskIndex = taskIndex;
+    this.tableLoader = tableLoader;
+    this.targetBranch = targetBranch;
+  }
+
+  @Override
+  public void open() throws Exception {
+    super.open();
+    if (!tableLoader.isOpen()) {
+      tableLoader.open();
+    }
+
+    table = tableLoader.loadTable();
+    bufferedPositions = Lists.newArrayList();
+  }
+
+  @Override
+  public void processElement1(StreamRecord<DVPosition> record) {
+    if (record.getValue().isAbort()) {
+      hasUpstreamError = true;
+    } else {
+      bufferedPositions.add(record.getValue());
+    }
+  }
+
+  @Override
+  public void processElement2(StreamRecord<EqualityConvertPlanResult> record) {
+    planResult = record.getValue();
+  }
+
+  @Override
+  public void processWatermark(Watermark mark) throws Exception {
+    if (planResult != null && mark.getTimestamp() >= 
planResult.doneTimestamp()) {
+      if (hasUpstreamError) {
+        output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+      } else {
+        try {
+          resolveAndEmit();
+        } catch (Exception e) {
+          LOG.error(
+              "Error resolving DVs for table {} task {}[{}]", tableName, 
taskName, taskIndex, e);
+          output.collect(TaskResultAggregator.ERROR_STREAM, new 
StreamRecord<>(e));
+          output.collect(new StreamRecord<>(DVMergeCommand.abort()));
+        }
+      }
+
+      bufferedPositions.clear();
+      hasUpstreamError = false;
+      planResult = null;
+    }
+
+    super.processWatermark(mark);
+  }
+
+  private void resolveAndEmit() {
+    if (bufferedPositions.isEmpty()) {
+      return;
+    }
+
+    table.refresh();
+
+    Snapshot mainSnapshot = table.snapshot(targetBranch);
+    Map<String, PartitionAndSpec> partitions = 
collectDataFilePartitions(mainSnapshot);
+    Map<String, DeleteFile> dvs = collectExistingDVs(mainSnapshot);
+
+    for (DataFile df : planResult.dataFiles()) {
+      partitions.put(
+          df.location(),
+          new PartitionAndSpec(
+              table.specs().get(df.specId()), 
StructLikeUtil.copy(df.partition())));
+    }
+
+    // Include staging DVs so the merger folds them in; prevents two DVs per 
data file (V3
+    // violation).
+    for (DeleteFile sd : planResult.stagingDVFiles()) {
+      if (ContentFileUtil.isDV(sd) && sd.referencedDataFile() != null) {
+        dvs.put(sd.referencedDataFile(), sd.copy());
+      }
+    }
+
+    Map<String, List<Long>> positionsByFile = Maps.newHashMap();
+    for (DVPosition pos : bufferedPositions) {
+      positionsByFile
+          .computeIfAbsent(pos.dataFilePath(), k -> Lists.newArrayList())
+          .add(pos.position());
+    }
+
+    for (Map.Entry<String, List<Long>> entry : positionsByFile.entrySet()) {
+      String dataFilePath = entry.getKey();
+      PartitionAndSpec partAndSpec =
+          partitions.getOrDefault(
+              dataFilePath, new 
PartitionAndSpec(PartitionSpec.unpartitioned(), null));
+      DeleteFile existingDV = dvs.get(dataFilePath);
+
+      output.collect(
+          new StreamRecord<>(
+              new DVMergeCommand(
+                  dataFilePath,
+                  entry.getValue(),
+                  partAndSpec.spec.specId(),
+                  partAndSpec.partition,
+                  existingDV != null ? existingDV.copy() : null)));
+    }
+
+    LOG.info(
+        "Emitted {} DV merge commands for table {} task {}[{}].",
+        positionsByFile.size(),
+        tableName,
+        taskName,
+        taskIndex);
+  }
+
+  @Override
+  public void close() throws Exception {
+    super.close();
+    tableLoader.close();
+  }
+
+  private Map<String, PartitionAndSpec> collectDataFilePartitions(Snapshot 
mainSnapshot) {
+    Map<String, PartitionAndSpec> partitions = Maps.newHashMap();
+    if (mainSnapshot == null) {
+      return partitions;
+    }
+
+    for (ManifestFile manifest : mainSnapshot.dataManifests(table.io())) {
+      try (ManifestReader<DataFile> reader =
+          ManifestFiles.read(manifest, table.io(), table.specs())) {
+        for (DataFile file : reader) {
+          partitions.put(
+              file.location(),
+              new PartitionAndSpec(
+                  table.specs().get(file.specId()), 
StructLikeUtil.copy(file.partition())));
+        }
+      } catch (IOException e) {
+        throw new UncheckedIOException("Failed to read manifest: " + 
manifest.path(), e);
+      }
+    }
+
+    return partitions;
+  }
+
+  private Map<String, DeleteFile> collectExistingDVs(Snapshot mainSnapshot) {
+    Map<String, DeleteFile> dvs = Maps.newHashMap();
+    if (mainSnapshot == null) {
+      return dvs;
+    }
+
+    for (ManifestFile manifest : mainSnapshot.deleteManifests(table.io())) {

Review Comment:
   Same, we need to merge with all existing DVs.



##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertReader.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.flink.maintenance.operator;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.api.common.functions.OpenContext;
+import org.apache.flink.streaming.api.functions.ProcessFunction;
+import org.apache.flink.util.Collector;
+import org.apache.flink.util.OutputTag;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.data.Record;
+import org.apache.iceberg.flink.TableLoader;
+import org.apache.iceberg.formats.FormatModelRegistry;
+import org.apache.iceberg.formats.ReadBuilder;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.DeleteSchemaUtil;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.TypeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Parallel reader that processes {@link ReadCommand}s from the planner and 
emits {@link
+ * IndexCommand}s. Each instance reads a subset of files and emits row-level 
index commands for the
+ * workers.
+ */
+@Internal
+public class EqualityConvertReader extends ProcessFunction<ReadCommand, 
IndexCommand> {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(EqualityConvertReader.class);
+
+  public static final OutputTag<DVPosition> DV_POSITION_STREAM =
+      new OutputTag<>("dv-position-stream") {};
+
+  private final TableLoader tableLoader;
+
+  private transient Table table;
+  private transient EqualityFieldSerializer fieldSerializer;

Review Comment:
   This will now fail if the equality ids are not contained in the current 
schema. We should perhaps also support 



##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertReader.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.flink.maintenance.operator;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.api.common.functions.OpenContext;
+import org.apache.flink.streaming.api.functions.ProcessFunction;
+import org.apache.flink.util.Collector;
+import org.apache.flink.util.OutputTag;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.data.Record;
+import org.apache.iceberg.flink.TableLoader;
+import org.apache.iceberg.formats.FormatModelRegistry;
+import org.apache.iceberg.formats.ReadBuilder;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.DeleteSchemaUtil;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.TypeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Parallel reader that processes {@link ReadCommand}s from the planner and 
emits {@link
+ * IndexCommand}s. Each instance reads a subset of files and emits row-level 
index commands for the
+ * workers.
+ */
+@Internal
+public class EqualityConvertReader extends ProcessFunction<ReadCommand, 
IndexCommand> {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(EqualityConvertReader.class);
+
+  public static final OutputTag<DVPosition> DV_POSITION_STREAM =
+      new OutputTag<>("dv-position-stream") {};
+
+  private final TableLoader tableLoader;
+
+  private transient Table table;
+  private transient EqualityFieldSerializer fieldSerializer;
+
+  public EqualityConvertReader(TableLoader tableLoader) {
+    this.tableLoader = tableLoader;
+  }
+
+  @Override
+  public void open(OpenContext openContext) throws Exception {
+    super.open(openContext);
+    if (!tableLoader.isOpen()) {
+      tableLoader.open();
+    }
+
+    table = tableLoader.loadTable();
+    fieldSerializer = new EqualityFieldSerializer();
+  }
+
+  @Override
+  public void processElement(ReadCommand cmd, Context ctx, 
Collector<IndexCommand> out)
+      throws Exception {
+    try {
+      if (cmd.type() == ReadCommand.Type.POS_DELETE_FILE) {
+        readPosDeleteFile(cmd, ctx);
+        return;
+      }
+
+      Schema keySchema = TypeUtil.select(table.schema(), 
Sets.newHashSet(cmd.equalityFieldIds()));

Review Comment:
   Added a bit more logic to check if the keySchema exists and also allow 
retrieving it from the old table schemas.



##########
flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/maintenance/operator/EqualityConvertReader.java:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.flink.maintenance.operator;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.api.common.functions.OpenContext;
+import org.apache.flink.streaming.api.functions.ProcessFunction;
+import org.apache.flink.util.Collector;
+import org.apache.flink.util.OutputTag;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.Table;
+import org.apache.iceberg.data.Record;
+import org.apache.iceberg.flink.TableLoader;
+import org.apache.iceberg.formats.FormatModelRegistry;
+import org.apache.iceberg.formats.ReadBuilder;
+import org.apache.iceberg.io.CloseableIterable;
+import org.apache.iceberg.io.DeleteSchemaUtil;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.relocated.com.google.common.collect.Sets;
+import org.apache.iceberg.types.TypeUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Parallel reader that processes {@link ReadCommand}s from the planner and 
emits {@link
+ * IndexCommand}s. Each instance reads a subset of files and emits row-level 
index commands for the
+ * workers.
+ */
+@Internal
+public class EqualityConvertReader extends ProcessFunction<ReadCommand, 
IndexCommand> {
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(EqualityConvertReader.class);
+
+  public static final OutputTag<DVPosition> DV_POSITION_STREAM =
+      new OutputTag<>("dv-position-stream") {};
+
+  private final TableLoader tableLoader;
+
+  private transient Table table;
+  private transient EqualityFieldSerializer fieldSerializer;
+
+  public EqualityConvertReader(TableLoader tableLoader) {
+    this.tableLoader = tableLoader;
+  }
+
+  @Override
+  public void open(OpenContext openContext) throws Exception {
+    super.open(openContext);
+    if (!tableLoader.isOpen()) {
+      tableLoader.open();
+    }
+
+    table = tableLoader.loadTable();
+    fieldSerializer = new EqualityFieldSerializer();
+  }
+
+  @Override
+  public void processElement(ReadCommand cmd, Context ctx, 
Collector<IndexCommand> out)
+      throws Exception {
+    try {
+      if (cmd.type() == ReadCommand.Type.POS_DELETE_FILE) {
+        readPosDeleteFile(cmd, ctx);
+        return;
+      }
+
+      Schema keySchema = TypeUtil.select(table.schema(), 
Sets.newHashSet(cmd.equalityFieldIds()));
+      InputFile input = table.io().newInputFile(cmd.filePath());
+      ReadBuilder<Record, Schema> builder =
+          FormatModelRegistry.readBuilder(cmd.format(), Record.class, input);
+      try (CloseableIterable<Record> records = 
builder.project(keySchema).build()) {
+        long position = 0;
+        for (Record record : records) {
+          SerializedEqualityValues key = fieldSerializer.serializeKey(record, 
keySchema.asStruct());
+          switch (cmd.type()) {
+            case DATA_FILE:
+              out.collect(
+                  IndexCommand.addDataRow(cmd.mainSnapshotId(), key, 
cmd.filePath(), position));

Review Comment:
   Added.



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