c-thiel commented on code in PR #2188:
URL: https://github.com/apache/iceberg-rust/pull/2188#discussion_r3588481213


##########
crates/iceberg/src/arrow/reader/projection.rs:
##########
@@ -81,7 +81,111 @@ impl ArrowReader {
                 Self::include_leaf_field_id(&map_type.key_field, field_ids);
                 Self::include_leaf_field_id(&map_type.value_field, field_ids);
             }
+            // Variant is a leaf type for Parquet projection purposes (like a 
primitive).
+            Type::Variant(_) => {
+                field_ids.push(field.id);
+            }
+        }
+    }
+
+    /// Recursive DFS over an Arrow `Fields` tree whose leaf numbering matches
+    /// `arrow_schema::Fields::filter_leaves`. For every leaf sitting inside a
+    /// variant column, stores `leaf_idx → variant_field_id` in `out`. A
+    /// "variant column" is any Arrow field whose embedded Parquet field id
+    /// resolves to `Type::Variant` in the Iceberg schema — including variants
+    /// nested inside a struct, list, or map.

Review Comment:
   Good catch — that parallel leaf-numbering was only safe by accident: Parquet 
can't emit Dictionary/RunEndEncoded/Union today, but the invariant shouldn't 
rest on writer capabilities.
   
   In this PR variant read is descoped — collect_variant_leaves_in_field is 
removed and projecting a variant now returns FeatureUnsupported. So this is 
resolved here by deletion.
   
   When read comes back in the follow-up PR, I won't reintroduce the fragile 
parallel traversal.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to