stevenzwu commented on code in PR #16936:
URL: https://github.com/apache/iceberg/pull/16936#discussion_r3599240558
##########
core/src/main/java/org/apache/iceberg/TrackedFileAdapters.java:
##########
@@ -425,4 +482,980 @@ private static PartitionSpec resolveSpec(
throw new IllegalArgumentException(
"Cannot find unpartitioned spec in specs: " + specsById.keySet());
}
+
+ // Content_entry on-disk schema field count and per-position ordering,
mirroring
+ // TrackedFileStruct.BASE_TYPE and TrackedFileStruct.getByPos so the
wrapper's StructLike view
+ // matches what the Parquet writer expects when it iterates fields by
ordinal.
+ private static final int CONTENT_ENTRY_FIELD_COUNT = 16;
+
+ /** Shared base for content-file (DATA / EQUALITY_DELETES) write-direction
wrappers. */
+ abstract static class ContentTrackedFile<F extends ContentFile<F>>
+ implements TrackedFile, StructLike {
+ private final int formatVersion;
+ private final Schema tableSchema;
+ private final Map<Integer, Type> primitiveTypesById;
+ private final Types.StructType partitionType;
+ private final WrappedTracking tracking = new WrappedTracking();
+ private F file;
+ private StructProjection partition;
+ private ContentStats stats;
+
+ ContentTrackedFile(int formatVersion, Schema tableSchema, Types.StructType
partitionType) {
+ Preconditions.checkArgument(
+ formatVersion >=
TableMetadata.MIN_FORMAT_VERSION_ADAPTIVE_MANIFEST_TREE,
+ "Invalid format version for adaptive manifest tree: %s (must be >=
%s)",
+ formatVersion,
+ TableMetadata.MIN_FORMAT_VERSION_ADAPTIVE_MANIFEST_TREE);
+ Preconditions.checkArgument(tableSchema != null, "Invalid table schema:
null");
+ Preconditions.checkArgument(partitionType != null, "Invalid partition
type: null");
+ this.formatVersion = formatVersion;
+ this.tableSchema = tableSchema;
+ this.primitiveTypesById = primitiveTypesFor(tableSchema);
+ this.partitionType = partitionType;
+ }
+
+ final void wrapWithTracking(
+ F newFile, EntryStatus status, Long snapshotId, Long dataSeq, Long
fileSeq) {
+ wrapWithTracking(newFile, status, snapshotId, dataSeq, fileSeq, null);
+ }
+
+ final void wrapWithTracking(
Review Comment:
Direct answer to the question: yes, the caller decides the status — that is
unavoidable and already how the `ManifestWriter` API works today
(`add`/`existing`/`delete` encode it).
The confusing part is that this PR's adapter re-encodes that decision as six
`wrap*` methods, layered on top of `TrackingBuilder` which also encodes it.
Proposal: drop the per-status methods and have the adapter take a pre-built
`Tracking`:
```java
DataTrackedFile wrap(DataFile file, Tracking tracking)
```
`TrackingBuilder` stays the single place that builds the `Tracking`
(added/from/deleted/replaced, `dv_snapshot_id`, deleted/replaced positions);
the adapter only forwards file-derived fields (partition, stats, manifest_info,
DV). The Phase 2 v4 writer builds the `Tracking` via `TrackingBuilder` or
`TrackingStruct` constructor and hands it to the adapter. Does that match what
you had in mind?
--
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]
