viirya commented on code in PR #2635:
URL: https://github.com/apache/iceberg-rust/pull/2635#discussion_r4188966510


##########
crates/iceberg/src/arrow/record_batch_transformer.rs:
##########
@@ -1360,6 +1371,51 @@ mod test {
         );
     }
 
+    #[test]
+    fn schema_evolution_absent_struct_with_initial_default_errors() {
+        for required in [false, true] {
+            let added_field = NestedField::new(
+                2,
+                "added_struct",
+                Type::Struct(crate::spec::StructType::new(vec![
+                    NestedField::optional(3, "child", 
Type::Primitive(PrimitiveType::Int))
+                        .with_initial_default(Literal::int(42))
+                        .into(),
+                ])),
+                required,
+            )
+            
.with_initial_default(Literal::Struct(Struct::from_iter(vec![None])));
+            let snapshot_schema = Arc::new(
+                Schema::builder()
+                    .with_fields(vec![
+                        NestedField::required(1, "id", 
Type::Primitive(PrimitiveType::Int)).into(),
+                        added_field.into(),
+                    ])
+                    .build()
+                    .unwrap(),
+            );
+            let mut transformer =
+                RecordBatchTransformerBuilder::new(snapshot_schema, &[1, 
2]).build();
+            let file_schema = Arc::new(ArrowSchema::new(vec![field_with_id(
+                "id",
+                DataType::Int32,
+                false,
+                1,
+            )]));
+            let file_batch =
+                RecordBatch::try_new(file_schema, 
vec![Arc::new(Int32Array::from(vec![1, 2, 3]))])
+                    .unwrap();
+
+            let err = 
transformer.process_record_batch(file_batch).unwrap_err();
+            assert_eq!(err.kind(), crate::ErrorKind::FeatureUnsupported);

Review Comment:
   Added `required={required}` to the assertions and the unexpected-success 
message, so failures identify which case was running.



##########
crates/iceberg/src/arrow/value.rs:
##########
@@ -526,24 +526,24 @@ pub(crate) fn create_primitive_array_single_element(
     data_type: &DataType,
     prim_lit: Option<&PrimitiveLiteral>,
 ) -> Result<ArrayRef> {
+    // No value: a single NULL of any (possibly nested) type (#2618). The `1` 
is

Review Comment:
   Trimmed the comment to the intent and removed the issue reference and 
row-count explanation. Also removed the issue reference from the repeated 
helper’s comment.



##########
crates/iceberg/src/arrow/value.rs:
##########
@@ -526,24 +526,24 @@ pub(crate) fn create_primitive_array_single_element(
     data_type: &DataType,
     prim_lit: Option<&PrimitiveLiteral>,
 ) -> Result<ArrayRef> {
+    // No value: a single NULL of any (possibly nested) type (#2618). The `1` 
is
+    // `new_null_array`'s row count.
+    if prim_lit.is_none() {
+        return Ok(new_null_array(data_type, 1));

Review Comment:
   Added a table-driven test covering both helpers with `None` for 
`Decimal128(10, 2)`, a microsecond timestamp with `+00:00`, a struct containing 
a list, and `Null`. It checks the exact data type, length, and NULL count, 
including repeated arrays with 0, 1, and 3 rows.
   
   One Arrow detail: `NullArray` has no validity bitmap, so its physical 
`null_count()` is zero. The test checks `logical_null_count()` for every type 
and additionally checks physical `null_count()` for the other types.



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