feat: add support for _partition metadata column by parthchandra · Pull Request #2668 · apache/iceberg-rust

parthchandra · 2026-06-18T00:34:03Z

Which issue does this PR close?

part of #Support for projecting metadata columns _pos, _spec_id, and _partition in table scan #2607.

What changes are included in this PR?

Implements the _partition metadata column for table scans. This is a struct column whose type is the union of all partition fields across all partition specs (handling partition evolution). Each row gets the
partition values for its data file.

Adds compute_unified_partition_type() to compute the union of partition fields across all specs (equivalent to Java's Partitioning.partitionType())
Adds PartitionColumnConstant and build_partition_column_constant() for pre-computing the struct values per file
Adds ColumnSource::AddStructConstant variant to RecordBatchTransformer for materializing struct columns
Threads the unified partition type through scan planning and populates the constant in FileScanTask
Pipeline detects RESERVED_FIELD_ID_PARTITION in projected fields and injects the struct constant

Are these changes tested?

Because we do not have write support yet, I made the corresponding change to comet and then tested by adding tests in Comet which uses iceberg-java to write files and then iceberg-rust to read them back.
https://github.com/parthchandra/datafusion-comet/blob/iceberg-metadata-columns/spark/src/test/resources/sql-tests/iceberg/metadata_column_partition.sql

parthchandra · 2026-06-18T00:36:27Z

@advancedxy fyi

advancedxy · 2026-06-18T07:39:47Z

+/// # Arguments
+/// * `partition_specs` - Iterator over all partition specs in the table
+/// * `schema` - The current table schema (needed to determine result types of transforms)
+pub fn compute_unified_partition_type<'a>(


nit: it's a bit of odd that this function is added in metadata_column.rs. Do you think it's a good idea to create partitioning.rs and put this function into partitioning.rs?

Created crates/iceberg/src/partitioning.rs, moved the function there, registered as pub mod partitioning in lib.rs

advancedxy · 2026-06-21T11:56:28Z

+    let mut seen_field_ids = std::collections::HashSet::new();
+    let mut struct_fields: Vec<NestedFieldRef> = Vec::new();
+
+    for spec in partition_specs {


I notice some inconsistent with java's impl:

unknown specs are rejected in java.

specs are sorted by spec id first(in reverse order), which means newer partition spec's field name will be picked fist.

V1 table's void transform field is also handled in java: the partition field that was dropped later.

Good point!
Updated the implementation (partitioning.rs) with -

Sort specs by spec_id descending (newer field names take precedence)

Skips Transform::Void fields (dropped partition columns)

Deduplicates by field_id

advancedxy · 2026-06-21T12:02:35Z

+        let partition_column_constant =
+            if let Some(ref unified_partition_type) = self.unified_partition_type {
+                let partition_spec = self
+                    .table_metadata
+                    .partition_spec_by_id(self.partition_spec_id);
+                if let Some(spec) = partition_spec {
+                    let constant = build_partition_column_constant(
+                        unified_partition_type,
+                        spec,
+                        &self.manifest_entry.data_file.partition,
+                    )?;
+                    Some(Arc::new(constant))
+                } else {
+                    None
+                }
+            } else {
+                None
+            };


I'm not sure this is a good idea to calculate the partition column in the plan/scan phase and it adds build_partition_column_constant dep from record_batch_transformer' mod into the scan side.

Fair point. Removed build_partition_column_constant from scan/context.rs. The scan phase only passes unified_partition_type through to the task. The actual struct constant is computed lazily at read time in pipeline.rs.

advancedxy · 2026-06-21T12:12:58Z

+    #[serde(serialize_with = "serialize_not_implemented")]
+    #[serde(deserialize_with = "deserialize_not_implemented")]
+    #[builder(default)]
+    pub partition_column_constant: Option<Arc<PartitionColumnConstant>>,


Like the comment in https://github.com/apache/iceberg-rust/pull/2668/changes#r3448373215, I think it's better to pass the unified partition type here rather than passing the actual unified partition value. It would be easier for comet to pooling the type rather than the actual value.

BTW, this would be unnecessary if we can rebuild/access the table/metadata when reading on the executor side. Java archives this by SerializableTable and with Spark's broadcast. We don't have similar thing on the rust yet.

FileScanTask now carries unified_partition_type: Option<Arc<StructType>> instead of partition_column_constant: Option<Arc<PartitionColumnConstant>>. The reader computes the value from type + spec + partition data.
Also removed the table_metadata field from ManifestEntryContext and ManifestFileContext (it was only used for the old precomputation path)

advancedxy · 2026-06-21T12:14:35Z

+                let mut ids: Vec<i32> = value.identifier_field_ids.into_iter().collect();
+                ids.sort_unstable();
+                Some(ids)
            },


the changes in this file seem unrelated? And I don't think the spec requires sorting identifier field ids.

You're right, this got left over by accident. Removed.

advancedxy · 2026-06-21T12:23:27Z

@parthchandra thanks for pinging me and working on this. I think I'm concerned that the unified partition value is carried in the file scan task, which seems a bit of odd.

advancedxy · 2026-06-23T06:51:58Z

@parthchandra I did another round and left some follow-up comments. I think it's in good shape overall.

mbutrovich · 2026-06-23T18:22:07Z

Comet generates the FileScanTasks from Iceberg Java. Does Iceberg Java provide unified_partition_type, otherwise we need to be sure Comet can generate these if you want Comet to work with this.

parthchandra · 2026-06-23T20:44:35Z

Comet generates the FileScanTasks from Iceberg Java. Does Iceberg Java provide unified_partition_type, otherwise we need to be sure Comet can generate these if you want Comet to work with this.

Good point! Iceberg Java doesn't provide unified_partition_type on individual FileScanTasks — Java computes it at a higher level via Partitioning.partitionType(table)

We can do the equivalent in Comet - https://github.com/parthchandra/datafusion-comet/blob/iceberg-metadata-columns/native/core/src/execution/planner.rs#L3409-L3440

advancedxy · 2026-06-25T06:28:01Z

+                {
+                    let struct_type = DataType::Struct(pc.fields.clone());
+                    let nullable = pc.fields.is_empty();
+                    let arrow_field = Field::new("_partition", struct_type, nullable)


nit: should use RESERVED_COL_NAME_PARTITION in metadata_columns directly.

advancedxy · 2026-06-25T06:31:23Z

+            }
+
+            // Skip void transforms (dropped partition columns) and unknown transforms
+            if matches!(field.transform, Transform::Void | Transform::Unknown) {


hmm. I don't think this is consistent with iceberg-java.

Unknown transform should be checked rather than ignored.

Considering a new transform introduced in iceberg-spec but not adapted by iceberg-rust yet should be considered as unknown transform.

mbutrovich · 2026-06-25T12:41:25Z

We can do the equivalent in Comet - https://github.com/parthchandra/datafusion-comet/blob/iceberg-metadata-columns/native/core/src/execution/planner.rs#L3409-L3440

Are you able to demonstrate this feature working in Comet? Usually when I bring an enhancement to iceberg-rust motivated by Comet usage, I create a draft PR on the Comet repo with tests and the feature built out (using the PR branch here) to demonstrate it works with the Iceberg Java suites.

parthchandra · 2026-06-26T14:42:07Z

Thanks @advancedxy @mbutrovich I'm on the road atm, will address these in a few days.

mbutrovich

Thanks for this @parthchandra! Computing the unified type once per scan and threading it through the context is a nice shape, and matching Java's "empty partition struct becomes null" choice is a good call for engine compatibility. A few things I'd like to talk through before it lands, mostly around matching Partitioning.partitionType exactly and trimming some surface area:

There looks to be an ordering dependency on #2695 (the partition_spec wiring). Could you confirm? See the context.rs note.
Two spots where I think we may diverge from Partitioning.partitionType (dropped source column; result field ordering), flagged as questions inline.
Some of the array construction could lean on arrow-rs helpers, plus a couple of API-surface trims.
Coverage is identity-transform-only right now; I listed the cases I'd find most reassuring.

Re: tests:

The two new unit tests are identity-only. Cases that would most increase my confidence:

non-identity transform (bucket/truncate), confirming the transformed partition value lands in _partition, consistent with PartitionUtil.constantsMap reading partitionData.get(pos);
unpartitioned table end-to-end (exercises the empty-struct-to-null branch);
explicit null partition value (distinct from evolution-missing);
_partition selected alongside _file/_pos/_spec_id in one scan, since they share the projection/transform path.

mbutrovich · 2026-06-30T17:51:10Z

+
+            seen_field_ids.insert(field.field_id);
+
+            let source_field = schema.field_by_id(field.source_id).ok_or_else(|| {


Java's Partitioning.partitionType only considers allActiveFieldIds, which filters out fields whose source column is gone (schema.findField(field.sourceId()) != null in Partitioning.java). Here we return ErrorKind::Unexpected instead. If I'm reading both right, a table that dropped a column it used to partition on (allowed under https://iceberg.apache.org/spec/#partition-evolution) would error the whole scan when _partition is projected. Would continue-ing on a missing source column match Java's intent better? A test that drops a partition source column would settle it either way.

Changed to skip fields where the source column gone so this should match Java's behavior. However, proving it with a test turned out to be tricky. The test failed; turns out in Spark/Iceberg DDL PartitionSpec.bind() rejects specs referencing dropped columns during writes so the condition cannot arise.

mbutrovich · 2026-06-30T17:56:37Z

        }
    }
+
+    fn create_struct_column(


create_struct_column is hand-rolled struct assembly that arrow-rs already provides: the empty/all-null path is StructArray::new_null(fields, len) / new_null_array (arrow-array/src/array/struct_array.rs:192), and the populated path is StructArray::try_new(fields, arrays, nulls), which validates fields.len() == arrays.len() and child-length agreement (struct_array.rs:106). I'd reuse those rather than maintain our own constructor: try_new also enforces the fields/child_values length invariant that the current zip silently truncates. Same principle as the value.rs note: prefer the library's validated constructors over duplicating them here.

Done. Thanks for pointing this one out!

mbutrovich · 2026-06-30T18:02:34Z

+    partition_specs: impl Iterator<Item = &'a PartitionSpec>,
+    schema: &Schema,
+) -> Result<StructType> {
+    let mut seen_field_ids = std::collections::HashSet::new();


Small consistency thing: use std::collections::HashSet; / use std::cmp::Reverse; up top matches the surrounding style.

mbutrovich · 2026-06-30T18:03:57Z

+                    let struct_type = DataType::Struct(pc.fields.clone());
+                    let nullable = pc.fields.is_empty();
+                    let arrow_field =
+                        Field::new(RESERVED_COL_NAME_PARTITION, struct_type, nullable)


This exact block recurs several times across these PRs. A small fn field_with_id(name, dt, nullable, field_id) -> Arc<Field> helper would remove the repetition and keep the field-id metadata format in one place.

Done. Created a new field_with_id helper function

mbutrovich · 2026-06-30T18:04:37Z

@@ -138,6 +142,7 @@ impl ManifestEntryContext {
            // TODO: Pass actual PartitionSpec through context chain for native flow
            .with_partition_spec(None)


build_partition_column_constant needs task.partition_spec to be Some for partitioned tables, but this PR keeps partition_spec(None); #2695 is the one that wires the real spec through context.rs. Is the plan to rebase on #2695? If so, an end-to-end test asserting non-null partition values would confirm the wiring once rebased.

The plan is for this PR to go in first and have #2695 rebase. I'll add end to end tests for all metadata fields in the Comet PR

parthchandra · 2026-06-30T23:45:27Z

We can do the equivalent in Comet - https://github.com/parthchandra/datafusion-comet/blob/iceberg-metadata-columns/native/core/src/execution/planner.rs#L3409-L3440

Are you able to demonstrate this feature working in Comet? Usually when I bring an enhancement to iceberg-rust motivated by Comet usage, I create a draft PR on the Comet repo with tests and the feature built out (using the PR branch here) to demonstrate it works with the Iceberg Java suites.

I've opened a draft PR in Comet: apache/datafusion-comet#4752 based on the same branch I referenced above. All existing tests pass.
The new tests covering this PR are in https://github.com/parthchandra/datafusion-comet/blob/iceberg-metadata-columns/spark/src/test/resources/sql-tests/iceberg/metadata_column_partition.sql

Working on addressing your newer comments (thanks for the detailed review!).

parthchandra · 2026-07-06T21:10:59Z

@mbutrovich @advancedxy addressed all your review comments.
Sync'd with @hsiang-c and will rebase this on top of #2695 (it will be easier)

parthchandra · 2026-07-10T17:21:37Z

Comet (draft) PR with @hsiang-c 's PRs included. CI is green. apache/datafusion-comet#4752
(branch with all the PRs merged - https://github.com/parthchandra/iceberg-rust/tree/metadata-columns-unified )

parthchandra · 2026-07-10T17:22:50Z

@mbutrovich could you re-review once @hsiang-c 's PRs are merged and I rebase on top of them?

advancedxy · 2026-07-21T12:34:55Z

@parthchandra since other prs are merged, would you like to rebase the PR on the master and let's continue.

parthchandra · 2026-07-21T20:56:53Z

@advancedxy, @mbutrovich, @CTTY rebased this PR after #2695 and
#2746 merged.

The corresponding PR in Comet: apache/datafusion-comet#4752 adds tests as well. (Note the CI failure is an issue in Comet main and not related to this)

PTAL

advancedxy

Left some style comments, the code should be correct as far as I can see.

advancedxy · 2026-07-22T07:05:07Z


+        // Compute unified partition type if _partition is projected
+        let unified_partition_type = if field_ids.contains(&RESERVED_FIELD_ID_PARTITION) {
+            let upt = compute_unified_partition_type(


nit: upt seems a bit of unusual in the iceberg-rust repo. how about simply call it partition_type or unified_type?

advancedxy · 2026-07-22T07:20:14Z

    snapshot_schema: Arc<IcebergSchema>,
    projected_iceberg_field_ids: Vec<i32>,
-    constant_fields: HashMap<i32, Datum>,
+    metadata_columns: HashMap<i32, MetadataColumnSource>,


hmm, I'd prefer to use constant fields here, which aligns with java's concept. Although, the constant fields are almost always metadata columns. But the identity partition source field is also injected into the constant map.

changed. Also renamed the enum from MetadataColumnSource to ColumnConstant since the values aren't exclusively metadata

advancedxy · 2026-07-22T07:21:58Z

+
+/// Pre-computed data for the _partition struct constant.
+#[derive(Debug, Clone, PartialEq)]
+pub struct PartitionColumnConstant {


this could be called as StructConstant? It could cover other struct constant if needed, not just the partition column.

advancedxy · 2026-07-22T07:29:36Z

+                        let struct_type = DataType::Struct(pc.fields().clone());
+                        let nullable = pc.fields().is_empty();
+                        let arrow_field = field_with_id(
+                            RESERVED_COL_NAME_PARTITION,


I think this branch specialized/assumed the MetadataColumnSource::Struct must come from the _partition field, which is true for current code but might change when the spec evolves. It would be better to check the field_id is indeed the partition col field first.

added a check for field id. Also for unexpected struct constants, we now return an error.

advancedxy

LGTM now, thanks for the effort, it's great work.

parthchandra · 2026-07-23T16:39:00Z

Thanks @advancedxy. @CTTY could we get a committer review/approval?

hsiang-c

Thank you @parthchandra LGTM

CTTY

Hi, thanks for your contribution!

CTTY · 2026-07-23T23:38:28Z

+                    Some(ColumnConstant::Struct(_)) => {
+                        return Err(Error::new(
+                            ErrorKind::Unexpected,
+                            format!("Unexpected struct constant for field id {field_id}"),


nit: can we make this error message more explicit? Like

format!( "Struct column constants are only supported for the `_partition` \ metadata column (field id {RESERVED_FIELD_ID_PARTITION}), but one \ was set for field id {field_id}" )

CTTY · 2026-07-23T23:38:54Z

+                    Some(ColumnConstant::Struct(_)) => {
+                        return Err(Error::new(
+                            ErrorKind::Unexpected,
+                            format!("Unexpected struct constant for field id {field_id}"),


nit: same here, the error message is a bit fuzzy

CTTY · 2026-07-23T23:44:53Z

+/// For each field in the unified partition type:
+/// - If it corresponds to a field in this file's partition spec, use the value from partition_data
+/// - Otherwise (partition evolution), use null
+pub fn build_partition_column_constant(


nit: similarly, build_partition_constant could be better. We can make the naming consistent

does this really need to be pub? I think a pub(crate) is more suitable but not sure if I overlooked anything.

Having 3 loosely coupled arguments for a pub function makes it easier for users mistakes

CTTY · 2026-07-23T23:48:11Z

This feels unneeded since we now have field_with_id

CTTY · 2026-07-23T23:52:50Z

+/// Covers both scalar constants (metadata columns like `_file` and `_spec_id`,
+/// as well as identity partition source fields) and the struct `_partition` column.
+#[derive(Debug, Clone, PartialEq)]
+pub enum ColumnConstant {


This should be pub(crate)

CTTY · 2026-07-23T23:52:54Z

+
+/// Pre-computed data for a struct constant column.
+#[derive(Debug, Clone, PartialEq)]
+pub struct StructConstant {


This should be pub(crate)

CTTY · 2026-07-24T21:43:49Z

    }

+    /// Set a pre-computed _partition column constant directly.
+    pub(crate) fn with_partition_column_precomputed(


nit: I think with_partition_constant is more straightforward

CTTY · 2026-07-24T23:01:53Z

+
+    for spec in specs {
+        for field in spec.fields() {
+            if seen_field_ids.contains(&field.field_id) {


Can we revisit this? Just read the java impl (Partitioning::buildPartitionProjectionType), and my understanding for the void transform handling is:

If it's new field, we record its id, type, and name. If we have an existing newer field and the current field is the older field, go to 2

We check if the newer field is Void transform AND if the older field is not Void transform, which implies that the field was dropped. In this case, java would preserve the type of the older field. But I don't see this happening in the current function?

Great point @CTTY. This did not match the java version. Redid this to match java's buildPartitionProjectionType. so that when a newer spec marks a field Void but an older spec has it with a real transform, the older spec's type is preserved while the newer spec's name is kept. Introduced all_active_field_ids helper matching Java's allActiveFieldIds.

CTTY · 2026-07-24T23:02:07Z

+/// # Arguments
+/// * `partition_specs` - Iterator over all partition specs in the table
+/// * `schema` - The current table schema (needed to determine result types of transforms)
+pub fn compute_unified_partition_type<'a>(


We should add unit tests for this function

added unit tests -
single spec identity,
year transform,
unpartitioned,
multiple fields sorted by id,
newer name precedence, void replaced by older non-void,
dropped source column skipped, and partition evolution adding new fields

parthchandra

@CTTY thank you for your review. Addressed your comments. Also, added some tests to cover compute_unified_partition_type in the Comet tests (draft PR: apache/datafusion-comet#4752) which compare with Spark results

parthchandra · 2026-07-27T00:27:11Z

+/// # Arguments
+/// * `partition_specs` - Iterator over all partition specs in the table
+/// * `schema` - The current table schema (needed to determine result types of transforms)
+pub fn compute_unified_partition_type<'a>(


added unit tests -
single spec identity,
year transform,
unpartitioned,
multiple fields sorted by id,
newer name precedence, void replaced by older non-void,
dropped source column skipped, and partition evolution adding new fields

parthchandra · 2026-07-27T00:35:04Z

+
+    for spec in specs {
+        for field in spec.fields() {
+            if seen_field_ids.contains(&field.field_id) {


Great point @CTTY. This did not match the java version. Redid this to match java's buildPartitionProjectionType. so that when a newer spec marks a field Void but an older spec has it with a real transform, the older spec's type is preserved while the newer spec's name is kept. Introduced all_active_field_ids helper matching Java's allActiveFieldIds.

parthchandra · 2026-07-27T03:15:49Z

+/// Covers both scalar constants (metadata columns like `_file` and `_spec_id`,
+/// as well as identity partition source fields) and the struct `_partition` column.
+#[derive(Debug, Clone, PartialEq)]
+pub enum ColumnConstant {


parthchandra · 2026-07-27T03:16:02Z

+
+/// Pre-computed data for a struct constant column.
+#[derive(Debug, Clone, PartialEq)]
+pub struct StructConstant {


parthchandra · 2026-07-27T03:16:12Z

    }

+    /// Set a pre-computed _partition column constant directly.
+    pub(crate) fn with_partition_column_precomputed(


parthchandra · 2026-07-27T03:16:28Z

+                    Some(ColumnConstant::Struct(_)) => {
+                        return Err(Error::new(
+                            ErrorKind::Unexpected,
+                            format!("Unexpected struct constant for field id {field_id}"),


parthchandra · 2026-07-27T03:16:41Z

+                    Some(ColumnConstant::Struct(_)) => {
+                        return Err(Error::new(
+                            ErrorKind::Unexpected,
+                            format!("Unexpected struct constant for field id {field_id}"),


parthchandra · 2026-07-27T03:17:03Z

+/// For each field in the unified partition type:
+/// - If it corresponds to a field in this file's partition spec, use the value from partition_data
+/// - Otherwise (partition evolution), use null
+pub fn build_partition_column_constant(


CTTY · 2026-07-27T21:42:43Z

Hi @parthchandra , looks like the code is not updated yet. Maybe you have commits that are not pushed to the remote?

parthchandra · 2026-07-27T22:25:55Z

@CTTY 6bf928a has the changes but they don't seem to be showing up. Let me rebase and push again.

parthchandra · 2026-07-27T23:05:59Z

@CTTY updated (had to force push)

parthchandra · 2026-07-28T16:08:55Z

Comet's apache/datafusion-comet#4752 CI is green with this PR.
Comet Iceberg tests for metadata columns are in CometIcebergNativeSuite.scala

CTTY

Thanks for working on this! Just took another pass

CTTY · 2026-07-28T21:46:35Z

-                // For identity-partitioned fields, the Iceberg spec's "Column Projection" rules
-                // only apply to "field ids which are not present in a data file". When the column
-                // IS present in the Parquet file, it must be read from the file; the partition
-                // metadata constant is only a fallback for when the column is absent (e.g. add_files).


this comment still holds true, can we keep this?

Added back on the scalar constant arm

CTTY · 2026-07-28T22:10:53Z

+
+    let active_field_ids = all_active_field_ids(specs.iter().copied(), schema);
+
+    let mut field_map: HashMap<i32, &crate::spec::PartitionField> = HashMap::new();


nit: we can just import this

Suggested change

let mut field_map: HashMap<i32, &crate::spec::PartitionField> = HashMap::new();

let mut field_map: HashMap<i32, &PartitionField> = HashMap::new();

CTTY · 2026-07-28T23:33:30Z

+
+            if matches!(field.transform, Transform::Unknown) {
+                return Err(Error::new(
+                    ErrorKind::FeatureUnsupported,


I'd say it's data invalid error. We can't support unknown transform since we don't know the result type

Fair point. Changed to ErrorKind::DataInvalid

CTTY · 2026-07-28T23:34:26Z

+                    name_map.insert(field_id, field.name.clone());
+                }
+                Some(existing) => {
+                    if is_void_transform(existing) && !is_void_transform(field) {


Java does a compatibility check before this to compare the existing field and the new field, and we need this because V1 doesn't ensure the field id is unique across specs: https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/Partitioning.java#L295

Thank you for catching this! Added the compatibility check.

CTTY · 2026-07-28T23:36:55Z

+                continue;
+            }
+
+            if matches!(field.transform, Transform::Unknown) {


This check should be moved before the continue above, otherwise it may be skipped

Done. moved

CTTY · 2026-07-28T23:41:35Z

+                (Some(spec), Some(data)) => (spec.clone(), data.clone()),
+                // Unpartitioned table or missing spec: build_partition_constant
+                // handles the empty unified_type case with an early return.
+                _ => (Arc::new(PartitionSpec::unpartition_spec()), Struct::empty()),


We should fail if !unified_type.fields().is_empty()

Done. Now errors if the unified type has fields but spec/data is missing.

CTTY · 2026-07-29T00:04:50Z

+            let name = &name_map[&fid];
+            let ty = type_map.remove(&fid).unwrap();


I understand that this is unlikely to fail due to the way it was constructed. But I still think we should not unwrap here, how about the below?

let struct_fields = field_ids .into_iter() .map(|fid| -> Result<NestedFieldRef> { let name = name_map.get(&fid).ok_or_else(|| { Error::new( ErrorKind::Unexpected, format!("Missing name for partition field {fid}"), ) })?; let ty = type_map.remove(&fid).ok_or_else(|| { Error::new( ErrorKind::Unexpected, format!("Missing type for partition field {fid}"), ) })?; Ok(NestedField::optional(fid, name, ty).into()) }) .collect::<Result<Vec<_>>>()?;

Fair enough. Replaced with ok_or_else.

parthchandra

Thank you for the very thorough review @CTTY!

parthchandra · 2026-07-29T17:16:34Z

+                (Some(spec), Some(data)) => (spec.clone(), data.clone()),
+                // Unpartitioned table or missing spec: build_partition_constant
+                // handles the empty unified_type case with an early return.
+                _ => (Arc::new(PartitionSpec::unpartition_spec()), Struct::empty()),


Done. Now errors if the unified type has fields but spec/data is missing.

parthchandra · 2026-07-29T17:18:03Z

+
+    let active_field_ids = all_active_field_ids(specs.iter().copied(), schema);
+
+    let mut field_map: HashMap<i32, &crate::spec::PartitionField> = HashMap::new();


parthchandra · 2026-07-29T17:18:50Z

+                continue;
+            }
+
+            if matches!(field.transform, Transform::Unknown) {


Done. moved

parthchandra · 2026-07-29T17:20:12Z

+
+            if matches!(field.transform, Transform::Unknown) {
+                return Err(Error::new(
+                    ErrorKind::FeatureUnsupported,


Fair point. Changed to ErrorKind::DataInvalid

parthchandra · 2026-07-29T17:21:46Z

+                    name_map.insert(field_id, field.name.clone());
+                }
+                Some(existing) => {
+                    if is_void_transform(existing) && !is_void_transform(field) {


Thank you for catching this! Added the compatibility check.

parthchandra · 2026-07-29T17:24:10Z

+            let name = &name_map[&fid];
+            let ty = type_map.remove(&fid).unwrap();


Fair enough. Replaced with ok_or_else.

parthchandra · 2026-07-29T17:26:48Z

-                // For identity-partitioned fields, the Iceberg spec's "Column Projection" rules
-                // only apply to "field ids which are not present in a data file". When the column
-                // IS present in the Parquet file, it must be read from the file; the partition
-                // metadata constant is only a fallback for when the column is absent (e.g. add_files).


Added back on the scalar constant arm

CTTY

LGTM! Thanks for contributing this feature

parthchandra · 2026-07-29T23:44:08Z

Thank you @CTTY @advancedxy @mbutrovich @hsiang-c !

parthchandra marked this pull request as draft June 18, 2026 00:34

parthchandra force-pushed the metadata-columns branch from 074f903 to caa7fb0 Compare June 18, 2026 01:19

parthchandra marked this pull request as ready for review June 18, 2026 01:46

advancedxy reviewed Jun 21, 2026

View reviewed changes

parthchandra force-pushed the metadata-columns branch from caa7fb0 to 33b690a Compare June 22, 2026 21:05

advancedxy reviewed Jun 23, 2026

View reviewed changes

Comment thread crates/iceberg/src/partitioning.rs Outdated

This was referenced Jun 23, 2026

map Iceberg Binary to Arrow Binary instead of LargeBinary #2698

Open

refactor: unify ColumnSource::Add and AddStructConstant using Literal #2699

Open

mbutrovich self-requested a review June 24, 2026 17:41

advancedxy reviewed Jun 25, 2026

View reviewed changes

viirya mentioned this pull request Jun 25, 2026

fix(arrow): map Iceberg Binary to Arrow Binary instead of LargeBinary #2710

Closed

parthchandra force-pushed the metadata-columns branch from 4df3d67 to 8da37c9 Compare June 29, 2026 22:39

mbutrovich reviewed Jun 30, 2026

View reviewed changes

mbutrovich mentioned this pull request Jun 30, 2026

feat: add support for _spec_id metadata column #2695

Merged

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

mbutrovich mentioned this pull request Jul 10, 2026

feat: add support for _pos metadata column #2746

Merged

parthchandra force-pushed the metadata-columns branch from b91fbd2 to ec53b41 Compare July 21, 2026 16:31

parthchandra force-pushed the metadata-columns branch from 89d5001 to f70931a Compare July 21, 2026 17:22

advancedxy reviewed Jul 22, 2026

View reviewed changes

advancedxy approved these changes Jul 23, 2026

View reviewed changes

hsiang-c approved these changes Jul 23, 2026

View reviewed changes

CTTY reviewed Jul 24, 2026

View reviewed changes

parthchandra commented Jul 27, 2026

View reviewed changes

parthchandra added 4 commits July 27, 2026 15:24

feat: add support for _partition metadata column

06b4a85

fix after rebase

804cdfc

fix public api

2ddb7b4

address review comments

eebccd6

address review comments

46bc0fb

parthchandra force-pushed the metadata-columns branch from 6bf928a to 46bc0fb Compare July 27, 2026 22:37

CTTY reviewed Jul 29, 2026

View reviewed changes

address review comments

65322f1

parthchandra commented Jul 29, 2026

View reviewed changes

CTTY approved these changes Jul 29, 2026

View reviewed changes

CTTY merged commit 3d84c81 into apache:main Jul 29, 2026
21 checks passed

CTTY linked an issue Jul 29, 2026 that may be closed by this pull request

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

Closed


		seen_field_ids.insert(field.field_id);

		let source_field = schema.field_by_id(field.source_id).ok_or_else(\|\| {

		@@ -138,6 +142,7 @@ impl ManifestEntryContext {
		// TODO: Pass actual PartitionSpec through context chain for native flow
		.with_partition_spec(None)


		let active_field_ids = all_active_field_ids(specs.iter().copied(), schema);

		let mut field_map: HashMap<i32, &crate::spec::PartitionField> = HashMap::new();

	let mut field_map: HashMap<i32, &crate::spec::PartitionField> = HashMap::new();
	let mut field_map: HashMap<i32, &PartitionField> = HashMap::new();

		let name = &name_map[&fid];
		let ty = type_map.remove(&fid).unwrap();

Uh oh!

Conversation

parthchandra commented Jun 18, 2026

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

Uh oh!

parthchandra commented Jun 18, 2026

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!

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!

advancedxy commented Jun 21, 2026

Uh oh!

Uh oh!

advancedxy commented Jun 23, 2026

Uh oh!

mbutrovich commented Jun 23, 2026

Uh oh!

parthchandra commented Jun 23, 2026

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!

mbutrovich commented Jun 25, 2026

Uh oh!

parthchandra commented Jun 26, 2026

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!

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!

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!