Skip to content

Conversation

yongkyunlee
Copy link
Contributor

@yongkyunlee yongkyunlee commented Aug 22, 2025

Which issue does this PR close?

Closes #8209

Rationale for this change

In the Field struct definition

/// A field within a [`Record`]
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct Field<'a> {
    /// Name of the field within the record
    #[serde(borrow)]
    pub name: &'a str,
    /// Optional documentation for this field
    #[serde(borrow, default)]
    pub doc: Option<&'a str>,
    /// The field's type definition
    #[serde(borrow)]
    pub r#type: Schema<'a>,
    /// Optional default value for this field
    #[serde(borrow, default)]
    pub default: Option<&'a str>,
}

type is of type Schema whereas default is of type str. The default should be supported for all types (e.g. int, array, map, nested record), so we should make it more lenient.

More details on reproduction is mentioned in the Github Issue.

What changes are included in this PR?

Relaxation of default type of avro scheam Field.

Are these changes tested?

Added a unit test.

Are there any user-facing changes?

It affects pub struct Field of arrow-avro package, but the impact should be minimal as the default attribute is not being used.

@yongkyunlee yongkyunlee changed the title Support more default types in avro to arrow schema conversion Support all default types for avro schema's record field Aug 22, 2025
@alamb
Copy link
Contributor

alamb commented Aug 23, 2025

@jecsand838 do you have time to review this PR?

@alamb alamb changed the title Support all default types for avro schema's record field [avro] Support all default types for avro schema's record field Aug 23, 2025
@alamb alamb added the arrow-avro arrow-avro crate label Aug 23, 2025
@github-actions github-actions bot added the arrow Changes to the arrow crate label Aug 23, 2025
@jecsand838
Copy link
Contributor

@yongkyunlee Thanks for making this contribution!

LMGTM, just had one nit.

@yongkyunlee
Copy link
Contributor Author

Thanks! Addressed the comment
@alamb @jecsand838

@mbrobbel mbrobbel added next-major-release the PR has API changes and it waiting on the next major version api-change Changes to the arrow API labels Aug 25, 2025
@alamb alamb merged commit ad756f9 into apache:main Aug 26, 2025
23 checks passed
@alamb
Copy link
Contributor

alamb commented Aug 26, 2025

Thanks @yongkyunlee @jecsand838 and @mbrobbel 🚀

@mbrobbel
Copy link
Member

Shouldn't we wait until after 56.2.0 (because this is a breaking change)?

@mbrobbel
Copy link
Member

Shouldn't we wait until after 56.2.0 (because this is a breaking change)?

Ah, I just learned this isn't public yet.

@alamb
Copy link
Contributor

alamb commented Aug 26, 2025

Shouldn't we wait until after 56.2.0 (because this is a breaking change)?

Ah, I just learned this isn't public yet.

Indeed -- but thank you for the review. I think the idea is that we'll bash on with new feature development / breaking API changes, etc until we are satisfied this crate is ready, and then we'll start subjecting it to the same API versioning as other crates.

@alamb alamb removed api-change Changes to the arrow API next-major-release the PR has API changes and it waiting on the next major version labels Sep 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate arrow-avro arrow-avro crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avro to arrow schema conversion fails when a field has a default type that is not string
4 participants