Core: Introduce builder for TrackedFile by gaborkaszab · Pull Request #16769 · apache/iceberg

gaborkaszab · 2026-06-11T08:50:32Z

No description provided.

stevenzwu · 2026-06-16T21:26:20Z

+    this.manifestInfo = manifestInfo;
+    this.keyMetadata = keyMetadata != null ? ByteBuffers.toByteArray(keyMetadata) : null;
+    this.splitOffsets = splitOffsets != null ? ArrayUtil.toLongArray(splitOffsets) : null;
+    this.equalityIds = equalityIds != null ? ArrayUtil.toIntArray(equalityIds) : null;


Follow-up, not a blocker: We are doing back and forth conversion btw array and list. I know this follows existing pattern like BaseFile. I am wondering if it makes sense to just switch to arrays for these to splitOffsets and equalityIds.

I can open a separate PR for this where we can continue the discussion. WDYT?

yes, need a bit broader discussion on that topic.

For the record: I did some investigation on this. Apparently, based on the schema the ValueReaders expect Lists for these types and not arrays so the internal get and set functions have to return List objects.

gaborkaszab

Thanks for taking a look @stevenzwu !
I addressed all the comments. I have a follow-up question for you firstRowId, though.

Since MODIFIED is merged, let me rebase and eliminate the relevant TODOs

gaborkaszab · 2026-06-17T08:32:15Z

+    this.manifestInfo = manifestInfo;
+    this.keyMetadata = keyMetadata != null ? ByteBuffers.toByteArray(keyMetadata) : null;
+    this.splitOffsets = splitOffsets != null ? ArrayUtil.toLongArray(splitOffsets) : null;
+    this.equalityIds = equalityIds != null ? ArrayUtil.toIntArray(equalityIds) : null;


I can open a separate PR for this where we can continue the discussion. WDYT?

gaborkaszab · 2026-06-17T12:06:58Z

Rebased with main and resolved the following:

TrackedFileAdapters to use the builder instead of the constructor
TestTrackedFileBuilder to adjust to MODIFIED state where necessary

gaborkaszab · 2026-06-17T12:08:27Z

    assertThat(deleteFile.format()).isEqualTo(FileFormat.AVRO);
    assertThat(deleteFile.recordCount()).isEqualTo(50L);
    assertThat(deleteFile.fileSizeInBytes()).isEqualTo(512L);
-    assertThat(deleteFile.sortOrderId()).isEqualTo(5);


EQ-deletes don't allow sort order and splitOffset through the builder. Removed the checks before no longer makes sense.

After our conversation, I restored setting these fields for eq-deletes too

stevenzwu · 2026-06-17T16:53:36Z

+    this.manifestInfo = manifestInfo;
+    this.keyMetadata = keyMetadata != null ? ByteBuffers.toByteArray(keyMetadata) : null;
+    this.splitOffsets = splitOffsets != null ? ArrayUtil.toLongArray(splitOffsets) : null;
+    this.equalityIds = equalityIds != null ? ArrayUtil.toIntArray(equalityIds) : null;


yes, need a bit broader discussion on that topic.

stevenzwu · 2026-06-17T17:13:51Z

  }

