zhengxiang378928908-code commented on PR #18361:
URL: https://github.com/apache/iceberg/pull/18361#issuecomment-6619330537
Thanks for the update. All four delete-file tests still pass with
`ManifestWriter` and `PartitionData` reverted to the merge base (`e58bb8aae`),
so they don't show that the delete-file normalization added in `b9aa4ab9b` is
needed. The other new tests are fine:
`identityPartitionAcceptsFileBuiltBeforeSourcePromotion`,
`mixedPromotionFilesCommitTogether`, `promotedFloatValueIsAccepted` and
`manifestAcceptsFileBuiltAfterSourcePromotion` all fail without the fix.
The pre-promotion delete files are all committed before `updateSchema()`,
and the one in `dataFileBeforePromotionDeleteFileAfter` is built with the
promoted spec, so the writer never gets an `Integer` value under the promoted
spec. For delete files, the regression test needs a delete built before the
promotion and committed after it. That's my mistake too: the second option I
suggested last time (commit the delete before promotion) has the same problem.
Sorry about that.
This test fails without the fix on v2, v3 and v4 (`Wrong class, expected
java.lang.Long, but was java.lang.Integer`) and passes with it. It needs an
`org.apache.iceberg.io.CloseableIterable` import:
```java
@TestTemplate
public void equalityDeleteBuiltBeforePromotionCommittedAfter() throws
IOException {
assumeThat(formatVersion).isGreaterThanOrEqualTo(2);
createIdentityPromotionTable();
PartitionSpec before = table.spec();
DataFile data = promotionFile(before, 5, "data.parquet");
DeleteFile deletes = promotionEqualityDeleteFile(before, 5,
"eq-deletes.parquet");
table.newAppend().appendFile(data).commit();
table.updateSchema().updateColumn("region", Types.LongType.get()).commit();
table.newRowDelta().addDeletes(deletes).commit();
try (CloseableIterable<FileScanTask> planned =
table.newScan().filter(Expressions.equal("region", 5L)).planFiles()) {
List<FileScanTask> tasks = Lists.newArrayList(planned);
assertThat(tasks).hasSize(1);
assertThat(tasks.get(0).deletes()).hasSize(1);
}
}
```
Position deletes work the same way: a position-delete file on v2, or a DV
referencing the data file on v3/v4, built before promotion and committed with
`newRowDelta().addDeletes(...)` after it, with the data file committed before
promotion. That also fails without the fix on all three versions.
Small nit: `build.gradle` still has an unrelated indentation change on the
closing brace of the `iceberg-gcp` block.
--
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]
