rdblue commented on code in PR #16769: URL: https://github.com/apache/iceberg/pull/16769#discussion_r3463659478
########## core/src/main/java/org/apache/iceberg/TrackedFileBuilder.java: ########## @@ -0,0 +1,364 @@ +/* + * 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; + +import java.nio.ByteBuffer; +import java.util.List; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; + +class TrackedFileBuilder { + private final long snapshotId; + private final FileContent contentType; + + // Required fields + private Integer writerFormatVersion = null; + private String location = null; + private FileFormat fileFormat = null; + private Long recordCount = null; + private Long fileSizeInBytes = null; + private PartitionData partitionData = null; + + // optional fields + private Integer specId = null; + private ContentStats contentStats = null; + private Integer sortOrderId = null; + private DeletionVector deletionVector = null; + private ManifestInfo manifestInfo = null; + private ByteBuffer keyMetadata = null; + private List<Long> splitOffsets = null; + private List<Integer> equalityIds = null; + + // tracking-related fields + private Tracking sourceTracking = null; + private boolean dvUpdated = false; + private ByteBuffer deletedPositions = null; + private ByteBuffer replacedPositions = null; + + /** + * Creates a builder for a newly added data file entry. + * + * @param newSnapshotId the snapshot ID in which the new tracked file will be committed + */ + static TrackedFileBuilder data(long newSnapshotId) { + return new TrackedFileBuilder(FileContent.DATA, newSnapshotId); + } + + /** + * Creates a builder for a newly added equality delete file entry. + * + * @param newSnapshotId the snapshot ID in which the new tracked file will be committed + */ + static TrackedFileBuilder equalityDelete(long newSnapshotId) { + return new TrackedFileBuilder(FileContent.EQUALITY_DELETES, newSnapshotId); + } + + /** + * Creates a builder for a newly added data manifest entry. + * + * @param newSnapshotId the snapshot ID in which the new tracked file will be committed + */ + static TrackedFileBuilder dataManifest(long newSnapshotId) { + return new TrackedFileBuilder(FileContent.DATA_MANIFEST, newSnapshotId); + } + + /** + * Creates a builder for a newly added delete manifest entry. + * + * @param newSnapshotId the snapshot ID in which the new tracked file will be committed + */ + static TrackedFileBuilder deleteManifest(long newSnapshotId) { + return new TrackedFileBuilder(FileContent.DELETE_MANIFEST, newSnapshotId); + } + + /** + * Creates a builder for a tracked file derived from {@code source}. + * + * @param source source tracked file to copy fields from + * @param newSnapshotId the snapshot ID in which the new tracked file will be committed + */ + static TrackedFileBuilder from(TrackedFile source, long newSnapshotId) { + Preconditions.checkArgument(source != null, "Invalid source: null"); + return new TrackedFileBuilder(source, newSnapshotId); + } + + /** + * Returns a DELETED tracked file derived from {@code source}. + * + * @param source source tracked file + * @param newSnapshotId the snapshot ID in which the new tracked file will be committed + */ + static TrackedFile deleted(TrackedFile source, long newSnapshotId) { + Preconditions.checkArgument(source != null, "Invalid source: null"); + return terminal(source, TrackingBuilder.deleted(source.tracking(), newSnapshotId)); + } + + /** + * Returns a REPLACED tracked file derived from {@code source}. + * + * <p>Manifest entries cannot transition to REPLACED. + * + * @param source source tracked file + * @param newSnapshotId the snapshot ID in which the new tracked file will be committed + */ + static TrackedFile replaced(TrackedFile source, long newSnapshotId) { + Preconditions.checkArgument(source != null, "Invalid source: null"); + Preconditions.checkArgument( + !isLeafManifest(source.contentType()), + "Manifest entries cannot transition to REPLACED, but entry type is: %s", + source.contentType()); + return terminal(source, TrackingBuilder.replaced(source.tracking(), newSnapshotId)); + } + + private static TrackedFile terminal(TrackedFile source, Tracking tracking) { + return new TrackedFileStruct( + tracking, + source.contentType(), + source.writerFormatVersion(), + source.location(), + source.fileFormat(), + (PartitionData) source.partition(), + source.recordCount(), + source.fileSizeInBytes(), + source.specId(), + source.contentStats(), + source.sortOrderId(), + source.deletionVector(), + source.manifestInfo(), + source.keyMetadata(), + source.splitOffsets(), + source.equalityIds()); + } + + private TrackedFileBuilder(FileContent contentType, long snapshotId) { + this.contentType = contentType; + this.snapshotId = snapshotId; + } + + private TrackedFileBuilder(TrackedFile source, long snapshotId) { + this.contentType = source.contentType(); + this.snapshotId = snapshotId; + this.writerFormatVersion = source.writerFormatVersion(); + this.location = source.location(); + this.fileFormat = source.fileFormat(); + this.recordCount = source.recordCount(); + this.fileSizeInBytes = source.fileSizeInBytes(); + this.partitionData = (PartitionData) source.partition(); + this.specId = source.specId(); + this.contentStats = source.contentStats(); + this.sortOrderId = source.sortOrderId(); + this.deletionVector = source.deletionVector(); + this.manifestInfo = source.manifestInfo(); + this.keyMetadata = source.keyMetadata(); + this.splitOffsets = source.splitOffsets(); + this.equalityIds = source.equalityIds(); + this.sourceTracking = source.tracking(); + } + + TrackedFileBuilder writerFormatVersion(int newWriterFormatVersion) { + Preconditions.checkArgument( + newWriterFormatVersion >= 0, + "Invalid writer format version: %s (must be >= 0)", + newWriterFormatVersion); + this.writerFormatVersion = newWriterFormatVersion; + return this; + } + + TrackedFileBuilder location(String newLocation) { + Preconditions.checkArgument(newLocation != null, "Invalid location: null"); + this.location = newLocation; + return this; + } + + TrackedFileBuilder fileFormat(FileFormat newFileFormat) { + Preconditions.checkArgument(newFileFormat != null, "Invalid file format: null"); + this.fileFormat = newFileFormat; + return this; + } + + TrackedFileBuilder recordCount(long newRecordCount) { + Preconditions.checkArgument( + newRecordCount >= 0, "Invalid record count: %s (must be >= 0)", newRecordCount); + this.recordCount = newRecordCount; + return this; + } + + TrackedFileBuilder fileSizeInBytes(long newFileSizeInBytes) { + Preconditions.checkArgument( + newFileSizeInBytes >= 0, + "Invalid file size in bytes: %s (must be >= 0)", + newFileSizeInBytes); + this.fileSizeInBytes = newFileSizeInBytes; + return this; + } + + TrackedFileBuilder specId(int newSpecId) { + Preconditions.checkArgument(newSpecId >= 0, "Invalid spec ID: %s (must be >= 0)", newSpecId); + this.specId = newSpecId; + return this; + } + + TrackedFileBuilder partition(PartitionData newPartitionData) { + Preconditions.checkArgument(newPartitionData != null, "Invalid partition: null"); + this.partitionData = newPartitionData; + return this; + } + + TrackedFileBuilder contentStats(ContentStats newContentStats) { + Preconditions.checkArgument(newContentStats != null, "Invalid content stats: null"); + this.contentStats = newContentStats; + return this; + } + + TrackedFileBuilder sortOrderId(int newSortOrderId) { + Preconditions.checkArgument( + !isLeafManifest(contentType), + "Sort order ID cannot be added to manifest entries, but entry type is: %s", + contentType); + Preconditions.checkArgument( + newSortOrderId >= 0, "Invalid sort order ID: %s (must be >= 0)", newSortOrderId); + this.sortOrderId = newSortOrderId; + return this; + } + + 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", + contentType); + Preconditions.checkArgument( + this.deletionVector == null || !this.deletionVector.equals(newDeletionVector), + "The same deletion vector already added"); Review Comment: The check that I would expect is `deletionVector == null || newDeletionVector.cardinality() > deletionVector.cardinality()`. That ensures that the deletion vector was updated, without relying on an equality check. Otherwise, bad data could still make it through just by using a different concrete class (which is not equal). The cardinality check is closer to what we want although not foolproof. The underlying location and offset could be the same, although it is unlikely that would be the case. If you want to make sure that this is actually a different deletion vector, then I think the right thing to do is to do that check here: ```java Preconditions.checkArgument( deletionVector == null || deletionVector.cardinality() < newDeletionVector.cardinality(), "Invalid DV update, cardinality must increase: existing=%s, new=%s", ...); Preconditions.checkArgument( deletionVector == null || !deletionVector.location().equals(newDeletionVector.location()) || !deletionVector.offset().equals(newDeletionVector.offset()), "Invalid DV update: same offset and location has changed cardinality"); ``` -- 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]
