feat: add support for `_pos` metadata column by hsiang-c · Pull Request #2746 · apache/iceberg-rust

hsiang-c · 2026-06-29T23:22:15Z

Which issue does this PR close?

Partially closes Support for projecting metadata columns _pos, _spec_id, and _partition in table scan #2607

What changes are included in this PR?

Support _pos metadata column by delegating it to the Parquet reader's RowNumber virtual column, inspired by feat: Plumb Parquet virtual columns (row_number) through TableSchema and ParquetOpener datafusion#22026.
Refactor the compare_schemas function b/c _pos field id exists in both source_schema and target_schema and the original implementation might result in SchemaComparison::NameChangesOnly, which produces a RecordBatch with correct content but incorrect column order.

Are these changes tested?

Yes, unit tests.
Iceberg Spark: feat: [iceberg] support iceberg metadata columns _file, _partition datafusion-comet#4752

Sample plan

== Physical Plan ==
AdaptiveSparkPlan isFinalPlan=false
+- CometSort [id#863L, data#864, _pos#868L], [id#863L ASC NULLS FIRST]
   +- CometExchange rangepartitioning(id#863L ASC NULLS FIRST, 200), ENSURE_REQUIREMENTS, CometNativeShuffle, [plan_id=2634]
      +- CometFilter [id#863L, data#864, _pos#868L], (id#863L >= 10)
         +- CometIcebergNativeScan [id#863L, data#864, _pos#868L], file:/var/folders/d2/b93h6k7174ddqxltrxgb51040000gn/T/hive2622216829356851627/table/metadata/00004-95932e2b-bbe6-444e-81b8-069c5e748a20.metadata.json, spark_catalog.default.table (branch=null) [filters=id IS NOT NULL, id >= 10, groupedBy=], 1

mbutrovich

I like this approach a lot. Delegating to the parquet reader's RowNumber virtual column is essentially what Java does with SupportsRowPosition / PositionReader, and it gets correct absolute file positions through row-group pruning and row selection without us hand-rolling a counter. The virtual_fields plus PassThrough wiring is clean. My only real ask is around tests that exercise _pos where it's actually load-bearing. The current tests read contiguous rows, which would pass even if selection/deletes mis-numbered positions:

_pos with row selection that genuinely prunes rows;
_pos with a delete file applied;
_pos on a split task (start/length not equal to the whole file);
_pos interleaved with other metadata columns.

Curious whether any of these already pass as-is. If so, even better, and they'd lock in the behavior.

mbutrovich · 2026-06-30T18:27:25Z

+            .table
+            .scan()
+            .select(["x", RESERVED_COL_NAME_POS])
+            .with_row_selection_enabled(true)


With no filter, all 1024 rows are read contiguously, so this passes whether or not _pos survives selection correctly. The interesting case, and the reason to use the reader's row-number column rather than a counter, is a predicate that prunes row groups/rows, where _pos must still report absolute file positions (for example non-contiguous like [0,1,2, 500,501,...]). Could we add that? It's the test that proves the design.

mbutrovich · 2026-06-30T18:27:38Z

        assert!(result.is_ok(), "Scan timed out - deadlock detected");
    }
+
+    #[tokio::test]


The spec defines _pos as the ordinal position in the source data file (https://iceberg.apache.org/spec/#reserved-field-ids). Two behaviors I'd love a test for, next to the ones you added here: (a) applying a delete filter doesn't renumber _pos; (b) a task scanning only [start, start+length) still emits absolute positions, matching Java's findStartingRowPos. Both look reachable with the existing TableTestFixture. Do these hold today?

mbutrovich · 2026-06-30T18:28:01Z


+        let project_pos = task.project_field_ids().contains(&RESERVED_FIELD_ID_POS);
+
+        let arrow_metadata = if project_pos {


There are now up to three sequential ArrowReaderMetadata::try_new rebuilds per file (field-ID schema, INT96 coercion, virtual columns), each re-deriving from parquet metadata. Could the final ArrowReaderOptions (schema plus virtual columns) be assembled once and try_new called a single time? Minor and not blocking; it just compounds with the two existing passes.

+1, the code is getting harder to follow. I have reviewed this part code several times in the past two weeks but still need to refresh my memory every time I see it :)

I think we can create a tracking issue and have AI check if there is any opportunity to refactor it

mbutrovich · 2026-06-30T18:29:30Z

            arrow_metadata
        };

+        let project_pos = task.project_field_ids().contains(&RESERVED_FIELD_ID_POS);


Reusing RowNumber instead of a manual counter is the right, idiomatic call. It inherits the parquet reader's correctness for selection and row-group boundaries and matches Java's SupportsRowPosition semantics. Nice.

mbutrovich

LGTM. Delegating _pos to the Parquet reader's RowNumber virtual column is the right call: it inherits the reader's correctness through row-group pruning and row selection, and matches Java's SupportsRowPosition / PositionReader semantics rather than hand-rolling a counter.

The tests requested in the prior round have all landed and prove the design point — test_pos_across_row_groups_via_table_scan, test_pos_reads_only_middle_row_group_via_filter, and test_pos_with_positional_deletes_via_table_scan confirm absolute file positions survive pruning and are not renumbered by deletes. Production footprint is small (~107 lines); the rest is tests.

No blocking comments. The downstream Comet PR (#4752), which exercises this against Iceberg-Java-written data, is green including the Iceberg 1.11 Spark SQL suite.

Merge order

Suggest #2746 → #2695 → #2668. #2746 is independent (virtual-column path; doesn't touch the constant-map logic the other two rewrite), so it can go first cleanly.

hsiang-c · 2026-07-10T20:00:35Z

Thank you for your suggestions and review @mbutrovich

hsiang-c · 2026-07-16T17:31:46Z

@CTTY if you could review it when you have time, thanks again for your help.

CTTY

LGTM! just some minor comments

CTTY · 2026-07-17T22:34:46Z

Since we added some good behavior changes/fixes to this function, can we update the comment here to say that we now explicitly handle reordered schema and mark them as different?

Also I think it would nice to have a test for this function

I'll add another test for it.

Added a test for reordered schema (it will lead to SchemaComparison::NameChangesOnly, which is incorrect) and updated comment accordingly.

CTTY · 2026-07-17T22:57:30Z


+        let project_pos = task.project_field_ids().contains(&RESERVED_FIELD_ID_POS);
+
+        let arrow_metadata = if project_pos {


+1, the code is getting harder to follow. I have reviewed this part code several times in the past two weeks but still need to refresh my memory every time I see it :)

I think we can create a tracking issue and have AI check if there is any opportunity to refactor it

hsiang-c · 2026-07-20T21:04:25Z

@CTTY I created a tracking issue and will refactor the process function later.

CTTY

LGTM, thanks for the contribution!

Handle virtual column: '_pos'

c738f04

mbutrovich self-requested a review June 30, 2026 17:46

This was referenced Jun 30, 2026

feat: add support for _partition metadata column #2668

Open

feat: add support for _spec_id metadata column #2695

Merged

mbutrovich reviewed Jun 30, 2026

View reviewed changes

hsiang-c and others added 4 commits July 6, 2026 16:23

Merge branch 'main' into metadata_col_pos

91cb67e

Remove redundant to_string()

54d05c7

Better comment

56e07d7

Handle reordered fields in the target schema

52bdcce

hsiang-c force-pushed the metadata_col_pos branch from bfa0ae3 to e16e307 Compare July 8, 2026 16:53

More test cases: row-group filtering, positional delete

3450222

hsiang-c force-pushed the metadata_col_pos branch from e16e307 to 3450222 Compare July 8, 2026 20:34

hsiang-c marked this pull request as ready for review July 8, 2026 20:41

mbutrovich self-requested a review July 10, 2026 17:06

mbutrovich approved these changes Jul 10, 2026

View reviewed changes

hsiang-c and others added 2 commits July 16, 2026 09:57

Merge branch 'main' into metadata_col_pos

09c2098

Merge branch 'main' into metadata_col_pos

cfa534d

CTTY reviewed Jul 17, 2026

View reviewed changes

hsiang-c and others added 3 commits July 20, 2026 09:46

Merge branch 'main' into metadata_col_pos

c5558be

Fix Clippy

dca93f0

Update comment and add a corresponding test

66c793d

hsiang-c mentioned this pull request Jul 20, 2026

Refactor ArrowReaderMetadata building in the process function of FileScanTaskReader #2860

Open

CTTY approved these changes Jul 20, 2026

View reviewed changes

CTTY merged commit 8b76dbe into apache:main Jul 20, 2026
21 checks passed

hsiang-c deleted the metadata_col_pos branch July 20, 2026 22:25

wjixiang mentioned this pull request Jul 21, 2026

Data column named pos (or file_path) is misread as a reserved metadata column during scan -> "field not found" #2837

Closed

LuciferYang mentioned this pull request Jul 22, 2026

fix(scan): don't treat data columns named pos or file_path as metadata columns #2865

Merged

anoopj mentioned this pull request Jul 22, 2026

Support reading _row_id and _last_updated_sequence_number metadata columns #2879

Open


		let project_pos = task.project_field_ids().contains(&RESERVED_FIELD_ID_POS);

		let arrow_metadata = if project_pos {

Uh oh!

Conversation

hsiang-c commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbutrovich left a comment

Choose a reason for hiding this comment

Merge order

Uh oh!

hsiang-c commented Jul 10, 2026

Uh oh!

hsiang-c commented Jul 16, 2026

Uh oh!

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

hsiang-c Jul 20, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

hsiang-c commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CTTY left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hsiang-c commented Jun 29, 2026 •

edited

Loading

hsiang-c Jul 20, 2026 •

edited

Loading

hsiang-c commented Jul 20, 2026 •

edited

Loading