You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "zhuqi-lucas (via GitHub)" <gi...@apache.org> on 2026/03/08 02:50:36 UTC
[PR] fix: handle Null type in try_merge for Struct, List, LargeList, and Union [arrow-rs]
zhuqi-lucas opened a new pull request, #9524:
URL: https://github.com/apache/arrow-rs/pull/9524
# Which issue does this PR close?
Field::try_merge correctly handles DataType::Null for primitive types
and when self is Null, but fails when self is a compound type (Struct,
List, LargeList, Union) and from is Null. This causes Schema::try_merge
to error when merging schemas where one has a Null field and another has a
concrete compound type for the same field.
This is common in JSON inference where some files have null values for
fields that are structs/lists in other files.
- Closes[ #9523](https://github.com/apache/arrow-rs/issues/9523)
# Rationale for this change
Add `DataType::Null` arms to the Struct, List, LargeList, and Union
branches in `Field::try_merge`, consistent with how primitive types
already handle it.
# What changes are included in this PR?
Add `DataType::Null` arms to the Struct, List, LargeList, and Union
branches in `Field::try_merge`, consistent with how primitive types
already handle it.
# Are these changes tested?
- Added test `test_merge_compound_with_null` covering Struct, List,
LargeList, and Union merging with Null in both directions.
- Existing tests continue to pass.
# Are there any user-facing changes?
No
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: handle Null type in try_merge for Struct, List, LargeList, and Union [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #9524:
URL: https://github.com/apache/arrow-rs/pull/9524
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: handle Null type in try_merge for Struct, List, LargeList, and Union [arrow-rs]
Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #9524:
URL: https://github.com/apache/arrow-rs/pull/9524#issuecomment-4018047014
cc @alamb , i met some corner cases when using infer schema, and i found it's the bug from arrow side. I hope this PR is helpful.
error:
```rust
Error: Arrow error: Schema error: Fail to merge schema field 'item' because the from data_type = Null is not DataType::Struct
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: handle Null type in try_merge for Struct, List, LargeList, and Union [arrow-rs]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #9524:
URL: https://github.com/apache/arrow-rs/pull/9524#discussion_r2901180084
##########
arrow-schema/src/field.rs:
##########
@@ -1461,4 +1473,58 @@ mod test {
assert_binary_serde_round_trip(field)
}
+
+ #[test]
+ fn test_merge_compound_with_null() {
+ // Struct + Null
+ let mut field = Field::new(
+ "s",
+ DataType::Struct(Fields::from(vec![Field::new("a", DataType::Int32, false)])),
+ false,
+ );
+ field
+ .try_merge(&Field::new("s", DataType::Null, true))
+ .expect("Struct should merge with Null");
+ assert!(field.is_nullable());
+ assert!(matches!(field.data_type(), DataType::Struct(_)));
+
+ // List + Null
+ let mut field = Field::new(
+ "l",
+ DataType::List(Field::new("item", DataType::Utf8, false).into()),
+ false,
+ );
+ field
+ .try_merge(&Field::new("l", DataType::Null, true))
+ .expect("List should merge with Null");
+ assert!(field.is_nullable());
+ assert!(matches!(field.data_type(), DataType::List(_)));
+
+ // LargeList + Null
+ let mut field = Field::new(
+ "ll",
+ DataType::LargeList(Field::new("item", DataType::Utf8, false).into()),
+ false,
+ );
+ field
+ .try_merge(&Field::new("ll", DataType::Null, true))
+ .expect("LargeList should merge with Null");
+ assert!(field.is_nullable());
+ assert!(matches!(field.data_type(), DataType::LargeList(_)));
+
+ // Union + Null
+ let mut field = Field::new(
+ "u",
+ DataType::Union(
+ UnionFields::try_new(vec![0], vec![Field::new("f", DataType::Int32, false)])
+ .unwrap(),
+ UnionMode::Dense,
+ ),
+ false,
+ );
+ field
+ .try_merge(&Field::new("u", DataType::Null, true))
+ .expect("Union should merge with Null");
Review Comment:
The Union + Null subtest doesn't assert that the field became nullable after the merge. Please add an `assert!(field.is_nullable())` here (similar to the Struct/List/LargeList cases) so the test actually validates the new Union-specific `DataType::Null` handling.
```suggestion
.expect("Union should merge with Null");
assert!(field.is_nullable());
```
##########
arrow-schema/src/field.rs:
##########
@@ -1461,4 +1473,58 @@ mod test {
assert_binary_serde_round_trip(field)
}
+
+ #[test]
+ fn test_merge_compound_with_null() {
+ // Struct + Null
+ let mut field = Field::new(
+ "s",
Review Comment:
PR description says the added test covers merging with Null "in both directions", but this test only exercises `compound.try_merge(Null)`. Consider adding a reverse-direction assertion (`Null.try_merge(compound)`) for at least one (or all) of Struct/List/LargeList/Union to match the stated coverage and guard the `DataType::Null` self-branch behavior.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org