Core: Basic fields and schemas for column files by gaborkaszab · Pull Request #16285 · apache/iceberg

gaborkaszab · 2026-05-11T13:52:03Z

This change introduces the interface for column files and also integrates it to the schema for TrackedFile.

gaborkaszab · 2026-05-11T16:02:14Z

First piece of the column update work: introducing the basic interface of the column updates files, aka column files
cc @anuragmantri @rdblue @pvary @RussellSpitzer @amogh-jahagirdar @anoopj @nastra

gaborkaszab · 2026-05-21T13:56:27Z

I opened a thread on dev@ to discuss the metadata structs for column files. Once that's finalized, I'll incorporate the changes here.

gaborkaszab · 2026-06-02T13:13:44Z

cc @amogh-jahagirdar @rdblue @anoopj

gaborkaszab · 2026-06-08T12:33:22Z

Adjusted field IDs because 157 is going to be allocated for writer_format_version in this PR.

gaborkaszab · 2026-06-12T14:39:56Z

+      this.status = EntryStatus.MODIFIED;
+    }
+    // Bumping 'dataSequenceNumber' to avoid having both equality deletes and column files.
+    this.dataSequenceNumber = null;


We discussed bumping the data sequence number when adding column files. We haven't mentioned file seq num, so I'm not bumping it here.
This works if the manifest owning this data file entry bumps its own seq num when adding column files. Let me know if there is any other way achieving this.

In a previous google doc discussion, @pvary raised the question if we should just bump up the dataSequenceNumber which captures the logical age of the row. Column file should materialize the _last_updated_sequence_number for unmodified rows and leave the modified rows with null value for inheritance. From row lineage perspective, bumping up dataSequenceNumber is correct and simpler semantically.

data sequence number is only used for v2 equality and position delete matching. it seems that we might be able to forbid writing new equality deletes for v4 tables. I also remember some previous discussion on rewriting equality delete and v2 position delete files when adding a new column file. With the writer requirement, it is safe to just bump up the dataSequenceNumber here.

But the comment line is a bit confusing. I would write as following: Reset to null to inherit from the new snapshot sequence number. It is safe to bump up the dataSequenceNumber as writers are required to rewrite v2 equality and position deletes to DVs when applying column update.

Thanks for the comment suggestion! Added

anuragmantri

I did another round after adding key_metdata and split_offsets. I think this is ready to be merged.

anuragmantri · 2026-08-07T18:13:03Z

+    if (status == EntryStatus.EXISTING) {
+      this.status = EntryStatus.MODIFIED;
+    }
+    // Bumping 'dataSequenceNumber' to avoid having both equality deletes and column files.


Should this comment be?

Suggested change

// Bumping 'dataSequenceNumber' to avoid having both equality deletes and column files.

// Clears dataSequenceNumber so it re-inherits from the manifest at read time.

Thanks for the suggestion! Steven also had one, I went with that.

anuragmantri · 2026-08-07T18:19:15Z

+    assertThat(withDeletedPositions.latestColumnFileSnapshotId()).isEqualTo(999L);
+    assertThat(withDeletedPositions.dvSnapshotId()).isEqualTo(999L);
+    assertThat(withDeletedPositions.deletedPositions()).isEqualTo(deletedBytes);
+


Should we verify the dataSequenceNumber is null?

Suggested change

assertThat(withDeletedPositions.dataSequenceNumber()).isNull();

Same on L314 and in manifestPositionsWithColumnFilesUpdated() test

stevenzwu · 2026-08-07T19:13:51Z