+  @Override
+  public boolean equals(Object other) {


StructLike classes in Iceberg often don't implement the equals method. but in this case, I am ok with it for usage in the TrackedFileBuilder

I think that we should remove this. There are multiple ways of considering something "equal" and we try to avoid equals methods when it is unclear which one should be used. That's the case here, where it would be reasonable to assume that two DeletionVector instances are the same if they represent the same DV (location and offset are equal) but also reasonable to assume two instances are equal if they have the same values (mostly implemented here).

In addition to the problem of what this means, there's another problem that this definition only works for other DeletionVectorStruct instances. That means the definition of equality is more murky: two DeletionVector instances can be equal by either definition above but still may not be equal here. The purpose of using interfaces for the public API is to make it so that implementations are interchangeable and this breaks that principle.

I think this was introduced to be able to detect changes applied to the builder, but we should find a better way to solve that problem rather than introducing this.

@rdblue the comparison was introduced due to my comment here. Do you think it is sth we should guard against? if not, we can maybe just add Javadoc to explain the usage. if yes, should we just do a reference comparison like this.deletionVector != newDeletionVector or we want a util method like `DelectionVectorUtil.sameContents(DeletionVector, DeletionVector)?

I dropped these overrides here, and went for the verification suggested by Ryan within the builder class.

gaborkaszab

Thanks again @stevenzwu for looking into this. I address your comments. For the new restrictions of eq-deletes I invited @amogh-jahagirdar to share some more context.

writer_format_version landed in the meantime. Let me rebase and get rid of the relevant TODOs

stevenzwu

LGTM except for one open question in the comment thread about equality files.

amogh-jahagirdar

I need a bit more time for an in-depth review on this but I did comment on the equality delete discussion, thanks for raising that @gaborkaszab @stevenzwu ! I'll update the spec PR , I think it was a miss to be that restrictive for split offsets and sort order ID, technically they can apply for equality deletes as well

amogh-jahagirdar · 2026-06-21T23:22:45Z

+  TrackedFileBuilder deletedPositions(ByteBuffer newDeletedPositions) {
+    Preconditions.checkArgument(newDeletedPositions != null, "Invalid deleted positions: null");
+    Preconditions.checkArgument(
+        contentType == FileContent.DATA_MANIFEST || contentType == FileContent.DELETE_MANIFEST,


we may want to add a isLeafManifest() helper to this class?

Added a helper function for that, thx! This is called from static context too so added a FileContent parameter to the function.

amogh-jahagirdar · 2026-06-21T23:23:46Z

+        source.contentType() != FileContent.DATA_MANIFEST
+            && source.contentType() != FileContent.DELETE_MANIFEST,


Mentioned below, but with the amount of times we do this kind of check, I think we'd benefit from a isLeafManifest helper

amogh-jahagirdar · 2026-06-21T23:29:05Z

Also cc @anoopj in case he's interested in taking a look at this .

anoopj · 2026-06-22T16:31:37Z

+  }
+
+  /**
+   * Creates a builder for a tracked file derived from {@code newSource}.


newSource reference is stale?

Thanks for spotting. Fixed

anoopj · 2026-06-22T16:33:03Z

+  TrackedFileBuilder sortOrderId(int newSortOrderId) {
+    Preconditions.checkArgument(
+        !isLeafManifest(contentType),
+        "Sort order ID can not be added to manifest entries, but entry type is: %s",


Suggested change

"Sort order ID can not be added to manifest entries, but entry type is: %s",

"Sort order ID cannot be added to manifest entries, but entry type is: %s",

anoopj · 2026-06-22T16:34:03Z

+    Preconditions.checkArgument(newSplitOffsets != null, "Invalid split offsets: null");
+    Preconditions.checkArgument(
+        !isLeafManifest(contentType),
+        "Split offsets can not be added to manifest entries, but entry type is: %s",


Suggested change

"Split offsets can not be added to manifest entries, but entry type is: %s",

"Split offsets cannot be added to manifest entries, but entry type is: %s",

anoopj · 2026-06-22T16:38:58Z

+    this.sortOrderId = sortOrderId;
+    this.deletionVector = deletionVector;
+    this.manifestInfo = manifestInfo;
+    this.keyMetadata = keyMetadata != null ? ByteBuffers.toByteArray(keyMetadata) : null;


ByteBuffers.toByteArray() and ArrayUtil.to*Array() are null safe. So it's ok to drop the ternary. e.g. just do:

this.keyMetadata = ByteBuffers.toByteArray(keyMetadata);

Thanks, fixed!

anoopj · 2026-06-22T16:55:47Z

+  }
+
+  /**
+   * This method is for signalling that a new deletion vector is added. It is forbidden to add the


You don't need to document the invariant in the comments. I would recommend just removing the javadoc for this method. It's the only method with a javadoc anyway.

The suggestion for the doc comment came from @stevenzwu here. I think this function is easy to read and interpret as is now, we can drop the comment. Let me know if you think otherwise @stevenzwu !

stevenzwu · 2026-06-22T17:48:11Z

+        fileSizeInBytes != null, "Missing required field: file size in bytes");
+    Preconditions.checkArgument(partitionData != null, "Missing required field: partition data");
+    Preconditions.checkArgument(
+        (contentType != FileContent.DATA_MANIFEST && contentType != FileContent.DELETE_MANIFEST)


nit: this check escaped the isLeafManifest consolidation — could be !isLeafManifest(contentType) || manifestInfo != null.

Indeed, thanks for spotting! Fixed

stevenzwu · 2026-06-22T22:18:25Z

I merged this PR to unblock other PRs.

We can follow up separately if there are more comments.

gaborkaszab · 2026-06-23T06:39:41Z

Thank you for the review @stevenzwu @anoopj @amogh-jahagirdar !
Let me know if you find anything that needs some adjustments! I'll take care of that

rdblue · 2026-06-23T22:50:25Z

  }

+  @Test
+  void testDvEquality() {


Style: Iceberg keeps most acronyms upper case, so this should be DV instead of Dv.

Thx! Dropped the test, because also dropped the equals override

rdblue · 2026-06-23T23:06:40Z

+  private final FileContent contentType;
+
+  // Required fields
+  private Integer writerFormatVersion = null;


Would this ever be null? I'm starting from the assumption that in v4 we will only write v4 metadata files. If that's true, then I think there are two cases when building a TrackedFile instance: either we are writing a v4 file or we are adapting an existing v1-3 file to be tracked in v4. The first case is easy: always set this to 4.

Does the second case go through this builder? I would expect to use a ManifestFile to TrackedFile adapter, like the adapters @anoopj introduced for TrackedFile to DataFile, DeleteFile, etc. That is a good pattern to follow because we never want a caller that is moving tracked files around to have the logic for migrating from v3 to v4.

yes, I have a follow-up PR (draft status now) to adapt v3 shapes (like ManifestFile and DataFile) to v4 TrackedFile. It would be good to use builder for those adaptions too, unless you are thinking about using the constructor directly.

Regarding the null or not, current proposal/spec PR said it is not null (0 for pre-v4 and format versions for v4+)

Thinking about this more, would we use adapters to wrap DataFile and DeleteFile anyway? Those are what our writers produce already and I doubt that we will want to go through and update all the plumbing to produce TrackedFile directly. The utility of this class may be very limited.

I could see us producing instances of TrackedFile by reading and wrapping v3 DataFile instances, or wrapping a new DataFile instance for v4. We should verify there are cases where we would use this.

Apparently, as Steven noted, we could use the TrackedFileBuilder for the wrappers around V3 metadata structs, and there we could use this writer_format_version field through the builder. See the draft PR linked by Steven and let me know if it makes sense.

I believe we concluded on (writer_)format_version related question on the other dedicated PR. Let me know if there is still something outstanding here!
Checking again Steven's PR on the wrappers, I think it makes sense to set this through the builder.

rdblue · 2026-06-23T23:29:33Z

+  private FileFormat fileFormat = null;
+  private Long recordCount = null;
+  private Long fileSizeInBytes = null;
+  private PartitionData partitionData = null;


PartitionData should be StructLike. We don't want to rely on concrete classes here.

Also, this is in the "required fields" section, but I think that we want to allow this to be null for unpartitioned cases. The partition tuple will be null when specId is null or an unpartitioned spec, because the actual partition type that we store is the union schema of all valid partition specs in the table. And support for empty struct types and instances is unreliable.

rdblue · 2026-06-23T23:35:33Z

+        source.writerFormatVersion(),
+        source.location(),
+        source.fileFormat(),
+        (PartitionData) source.partition(),


We should not need unchecked casts like this.

The difficulty here is the TrackedFile.partition() return StructLike while the TrackedFileStruct constructor accepts PartitionData. Since internally in TrackedFile we also have PartitionData this seemed safe

rdblue · 2026-06-23T23:41:01Z

+    Preconditions.checkArgument(source != null, "Invalid source: null");
+    Preconditions.checkArgument(
+        !isLeafManifest(source.contentType()),
+        "Manifest entries cannot transition to REPLACED, but entry type is: %s",


This is less direct than we typically prefer, but is not bad. This states a general rule and then says that it applies. We would normally say that at once: "Cannot transition %s to REPLACED".

Thx! I think we can even remove the content type parameter of the message:
Cannot transition manifest files to REPLACED

rdblue · 2026-06-23T23:45:01Z

+  TrackedFileBuilder sortOrderId(int newSortOrderId) {
+    Preconditions.checkArgument(
+        !isLeafManifest(contentType),
+        "Sort order ID cannot be added to manifest entries, but entry type is: %s",


"Cannot set sort order for manifest files"?

rdblue · 2026-06-23T23:46:21Z

+    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"

Fixed, and found couple other similar messages, fixed them too.

rdblue · 2026-06-23T23:47:11Z

+        "Deletion vector can only be added to DATA entries, but entry type is: %s",
+        contentType);
+    Preconditions.checkArgument(
+        this.deletionVector == null || !this.deletionVector.equals(newDeletionVector),


Style: this. is used when assigning to a field, but not when accessing a field.

rdblue · 2026-06-23T23:55:37Z

+        contentType);
+    Preconditions.checkArgument(
+        this.deletionVector == null || !this.deletionVector.equals(newDeletionVector),
+        "The same deletion vector already added");


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:

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");

Thanks, borrowed these checks!

rdblue · 2026-06-24T00:00:38Z

+    return this;
+  }
+
+  TrackedFileBuilder deletedPositions(ByteBuffer newDeletedPositions) {


I'm not against this since I haven't seen how it is used yet. But I find these methods that capture data that will be passed through to Tracking suspicious. Why not pass in a new Tracking and not mirror its builder's API here? Also, if this is the pattern we want, then why not wrap a TrackingBuilder and pass this directly?

Felt a bit odd for me when I implemented this but went this way for two reasons:

I tried to avoid allow passing Tracking in the TrackedFileBuilder for the users because similar reasons we don't allow passing status for TrackingBuilder: seems to represent an internal state and users might get it wrong to construct. We could guard against malformed Tracking objects with checks, but seemed that we already have everything to construct it internally and not push one more responsibility to the users of the builder.

TrackingBuilder itself can't check for all of the constraints. For instance it doesn't know if it belongs to a data file or a delete manifest, hence can't verify that deleted/replaced positions are only allowed for manifests. We can perform such a check here, in the TrackedFileBuilder.

Introduced a TrackingBuilder member to the TrackedFileBuilder and we can delegate all these calls right away without keeping track the required information.

rdblue · 2026-06-24T00:00:50Z

+    Preconditions.checkArgument(recordCount != null, "Missing required field: record count");
+    Preconditions.checkArgument(
+        fileSizeInBytes != null, "Missing required field: file size in bytes");
+    Preconditions.checkArgument(partitionData != null, "Missing required field: partition data");


I don't think this is correct.

Thx!
I change partition not to be required through the builder

rdblue · 2026-06-24T00:02:15Z

+    return this;
+  }
+
+  TrackedFileBuilder partition(PartitionData newPartitionData) {


I think that this should be combined with specId: partition(int newSpecId, StructLike partitionTuple). If the tuple is set, then spec ID is required so that we know how to interpret it. I suspect that this should also be responsible for wrapping the partition tuple with a projection to the union type.

I probably miss something here, but I see that TrackedFile has a PartitionData member. If we want to produce that, is specId and partitionTuple enough? Don't we need a PartitionSpec to produce PartitionData?

rdblue · 2026-06-24T00:03:54Z

-    file.set(SORT_ORDER_ID_ORDINAL, 3);
-    file.set(KEY_METADATA_ORDINAL, ByteBuffer.wrap(new byte[] {1, 2, 3}));
-    file.set(SPLIT_OFFSETS_ORDINAL, ImmutableList.of(50L, 100L));
+    TrackedFile file =


What is the use of going through the builder for tests? I think that this should simply use the constructor and pass all values. There's an argument for using this to make the test cases more clear (this is certainly readable) but it also doesn't allow us to construct some instances that we may want to test because they violate rules that the builder enforces.

I wanted to construct these objects in a "proper" way, however for these tests it doesn't really matter. Went back to using the constructor instead.
Also revived the createTracking() function that we had earlier, including reviving 2 Tracking ordinals, because using the TrackedFileStruct contractor makes those changes independent from this PR.

rdblue · 2026-06-24T00:04:50Z

      TrackedFile.schemaWithContentStats(Types.StructType.of(), Types.StructType.of());
+  private static final int CONTENT_TYPE_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "content_type");
  private static final int SPEC_ID_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "spec_id");
-  private static final int CONTENT_STATS_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "content_stats");


Why doesn't this fix the use of SPEC_ID_ORDINAL if it is updating these tests?

2 tests still used it. Got rid of those usages too.
Also got rid of the DELETION_VECTOR_ORDINAL below

rdblue · 2026-06-24T00:05:36Z

  // TrackedFile optional field ordinals, looked up from the schema.
  private static final Types.StructType TRACKED_FILE_SCHEMA =
      TrackedFile.schemaWithContentStats(Types.StructType.of(), Types.StructType.of());
+  private static final int CONTENT_TYPE_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "content_type");


Why does this introduce another ordinal? I think these are bad practice (these tests should not be using set/get) so I'm surprised to see that this eliminates some, but introduces others.

See my answer below.
Removed

rdblue · 2026-06-24T00:07:04Z

-        NO_PARTITION,
-        1L,
-        1L);
+  private static TrackedFileStruct dummyTrackedFile(FileContent contentType) {


Please avoid unnecessary renames. This should be reverted.

I wanted to warn future readers/editor of this file that the TrackedFile returned here might have broken internal invariants because of going through the constructor. (it does, it has no Tracking for instance)
Reverted the rename

rdblue · 2026-06-24T00:08:38Z

-        1L);
+  private static TrackedFileStruct dummyTrackedFile(FileContent contentType) {
+    TrackedFileStruct file = new TrackedFileStruct();
+    file.set(CONTENT_TYPE_ORDINAL, contentType.id());


This introduces a bad practice, using the ordinals and the set methods. I'm surprised to see this change in the addition of builders. Instead, just update the usage of the constructor.

I wanted to find the easiest way to construct TrackedFile with the desired content type, where none go the other members matter.
Got rid of this new ordinal and changed to use the constructor.

gaborkaszab

Thanks for taking a look, @rdblue !
I published the follow-up PR for this: #16964
Note, I'm not entirely sure about the partition StructLike / PartitionData part, but we can continue there.

gaborkaszab · 2026-06-25T07:57:40Z

-    file.set(SORT_ORDER_ID_ORDINAL, 3);
-    file.set(KEY_METADATA_ORDINAL, ByteBuffer.wrap(new byte[] {1, 2, 3}));
-    file.set(SPLIT_OFFSETS_ORDINAL, ImmutableList.of(50L, 100L));
+    TrackedFile file =


I wanted to construct these objects in a "proper" way, however for these tests it doesn't really matter. Went back to using the constructor instead.
Also revived the createTracking() function that we had earlier, including reviving 2 Tracking ordinals, because using the TrackedFileStruct contractor makes those changes independent from this PR.

gaborkaszab · 2026-06-25T07:58:48Z

-        1L);
+  private static TrackedFileStruct dummyTrackedFile(FileContent contentType) {
+    TrackedFileStruct file = new TrackedFileStruct();
+    file.set(CONTENT_TYPE_ORDINAL, contentType.id());


I wanted to find the easiest way to construct TrackedFile with the desired content type, where none go the other members matter.
Got rid of this new ordinal and changed to use the constructor.

gaborkaszab · 2026-06-25T08:00:04Z

-        NO_PARTITION,
-        1L,
-        1L);
+  private static TrackedFileStruct dummyTrackedFile(FileContent contentType) {


I wanted to warn future readers/editor of this file that the TrackedFile returned here might have broken internal invariants because of going through the constructor. (it does, it has no Tracking for instance)
Reverted the rename

gaborkaszab · 2026-06-25T08:03:33Z

      TrackedFile.schemaWithContentStats(Types.StructType.of(), Types.StructType.of());
+  private static final int CONTENT_TYPE_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "content_type");
  private static final int SPEC_ID_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "spec_id");
-  private static final int CONTENT_STATS_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "content_stats");


2 tests still used it. Got rid of those usages too.
Also got rid of the DELETION_VECTOR_ORDINAL below

gaborkaszab · 2026-06-25T08:04:57Z

  // TrackedFile optional field ordinals, looked up from the schema.
  private static final Types.StructType TRACKED_FILE_SCHEMA =
      TrackedFile.schemaWithContentStats(Types.StructType.of(), Types.StructType.of());
+  private static final int CONTENT_TYPE_ORDINAL = ordinalOf(TRACKED_FILE_SCHEMA, "content_type");


See my answer below.
Removed

gaborkaszab · 2026-06-25T09:10:21Z

+    Preconditions.checkArgument(source != null, "Invalid source: null");
+    Preconditions.checkArgument(
+        !isLeafManifest(source.contentType()),
+        "Manifest entries cannot transition to REPLACED, but entry type is: %s",


Thx! I think we can even remove the content type parameter of the message:
Cannot transition manifest files to REPLACED

gaborkaszab · 2026-06-25T09:12:14Z

+  private final FileContent contentType;
+
+  // Required fields
+  private Integer writerFormatVersion = null;


I believe we concluded on (writer_)format_version related question on the other dedicated PR. Let me know if there is still something outstanding here!
Checking again Steven's PR on the wrappers, I think it makes sense to set this through the builder.

gaborkaszab · 2026-06-25T09:15:09Z

  }

+  @Override
+  public boolean equals(Object other) {


I dropped these overrides here, and went for the verification suggested by Ryan within the builder class.

gaborkaszab · 2026-06-25T19:01:19Z

+    return this;
+  }
+
+  TrackedFileBuilder partition(PartitionData newPartitionData) {


I probably miss something here, but I see that TrackedFile has a PartitionData member. If we want to produce that, is specId and partitionTuple enough? Don't we need a PartitionSpec to produce PartitionData?

gaborkaszab · 2026-06-25T19:04:32Z

+        source.writerFormatVersion(),
+        source.location(),
+        source.fileFormat(),
+        (PartitionData) source.partition(),


The difficulty here is the TrackedFile.partition() return StructLike while the TrackedFileStruct constructor accepts PartitionData. Since internally in TrackedFile we also have PartitionData this seemed safe

github-actions Bot added the core label Jun 11, 2026

pvary reviewed Jun 11, 2026

View reviewed changes

Comment thread core/src/test/java/org/apache/iceberg/TestTrackedFileStruct.java

stevenzwu reviewed Jun 17, 2026

View reviewed changes

gaborkaszab force-pushed the main_trackedfile_builder branch from f9c86c3 to 2ad4d31 Compare June 17, 2026 10:04

gaborkaszab commented Jun 17, 2026

View reviewed changes

gaborkaszab force-pushed the main_trackedfile_builder branch from 2ad4d31 to 2319f92 Compare June 17, 2026 12:05

gaborkaszab commented Jun 17, 2026

View reviewed changes

gaborkaszab force-pushed the main_trackedfile_builder branch from 2319f92 to bf22506 Compare June 17, 2026 12:10

gaborkaszab commented Jun 17, 2026

View reviewed changes

Comment thread core/src/main/java/org/apache/iceberg/TrackedFileBuilder.java

gaborkaszab force-pushed the main_trackedfile_builder branch 2 times, most recently from 6060c35 to 082a487 Compare June 17, 2026 12:37

gaborkaszab requested a review from stevenzwu June 17, 2026 13:19

stevenzwu added this to V4: metadata tree Jun 17, 2026

stevenzwu reviewed Jun 17, 2026

View reviewed changes

stevenzwu moved this to In review in V4: metadata tree Jun 17, 2026

stevenzwu reviewed Jun 17, 2026

View reviewed changes

gaborkaszab force-pushed the main_trackedfile_builder branch from 082a487 to 072a4e9 Compare June 18, 2026 12:29

gaborkaszab commented Jun 18, 2026

View reviewed changes

gaborkaszab force-pushed the main_trackedfile_builder branch 3 times, most recently from 0360884 to 707b01c Compare June 18, 2026 14:13

gaborkaszab requested a review from stevenzwu June 18, 2026 14:18

stevenzwu approved these changes Jun 18, 2026

View reviewed changes

stevenzwu mentioned this pull request Jun 19, 2026

Core: Add ManifestFile adapter for v4 tracked files #16867

Open

gaborkaszab mentioned this pull request Jun 19, 2026

Core: Basic fields and schemas for column files #16285

Open

amogh-jahagirdar requested a review from rdblue June 21, 2026 23:20

amogh-jahagirdar reviewed Jun 21, 2026

View reviewed changes

gaborkaszab force-pushed the main_trackedfile_builder branch from 707b01c to bf3dfe7 Compare June 22, 2026 12:06

gaborkaszab requested a review from stevenzwu June 22, 2026 12:14

anoopj approved these changes Jun 22, 2026

View reviewed changes

stevenzwu approved these changes Jun 22, 2026

View reviewed changes

gaborkaszab force-pushed the main_trackedfile_builder branch from bf3dfe7 to 05fec44 Compare June 22, 2026 19:00

gaborkaszab requested a review from stevenzwu June 22, 2026 19:00

Core: Introduce builder for TrackedFile

aa0ec94

gaborkaszab force-pushed the main_trackedfile_builder branch from 05fec44 to aa0ec94 Compare June 22, 2026 21:29

stevenzwu merged commit 7c13104 into apache:main Jun 22, 2026
54 checks passed

github-project-automation Bot moved this from In review to Done in V4: metadata tree Jun 22, 2026

rdblue reviewed Jun 23, 2026

View reviewed changes

rdblue reviewed Jun 24, 2026

View reviewed changes

gaborkaszab mentioned this pull request Jun 25, 2026

Core: Adjustments for TrackedFileBuilder #16964

Open

gaborkaszab commented Jun 25, 2026

View reviewed changes

		source.contentType() != FileContent.DATA_MANIFEST
		&& source.contentType() != FileContent.DELETE_MANIFEST,

	"Sort order ID can not be added to manifest entries, but entry type is: %s",
	"Sort order ID cannot be added to manifest entries, but entry type is: %s",

	"Split offsets can not be added to manifest entries, but entry type is: %s",
	"Split offsets cannot be added to manifest entries, but entry type is: %s",

Uh oh!

Conversation

gaborkaszab commented Jun 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaborkaszab left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaborkaszab commented Jun 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaborkaszab left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevenzwu left a comment

Choose a reason for hiding this comment

Uh oh!

amogh-jahagirdar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amogh-jahagirdar commented Jun 21, 2026

gaborkaszab left a comment •

edited

Loading