dramaticlly commented on code in PR #16964:
URL: https://github.com/apache/iceberg/pull/16964#discussion_r3493443393
##########
core/src/main/java/org/apache/iceberg/TrackedFileBuilder.java:
##########
@@ -238,21 +231,29 @@ TrackedFileBuilder deletionVector(DeletionVector
newDeletionVector) {
Preconditions.checkArgument(newDeletionVector != null, "Invalid deletion
vector: null");
Preconditions.checkArgument(
contentType == FileContent.DATA,
- "Deletion vector can only be added to DATA entries, but entry type is:
%s",
+ "Cannot add deletion vector for file with content: %s",
contentType);
- Preconditions.checkArgument(
- this.deletionVector == null ||
!this.deletionVector.equals(newDeletionVector),
- "The same deletion vector already added");
+ if (deletionVector != null) {
+ Preconditions.checkArgument(
+ deletionVector.cardinality() < newDeletionVector.cardinality(),
+ "Invalid DV update, cardinality must increase: existing=%s, new=%s",
+ deletionVector == null ? null : deletionVector.cardinality(),
Review Comment:
is deletionVector nullable here? I think line 236 ensure only the nonnull
will lands here? So we can use `deletionVector.cardinality()` directly
--
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]