+      case 5 -> this.keyMetadata = ByteBuffers.toByteArray((ByteBuffer) value);
+      case 6 -> this.splitOffsets = ArrayUtil.toLongArray((List<Long>) value);
+      default -> {
+        // ignore the object, it must be from a newer version of the format


nit: should the comment say `ignore the unknown positions, as they must come from a newer version of the format"

This comment is inline with the same in TrackedFileStruct and TrackingStruct. I'd rather keep consistency with these.

that's fine for consistency. I found "ignore the object" not very accurate.

stevenzwu · 2026-08-07T19:54:18Z

+      this.status = EntryStatus.MODIFIED;
+    }
+    // Bumping 'dataSequenceNumber' to avoid having both equality deletes and column files.
+    this.dataSequenceNumber = null;


In a previous google doc discussion, @pvary raised the question if we should just bump up the dataSequenceNumber which captures the logical age of the row. Column file should materialize the _last_updated_sequence_number for unmodified rows and leave the modified rows with null value for inheritance. From row lineage perspective, bumping up dataSequenceNumber is correct and simpler semantically.

data sequence number is only used for v2 equality and position delete matching. it seems that we might be able to forbid writing new equality deletes for v4 tables. I also remember some previous discussion on rewriting equality delete and v2 position delete files when adding a new column file. With the writer requirement, it is safe to just bump up the dataSequenceNumber here.

But the comment line is a bit confusing. I would write as following: Reset to null to inherit from the new snapshot sequence number. It is safe to bump up the dataSequenceNumber as writers are required to rewrite v2 equality and position deletes to DVs when applying column update.

stevenzwu · 2026-08-07T20:15:59Z

+    Tracking withDeletedPositions =
+        TrackingBuilder.from(manifestSourceTracking(), 999L)
+            .columnFilesUpdated()
+            .deletedPositions(deletedBytes)


deletedPositions bitmap is only meant for leaf manifest entry in the root manifest file? Ae we testing the scenario of column update for a leaf manifest file in this test?

I don't think technically we want to avoid providing deleted/replaced positions together with column files. I just wanted to pin this down with a test.
Giving this some further thought, I think you're right: Such a Tracking that has these positions is an entry in the root manifest pointing to a leaf manifest. I don't think we plan to add column files for leaf manifest at this point, but it seems too strict to reject such a setting.

Could such a test remain? WDYT @stevenzwu ?

I don't think we plan to add column files for leaf manifest at this point

We will use column files for leaf manifests in v4. we should keep this test.

I was mainly alluding to if we should cover the column update for data file cases, where deletedPositions is not applicable.

In Tracking and TrackingBuilder we don't really know if it belongs to a data file entry or a manifest entry. There might be implications like presence of deleted positions or dv_snapshot_id but nothing decisive. We can add a separate test where we don't set deleted/replaced positions, but probably it doesn't add much to the coverage.

gaborkaszab

Thanks for the reviews @anuragmantri and @stevenzwu ! I believe I addressed all your comments. Would you mind taking another look?

gaborkaszab · 2026-08-08T11:06:34Z

+      case 5 -> this.keyMetadata = ByteBuffers.toByteArray((ByteBuffer) value);
+      case 6 -> this.splitOffsets = ArrayUtil.toLongArray((List<Long>) value);
+      default -> {
+        // ignore the object, it must be from a newer version of the format


This comment is inline with the same in TrackedFileStruct and TrackingStruct. I'd rather keep consistency with these.

gaborkaszab · 2026-08-08T11:12:22Z

+    if (status == EntryStatus.EXISTING) {
+      this.status = EntryStatus.MODIFIED;
+    }
+    // Bumping 'dataSequenceNumber' to avoid having both equality deletes and column files.


Thanks for the suggestion! Steven also had one, I went with that.

gaborkaszab · 2026-08-08T11:12:39Z

+      this.status = EntryStatus.MODIFIED;
+    }
+    // Bumping 'dataSequenceNumber' to avoid having both equality deletes and column files.
+    this.dataSequenceNumber = null;


Thanks for the comment suggestion! Added

gaborkaszab · 2026-08-08T11:54:32Z

+    assertThat(withDeletedPositions.latestColumnFileSnapshotId()).isEqualTo(999L);
+    assertThat(withDeletedPositions.dvSnapshotId()).isEqualTo(999L);
+    assertThat(withDeletedPositions.deletedPositions()).isEqualTo(deletedBytes);
+


gaborkaszab · 2026-08-08T11:57:55Z

+    Tracking withDeletedPositions =
+        TrackingBuilder.from(manifestSourceTracking(), 999L)
+            .columnFilesUpdated()
+            .deletedPositions(deletedBytes)


I don't think technically we want to avoid providing deleted/replaced positions together with column files. I just wanted to pin this down with a test.
Giving this some further thought, I think you're right: Such a Tracking that has these positions is an entry in the root manifest pointing to a leaf manifest. I don't think we plan to add column files for leaf manifest at this point, but it seems too strict to reject such a setting.

Could such a test remain? WDYT @stevenzwu ?

stevenzwu

LGTM overall

gaborkaszab · 2026-08-24T10:49:41Z

Thanks for the approval @stevenzwu and for the reviews @anuragmantri , @amogh-jahagirdar , @RussellSpitzer!
Are there anything else before we merge this? @rdblue Would you like to take a look yourself too?

Defines the column_file element struct referenced by the column_files field (158) in the v4 content entry, matching the ColumnFile schema added in apache#16285. Co-authored-by: Gabor Kaszab <gaborkaszab@gmail.com> Co-authored-by: Anurag Mantripragada <amantripragada@apple.com>

Makes the column_files field type list<159: column_file> to match the inline element-id convention used by other list fields in the content entry, matching apache#16285. Co-authored-by: Gabor Kaszab <gaborkaszab@gmail.com> Co-authored-by: Anurag Mantripragada <amantripragada@apple.com>

laskoviymishka

Nice work on this. The ColumnFile / ColumnFileStruct split fits the existing DeletionVector / Tracking pattern, and the schema wiring looks consistent with the rest of the file.

I’d still hold the merge on one thing: columnFilesUpdated() resets dataSequenceNumber, and the new inheritFrom() path can then assign a newer value to a MODIFIED entry.

That means attaching a column file can move the data sequence number forward even though the row data did not change. Since delete planning uses that sequence number, this could make existing deletes stop applying and bring deleted rows back.

The comment says this is safe because writers rewrite old deletes to DVs during a column update, but that is not enforced here yet. I think we should either keep the original sequence number for MODIFIED entries, or enforce that precondition here. A short spec note would also help.

A few smaller things:

no manifest round-trip test covers a populated column_files list
internalSet does not copy columnFiles, unlike the constructor and sibling fields
the copy constructor drops null elements instead of preserving the list shape
a couple of small builder-message / javadoc nits inline

The sequence-number behavior is the main one for me. Once that is settled, I’m happy to take another look.

laskoviymishka

LGTM! Nice work in general.

I think we can merge this to unblock further work on column-updates.

This change introduces the interface for column files and also integrates it to the schema for TrackedFile.

gaborkaszab · 2026-10-05T13:42:58Z

+        .toString();
+  }
+
+  static class Builder {


Since ColumnFile is something being passed in by the users through the table API, at some point we would want to make some builder public for them. I'd keep this here as long as it's possible and then I'd introduce something like a ColumnFiles class that can be used to produce the ColumnFile objects, similarly to DataFiles

gaborkaszab · 2026-10-05T13:50:17Z

Latest changes:
Rebased with latest main and resolved conflicts
Renames lates_column_file_snapshot_id to column_file_snapshot_id. Moved it right after dv_snapshot_id and changed the field ID to 8.
Removed split_offsets from ColumnFile

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

gaborkaszab commented May 11, 2026

View reviewed changes

Comment thread core/src/main/java/org/apache/iceberg/ColumnFileInfo.java Outdated

anuragmantri added this to V4: Efficient Column Updates May 11, 2026

anuragmantri moved this to Backlog in V4: Efficient Column Updates May 11, 2026

anuragmantri moved this from Backlog to In progress in V4: Efficient Column Updates May 11, 2026

gaborkaszab force-pushed the main_column_file_interface branch from 630b00e to ca3259e Compare May 11, 2026 17:04

anuragmantri reviewed May 11, 2026

View reviewed changes

gaborkaszab force-pushed the main_column_file_interface branch from ca3259e to e6f7cf6 Compare May 12, 2026 09:35

gaborkaszab commented May 13, 2026

View reviewed changes

Comment thread core/src/main/java/org/apache/iceberg/ColumnFileInfo.java Outdated

Comment thread core/src/main/java/org/apache/iceberg/ColumnFileInfo.java Outdated

gaborkaszab force-pushed the main_column_file_interface branch 3 times, most recently from 681633b to 813d5c0 Compare May 13, 2026 12:52

gaborkaszab force-pushed the main_column_file_interface branch 3 times, most recently from 596f6a4 to 6a1cbe9 Compare June 2, 2026 13:12

gaborkaszab changed the title ~~Core: Introduce interface for column files~~ Core: Basic fields and schemas for column files Jun 3, 2026

gaborkaszab force-pushed the main_column_file_interface branch from 6a1cbe9 to c683e72 Compare June 4, 2026 06:55

pvary moved this to In review in V4: metadata tree Jun 8, 2026

pvary added this to V4: metadata tree Jun 8, 2026

gaborkaszab force-pushed the main_column_file_interface branch from c683e72 to 6222fad Compare June 8, 2026 12:32

stevenzwu reviewed Jun 9, 2026

View reviewed changes

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

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

gaborkaszab force-pushed the main_column_file_interface branch from 6222fad to 5c04f55 Compare June 11, 2026 09:27

gaborkaszab commented Jun 11, 2026

View reviewed changes

Comment thread core/src/main/java/org/apache/iceberg/TrackedFileStruct.java Outdated

Comment thread core/src/main/java/org/apache/iceberg/TrackingBuilder.java Outdated

gaborkaszab force-pushed the main_column_file_interface branch from 5c04f55 to b6ae446 Compare June 12, 2026 14:37

gaborkaszab commented Jun 12, 2026

View reviewed changes

gaborkaszab force-pushed the main_column_file_interface branch from b6ae446 to 0a252e8 Compare June 12, 2026 15:09

gaborkaszab force-pushed the main_column_file_interface branch from c303670 to 5d96c17 Compare August 6, 2026 07:56

anuragmantri approved these changes Aug 7, 2026

View reviewed changes

stevenzwu reviewed Aug 7, 2026

View reviewed changes

gaborkaszab force-pushed the main_column_file_interface branch from 5d96c17 to 2bdf7bb Compare August 8, 2026 12:11

gaborkaszab commented Aug 8, 2026

View reviewed changes

gaborkaszab requested review from anuragmantri and stevenzwu August 8, 2026 12:14

gaborkaszab force-pushed the main_column_file_interface branch from 2bdf7bb to d0509a7 Compare August 10, 2026 20:09

stevenzwu approved these changes Aug 14, 2026

View reviewed changes

gaborkaszab force-pushed the main_column_file_interface branch 4 times, most recently from c26a609 to dd2433c Compare August 26, 2026 16:11

gaborkaszab force-pushed the main_column_file_interface branch from dd2433c to b0712b8 Compare September 7, 2026 11:11

gaborkaszab force-pushed the main_column_file_interface branch 2 times, most recently from abbdc88 to 0d37967 Compare September 16, 2026 16:00

laskoviymishka requested changes Sep 22, 2026

View reviewed changes

laskoviymishka approved these changes Sep 23, 2026

View reviewed changes

gaborkaszab added 2 commits October 5, 2026 15:29

Core: Basic fields and schemas for column files

47339f9

This change introduces the interface for column files and also integrates it to the schema for TrackedFile.

Addressing minor review comments

e95ed0f

gaborkaszab commented Oct 5, 2026

View reviewed changes

gaborkaszab added 4 commits October 6, 2026 22:27

Remove column_file_snapshot_id from Tracking

0268ab5

Rename dv_snapshot_id to modified_snapshot_id

da3efc7

Remove FORMAT_VERSION, make LOCATION first, reassign field IDs

12c9560

Allow setting key_metadata to null

81571b3

	// Bumping 'dataSequenceNumber' to avoid having both equality deletes and column files.
	// Clears dataSequenceNumber so it re-inherits from the manifest at read time.


	assertThat(withDeletedPositions.dataSequenceNumber()).isNull();

Conversation

gaborkaszab commented May 11, 2026

Uh oh!

Uh oh!

gaborkaszab commented May 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaborkaszab commented May 21, 2026

Uh oh!

gaborkaszab commented Jun 2, 2026

Uh oh!

gaborkaszab commented Jun 8, 2026

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!

anuragmantri 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!

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!

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!

gaborkaszab left a comment

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stevenzwu left a comment