I know we usually check if result is null to see if the writer was closed and we are not re-assigning the result object. It probably works in this case (knowing the underlying implementation), but I wonder whether making this check explicit will make the behavior of this method clear even without checking how the delegate writer works.

if (result == null) {
  fileWriter.close();
  this.result = fileWriter.result();
}

Aren't DVs considered file-scoped? I think isFileScoped will always be true for DVs (please check).

Actually, if DVs are enabled, we have to include all position deletes, not just file-scoped. The first ever produced DV must include all previous deletes. The only difference is that we can drop file-scoped from the table state (decided in the writer) and still have to keep partition-scoped deletes as they may apply to other data files.

Can we please test migration to DVs for tables that have a mix of file-scoped and partition-scoped position deletes?

Great catch, yes to both points. the file scoped Util correctly handles the DV case by checking the referenced data file, and we for DVs we do need to merge all existing position deletes, regardless of scope. I've fixed that and added a test case for update/delete/merge which will create a mix of file scoped and partition scoped deletes, and then finally produce a DV which has the positions merged from these existing deletes.

What about equality deletes? Those should be ignored. If so, what about shouldRewrite method?

for (DeleteFile deleteFile : fileScanTask.deletes()) {
  if (shouldRewrite(deleteFile, forDVs)) {
    ...
  }
}

private boolean shouldRewrite(DeleteFile deleteFile, boolean dvsEnabled) {
  ...
}

It would be nice to also cover this with tests.

Ah I fixed to avoid broadcasting eq. deletes unnecessarily but need to figure out how to add a test which would inspect the broadcast contents, will a publish a separate PR for that